git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
From: René Scharfe <l.s.r@web.de>
To: Git List <git@vger.kernel.org>
Cc: Junio C Hamano <gitster@pobox.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: [PATCH] fast-export: avoid NULL pointer arithmetic
Date: Thu, 10 May 2018 11:24:01 +0200
Message-ID: <99d443cd-e817-7db5-f758-bf4cf47f7c06@web.de> (raw)
In-Reply-To: <d50a4d5d-3b99-453e-1b52-4e733453fb78@web.de>

Am 09.05.2018 um 23:06 schrieb René Scharfe:
> Clang 6 reports the following warning, which is turned into an error in a
> DEVELOPER build:
> 
> 	builtin/fast-export.c:162:28: error: performing pointer arithmetic on a null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic]
> 		return ((uint32_t *)NULL) + mark;
> 		       ~~~~~~~~~~~~~~~~~~ ^
> 	1 error generated.
> 
> The compiler is correct, and the error message speaks for itself.  There
> is no need for any undefined operation -- just cast mark to void * or
> uint32_t after an intermediate cast to uintptr_t.  That encodes the
> integer value into a pointer and later decodes it as intended.

Having thought about it a bit more I have to say: That seems to work,
but it's not portable.  

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?

> While at it remove an outdated comment -- intptr_t has been used since
> ffe659f94d (parse-options: make some arguments optional, add callbacks),
> committed in October 2007.
> 
> Signed-off-by: Rene Scharfe <l.s.r@web.de>
> ---
>   builtin/fast-export.c | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/builtin/fast-export.c b/builtin/fast-export.c
> index 530df12f05..fa556a3c93 100644
> --- a/builtin/fast-export.c
> +++ b/builtin/fast-export.c
> @@ -156,15 +156,14 @@ static void anonymize_path(struct strbuf *out, const char *path,
>   	}
>   }
>   
> -/* Since intptr_t is C99, we do not use it here */
> -static inline uint32_t *mark_to_ptr(uint32_t mark)
> +static inline void *mark_to_ptr(uint32_t mark)
>   {
> -	return ((uint32_t *)NULL) + mark;
> +	return (void *)(uintptr_t)mark;
>   }
>   
>   static inline uint32_t ptr_to_mark(void * mark)
>   {
> -	return (uint32_t *)mark - (uint32_t *)NULL;
> +	return (uint32_t)(uintptr_t)mark;
>   }
>   
>   static inline void mark_object(struct object *object, uint32_t mark)
> 

  parent reply index

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-09 21:06 René Scharfe
2018-05-09 21:43 ` Johannes Schindelin
2018-05-10  9:24 ` René Scharfe [this message]
2018-05-10 10:51   ` Junio C Hamano
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 publically 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=99d443cd-e817-7db5-f758-bf4cf47f7c06@web.de \
    --to=l.s.r@web.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.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

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox