From: Junio C Hamano <firstname.lastname@example.org> To: René Scharfe <email@example.com> Cc: Git List <firstname.lastname@example.org>, Johannes Schindelin <email@example.com> Subject: Re: [PATCH] fast-export: avoid NULL pointer arithmetic Date: Thu, 10 May 2018 19:51:08 +0900 Message-ID: <firstname.lastname@example.org> (raw) In-Reply-To: <email@example.com> René Scharfe <firstname.lastname@example.org> 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.
next prev 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 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 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 \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ /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
firstname.lastname@example.org list mirror (unofficial, 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/ AGPL code for this site: git clone https://public-inbox.org/ public-inbox