git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] tag: avoid NULL pointer arithmetic

From
Junio C Hamano <gitster@pobox.com>
Date
Oct 2, 2017, 04:13 UTC
Message-ID
<xmqqwp4ep4qb.fsf@gitster.mtv.corp.google.com>
In-Reply-To
<99e56671-bdf9-a59f-ae7b-758f1b7a8f14@web.de>
René Scharfe <l.s.r@web.de> writes:
Show 13 quoted lines
> lookup_blob() etc. can return NULL if the referenced object isn't of the
> expected type.  In theory it's wrong to reference the object member in
> that case.  In practice it's OK because it's located at offset 0 for all
> types, so the pointer arithmetic (NULL + 0) is optimized out by the
> compiler.  The issue is reported by Clang's AddressSanitizer, though.
>
> Avoid the ASan error by casting the results of the lookup functions to
> struct object pointers.  That works fine with NULL pointers as well.  We
> already rely on the object member being first in all object types in
> other places in the code.
>
> Signed-off-by: Rene Scharfe <l.s.r@web.de>
> ---
Yikes, that is tricky.

Taking the address of .object field appears to be a lot cleaner and more kosher than casting, but from the compiler's point of view it's quite the opposite. Sigh.

Thanks.  The patch itself looks like the right solution to me.
Show 25 quoted lines
>  tag.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tag.c b/tag.c
> index 7e10acfb6e..fcbe012f7a 100644
> --- a/tag.c
> +++ b/tag.c
> @@ -142,13 +142,13 @@ int parse_tag_buffer(struct tag *item, const void *data, unsigned long size)
>  	bufptr = nl + 1;
>  
>  	if (!strcmp(type, blob_type)) {
> -		item->tagged = &lookup_blob(&oid)->object;
> +		item->tagged = (struct object *)lookup_blob(&oid);
>  	} else if (!strcmp(type, tree_type)) {
> -		item->tagged = &lookup_tree(&oid)->object;
> +		item->tagged = (struct object *)lookup_tree(&oid);
>  	} else if (!strcmp(type, commit_type)) {
> -		item->tagged = &lookup_commit(&oid)->object;
> +		item->tagged = (struct object *)lookup_commit(&oid);
>  	} else if (!strcmp(type, tag_type)) {
> -		item->tagged = &lookup_tag(&oid)->object;
> +		item->tagged = (struct object *)lookup_tag(&oid);
>  	} else {
>  		error("Unknown type %s", type);
>  		item->tagged = NULL;
Previous: René ScharfeNext: Jeff King
Message 2 of 13 in “tag: avoid NULL pointer arithmetic”
  1. tag: avoid NULL pointer arithmeticRené Scharfe, Oct 1, 2017
  2. Junio C HamanoOct 2, 2017
  3. Jeff KingOct 2, 2017
  4. René ScharfeOct 2, 2017
  5. Jeff KingOct 2, 2017
  6. SZEDER GáborOct 3, 2017
  7. René ScharfeOct 3, 2017
  8. fsck: check results of lookup_blob() and lookup_tree() for NULLRené Scharfe, Oct 3, 2017
  9. Junio C HamanoOct 4, 2017
  10. René ScharfeOct 5, 2017
  11. fsck: handle NULL return of lookup_blob() and lookup_tree()René Scharfe, Oct 5, 2017
  12. Junio C HamanoOct 6, 2017
  13. René ScharfeOct 6, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.