Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)
От | Alvaro Herrera |
---|---|
Тема | Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c) |
Дата | |
Msg-id | 202205180854.i53lyhxum32v@alvherre.pgsql обсуждение исходный текст |
Ответ на | Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c) (Ranier Vilela <ranier.vf@gmail.com>) |
Ответы |
Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)
(Ranier Vilela <ranier.vf@gmail.com>)
|
Список | pgsql-hackers |
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? 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. 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): diff --git a/contrib/bloom/blutils.c b/contrib/bloom/blutils.c index a434cf93ef..e92c03686f 100644 --- a/contrib/bloom/blutils.c +++ b/contrib/bloom/blutils.c @@ -438,7 +438,7 @@ BloomFillMetapage(Relation index, Page metaPage) */ BloomInitPage(metaPage, BLOOM_META); metadata = BloomPageGetMeta(metaPage); - memset(metadata, 0, sizeof(BloomMetaPageData)); + memset(metadata, 0, sizeof(*metadata)); metadata->magickNumber = BLOOM_MAGICK_NUMBER; metadata->opts = *opts; ((PageHeader) metaPage)->pd_lower += sizeof(BloomMetaPageData); -- Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
В списке pgsql-hackers по дате отправления:
Предыдущее
От: "wangw.fnst@fujitsu.com"Дата:
Сообщение: RE: Data is copied twice when specifying both child and parent table in publication