Re: Avoid unecessary MemSet call (src/backend/utils/cache/relcache.c)

Em ter., 17 de mai. de 2022 às 10:33, Ranier Vilela <ranier(dot)vf(at)gmail(dot)com>
escreveu:

> Em seg., 16 de mai. de 2022 às 20:26, David Rowley <dgrowleyml(at)gmail(dot)com>
> escreveu:
>
>> On Sun, 15 May 2022 at 09:47, Ranier Vilela <ranier(dot)vf(at)gmail(dot)com> wrote:
>> > At function load_relcache_init_file, there is an unnecessary function
>> call,
>> > to initialize pgstat_info pointer to NULL.
>> >
>> > MemSet(&rel->pgstat_info, 0, sizeof(rel->pgstat_info));
>>
>> What seems to have happened here is the field was changed to become a
>> pointer in 77947c51c. It's not incorrect to use MemSet() to zero out
>> the pointer field. What it does probably do is confuse the casual
>> reader into thinking the field is a struct rather than a pointer to
>> one. It's probably worth making that consistent with the other
>> fields so nobody gets confused.
>>
>> Can you add a CF entry for PG16 for this so we come back to it after we
>> branch?
>>
> Of course.
> I will add it.
>
Created https://commitfest.postgresql.org/38/3640/
However, I would like to add more.
I found, I believe, a serious problem of incorrect usage of the memset api.
Historically, people have relied on using memset or MemSet, using the
variable name as an argument for the sizeof.
While it works correctly, for arrays, when it comes to pointers to
structures, things go awry.

#include <stdio.h>

struct test_t
{
double b;
int a;
char c;
};

typedef struct test_t Test;

int main()
{
Test * my_test;

printf("Sizeof pointer=%u\n", sizeof(my_test));
printf("Sizeof struct=%u\n", sizeof(Test));
}

Output:
Sizeof pointer=8
Sizeof struct=16

So throughout the code there are these misuses.

So, taking advantage of this CF I'm going to add one more big patch, with
suggestions to fix the calls.
This pass vcregress check.

regards,
Ranier Vilela