[Mesa-dev] [PATCH] st/mesa: init hash keys with memset(), not designated initializers

[Ilia Mirkin]

I believe the distinction here is between initializing all the fields
and using them individually (thus leaving the padding uninitialized),
vs using the fields to create a structure to hash as a sequence of
bytes. For the latter, you really need all the memory initialized, not
just the "valid" parts of the structure. In at least my mind, it's
fairly well-established that compilers don't always initialize all of
a structure's underlying bytes, but I also don't have a specific
instance of that situation I can point to.

For most usage, foo = {0} is fine since you're not hashing the bytes
but rather accessing the fields directly. But for hashing, you really
want all the bits initialized.

Cheers,

  -ilia

On Tue, Mar 12, 2019 at 12:31 PM Eric Anholt <eric at anholt.net> wrote:
>
> Brian Paul <brianp at vmware.com> writes:
>
> > On 03/11/2019 04:47 PM, Eric Anholt wrote:
> >> Brian Paul <brianp at vmware.com> writes:
> >>
> >>> Since the compiler may not zero-out padding in the object.
> >>> Add a couple comments about this to prevent misunderstandings in
> >>> the future.
> >>>
> >>> Fixes: 67d96816ff5 ("st/mesa: move, clean-up shader variant key decls/inits")
> >>> ---
> >>>   src/mesa/state_tracker/st_atom_shader.c |  9 +++++++--
> >>>   src/mesa/state_tracker/st_program.c     | 13 ++++++++++---
> >>>   2 files changed, 17 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/src/mesa/state_tracker/st_atom_shader.c b/src/mesa/state_tracker/st_atom_shader.c
> >>> index ac7a1a5..a4475e2 100644
> >>> --- a/src/mesa/state_tracker/st_atom_shader.c
> >>> +++ b/src/mesa/state_tracker/st_atom_shader.c
> >>> @@ -112,7 +112,10 @@ st_update_fp( struct st_context *st )
> >>>          !stfp->variants->key.bitmap) {
> >>>         shader = stfp->variants->driver_shader;
> >>>      } else {
> >>> -      struct st_fp_variant_key key = {0};
> >>> +      struct st_fp_variant_key key;
> >>> +
> >>> +      /* use memset, not an initializer to be sure all memory is zeroed */
> >>> +      memset(&key, 0, sizeof(key));
> >>
> >> Wait, what?  We rely on this form of initialization all over, what's
> >> changed?
> >
> > The question is do all compilers, when presented with
> >
> > struct st_fp_variant_key key = {0};
>
> You seem to be saying that not all compilers do, but throughout the
> compiler we're relying on all compilers doing so.  Can we get some more
> information about what compiler you're fixing here?
> -----BEGIN PGP SIGNATURE-----
>
> iQIzBAEBCgAdFiEE/JuuFDWp9/ZkuCBXtdYpNtH8nugFAlyH3tsACgkQtdYpNtH8
> nujFFxAAkzNBlIJyvZhaUuYZ3B7ugbHxCnyfsi6J9AmAKcXyyJ0oIAi1sXkLwflB
> e5WbyksenHiE+NnihdQZICV2fzb0G8WObn5HiBSxNGKqEwATQY3ND3gkzvH2npKT
> EGY0AZCPX/yJPA3yci/MRTLLPzLgSYTUstfEsAmTlRlqmOlWDenV6BV7OpOoTz/F
> bVdrk6QXufLcQ+ZtQS2OFKEKlXdaUaE5ruwanasc8qzXmJ0TMonvRZp4ZrNUNc4j
> p1sUQnAU0SxwuSqIVS6iVmPEH/N2e3u4XdNsZ+45KxFkAnV0wnpIKAbREa1tzOdP
> d2nhZ70BUNnHJE5q11I070mGWkburTKk5hbLvFGS59np/2nLjZQLcb3hYNyCPDP2
> sdRwqzPtwbdNLAbHpiQ3x0OuVJ6P5fuxGUjuB/AQfl1ixV03nB77AXogqhlQ/cK4
> WceUn9AXmhBb5tIdhfKK1uzuplmCSpXaKxdPubvgfI4Rey5KMvcZwBFNqztcfe24
> hc3wPFyfmayNAuFHkC+0jd4po20ibJkF1F0kXjoyl7dKMG6IFX4fICJKPsafIgNV
> cex67m6VAF9ZalBaSgnXZpcmHXSFbVSHYvES/WxRncefgAzXG/BMuywsLZ+US99A
> LfrnLaq0eRDAdpcSfITQmDgpjEvWFThxCDXXBCI5a+pQpYBB6lk=
> =4Bvh
> -----END PGP SIGNATURE-----_______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev