Em qua., 18 de mai. de 2022 às 05:54, Alvaro Herrera < alvhe...@alvh.no-ip.org> escreveu:
> This one caught my attention: > > diff --git a/contrib/pgcrypto/crypt-blowfish.c > b/contrib/pgcrypto/crypt-blowfish.c > index a663852ccf..63fcef562d 100644 > --- a/contrib/pgcrypto/crypt-blowfish.c > +++ b/contrib/pgcrypto/crypt-blowfish.c > @@ -750,7 +750,7 @@ _crypt_blowfish_rn(const char *key, const char > *setting, > /* Overwrite the most obvious sensitive data we have on the stack. Note > * that this does not guarantee there's no sensitive data left on the > * stack and/or in registers; I'm not aware of portable code that does. */ > - px_memset(&data, 0, sizeof(data)); > + px_memset(&data, 0, sizeof(struct data)); > > return output; > } > > The curious thing here is that sizeof(data) is correct, because it > refers to a variable defined earlier in that function, whose type is an > anonymous struct declared there. But I don't know what "struct data" > refers to, precisely because that struct is unnamed. Am I misreading it? > No, you are right. This is definitely wrong. > > Also: > > diff --git a/contrib/pgstattuple/pgstatindex.c > b/contrib/pgstattuple/pgstatindex.c > index e1048e47ff..87be62f023 100644 > --- a/contrib/pgstattuple/pgstatindex.c > +++ b/contrib/pgstattuple/pgstatindex.c > @@ -601,7 +601,7 @@ pgstathashindex(PG_FUNCTION_ARGS) > errmsg("cannot access temporary indexes > of other sessions"))); > > /* Get the information we need from the metapage. */ > - memset(&stats, 0, sizeof(stats)); > + memset(&stats, 0, sizeof(HashIndexStat)); > metabuf = _hash_getbuf(rel, HASH_METAPAGE, HASH_READ, > LH_META_PAGE); > metap = HashPageGetMeta(BufferGetPage(metabuf)); > stats.version = metap->hashm_version; > > I think the working theory here is that the original line is correct > now, and it continues to be correct if somebody edits the function and > makes variable 'stats' be of a different type. But if you change the > sizeof() to use the type name, then there are two places that you need > to edit, and they are not necessarily close together; so it is correct > now and could become a bug in the future. I don't think we're fully > consistent about this, but I think you're proposing to change it in the > opposite direction that we'd prefer. > Yes. I think that only advantage using the name of structure is when you read the line of MemSet, you know what kind type is filled. > For the case where the variable is a pointer, the developer could write > 'sizeof(*variable)' instead of being forced to specify the type name, > for example (just a random one): > Could have used this style to make the patch. But the intention was to correct a possible misinterpretation, which in this case, showed that I was totally wrong. Sorry by the noise. regards, Ranier Vilela