> On May 14, 2020, at 10:40 AM, Ranier Vilela <ranier...@gmail.com> wrote:
>
> Hi,
> ItemPointerData, on the contrary, from what the name says,
> it is not a pointer to a structure, but a structure in fact.
The project frequently uses the pattern
typedef struct FooData {
...
} FooData;
typedef FooData *Foo;
where, in this example, "Foo" = "ItemPointer".
So the "Data" part of "ItemPointerData" clues the reader into the fact that
ItemPointerData is a struct, not a pointer. Granted, the "Pointer" part of
that name may confuse some readers, but the struct itself does contain what is
essentially a 48-bit pointer, so that name is not nuts.
> When assigning the name of the structure variable to a pointer, it may even
> work,
> but, it is not the right thing to do and it becomes a nightmare,
> to discover that any other error they have is at cause.
Can you give a code example of the type of assigment you mean?
> So:
> 1. In some cases, there may be a misunderstanding in the use of
> ItemPointerData.
> 2. When using the variable name in an assignment, the variable's address is
> used.
> 3. While this works for a structure, it shouldn't be the right thing to do.
> 4. If we have a local variable, its scope is limited and when it loses its
> scope, memory is certainly garbage.
> 5. While this may be working for heapam.c, I believe it is being abused and
> should be compliant with
> the Postgres API and use the functions that were created for this.
>
> The patch is primarily intended to correct the use of ItemPointerData.
> But it is also changing the style, reducing the scope of some variables.
> If that was not acceptable, reduce the scope and someone has objections,
> I can change the patch, to focus only on the use of ItemPointerData.
> But as style changes are rare, if possible, it would be good to seize the
> opportunity.
I would like to see a version of the patch that only addresses your concerns
about ItemPointerData, not because other aspects of the patch are unacceptable
(I'm not ready to even contemplate that yet), but because I'm not sure what
your complaint is about. Can you restrict the patch to just address that one
issue?
—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
