git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "René Scharfe" <l.s.r@web.de>
Cc: Git List <git@vger.kernel.org>,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: [PATCH] fast-export: avoid NULL pointer arithmetic
Date: Thu, 10 May 2018 19:51:08 +0900	[thread overview]
Message-ID: <xmqqo9hniy1v.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <99d443cd-e817-7db5-f758-bf4cf47f7c06@web.de> ("René Scharfe"'s message of "Thu, 10 May 2018 11:24:01 +0200")

René Scharfe <l.s.r@web.de> writes:

> The standard says about uintptr_t that "any valid pointer to void can
> be converted to this type, then converted back to pointer to void, and
> the result will compare equal to the original pointer".  So void * ->
> uintptr_t -> void * is a proper roundtrip, but that doesn't imply that
> casting arbitrary uintptr_t values to void * would be lossless.
>
> I don't know an architecture where this would bite us, but I wonder if
> there is a cleaner way.  Perhaps changing the type of the decoration
> member of struct decoration_entry in decorate.h to uintptr_t?

In order to ensure "void * -> uintptr_t -> void *" roundtrip holds,
the implementation would guarantee that uintptr_t is wider than
void*, so what you suggest technically makes sense.  We should be
able to store any pointer in the field.  And we should be able to
store any value of an unsigned integral type that is narrower than
uintptr_t.

But it somehow feels backwards in spirit to me, as the reason why we
use "void *" there in the decoration field is because we expect that
we'd have a pointer to some struture most of the time, and we have
to occasionally store a small integer there.  So I'd naively expect
that

	uint32_t mark = 23;
	de->decoration = (void *)mark;

would be a good way to store mark #23 in the field and

	uint32_t mark;
	mark = (typeof(mark))de->decoration;

would be a good way to read it off of the "void *" field.  Of
course, this assume that (void *) is at least as wide as 32-bit and
it also ignores the standard ;-)

This is an unrelated tangent but the mark-to-ptr() and ptr-to-mark()
implementations feel wasteful, especially when we worry about 32-bit
archs.  A naive platform implementation of

	(uint32_t *)mark - (uint32_t *)NULL;

would be ((uintptr_t)mark) / 4, i.e. the de->decoration field will
always have two LSB clear and only utilize top 30-bit to represent
the value of mark.

  reply	other threads:[~2018-05-10 10:51 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-09 21:06 [PATCH] fast-export: avoid NULL pointer arithmetic René Scharfe
2018-05-09 21:43 ` Johannes Schindelin
2018-05-10  9:24 ` René Scharfe
2018-05-10 10:51   ` Junio C Hamano [this message]
2018-05-10 19:47     ` René Scharfe
2018-05-11  2:16       ` Junio C Hamano
2018-05-11  4:49         ` Junio C Hamano
2018-05-11  6:19           ` Duy Nguyen
2018-05-11  8:56             ` Jeff King
2018-05-11 13:11               ` Duy Nguyen
2018-05-11 13:34                 ` Duy Nguyen
2018-05-11 17:42                   ` Jeff King
2018-05-12  8:45                     ` René Scharfe
2018-05-12  8:49                       ` René Scharfe
2018-05-14  1:37                       ` Junio C Hamano
2018-05-15 19:36                         ` René Scharfe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqo9hniy1v.fsf@gitster-ct.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=johannes.schindelin@gmx.de \
    --cc=l.s.r@web.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).