<div>I'm sorry to be a pain, but the key thing is: ".. read from another object..". In the statement you are trying to "fix", the "base" does not actually _read_ from the object (structure, to be precise), instead the address of the structure itself used. In other words, "p->base" is a shortcut for "(uint8_t *) p".<br/>
<br/>
--<br/>
Max<br/><br/>-----Original Message-----<br/>From: Andrew Clayton <andrew@digital-domain.net><br/>To: Alejandro Colomar <alx.manpages@gmail.com><br/>Cc: unit@nginx.org, Max Romanov <max.romanov@gmail.com><br/>Sent: пт, 17 июн. 2022 2:05<br/>Subject: Re: [PATCH 06/11] Sptr: avoided potentially undefined behaviour.<br/><br/></div>On Fri, 17 Jun 2022 00:17:20 +0200 <br/>
Alejandro Colomar <<a href="mailto:alx.manpages@gmail.com">alx.manpages@gmail.com</a>> wrote: <br/>
<br/>
> Woah, wait a bit.  No it doesn't.  Today I learnt: <br/>
>  <br/>
> <a href="http://6.5.16.1/3">6.5.16.1/3</a> (Assignment operators::Simple Assignment::Semantics): <br/>
>  <br/>
> If the value being stored in an object is read from another object that  <br/>
> overlaps in any way the storage of the first object, then the overlap  <br/>
> shall be exact and the two objects shall have qualified or unqualified  <br/>
> versions of a compatible type; otherwise, the behavior is undefined. <br/>
>  <br/>
> <<a href="https://www.open-std.org/JTC1/SC22/WG14/www/docs/n2731.pdf">https://www.open-std.org/JTC1/SC22/WG14/www/docs/n2731.pdf</a>> <br/>
>  <br/>
> I thought union would have this issue in C++, but not in C; but it does.  <br/>
>   So this patch is actually correct. <br/>
<br/>
I knew it! ;) <br/>
  <br/>
> We have: <br/>
>  <br/>
> $ grepc nxt_unit_sptr_. <br/>
> ./src/nxt_unit_sptr.h:18: <br/>
> union nxt_unit_sptr_u { <br/>
>      uint8_t   base[1]; <br/>
>      uint32_t  offset; <br/>
> }; <br/>
>  <br/>
>  <br/>
> ./src/nxt_unit_typedefs.h:18: <br/>
> typedef union nxt_unit_sptr_u              nxt_unit_sptr_t; <br/>
>  <br/>
>  <br/>
> Since `uint8_t [1]` (or `uint8_t *`, to which it decays) is incompatible  <br/>
> with `uint32_t`, it is UB, and storing it in a temporary fixes that. <br/>
<br/>
Thanks for looking into this. <br/>
  <br/>
> So, for the patch: <br/>
>  <br/>
> Reviewed-by: Alejandro Colomar <<a href="mailto:alx.manpages@gmail.com">alx.manpages@gmail.com</a>> <br/>
<br/>
Well, I'm glad I wasn't completely wrong! I'll maybe tweak the commit <br/>
message and quote the spec... <br/>
  <br/>
> BTW, updated Stackoverflow: <<a href="https://stackoverflow.com/a/72652427/6872717">https://stackoverflow.com/a/72652427/6872717</a>> <br/>
<br/>
And the above paragraph is in at least the C99 spec. <br/>
<br/>
Cheers, <br/>
Andrew <br/>