Re: [PATCH] Fix ouside scope t_ctid (ItemPointerData)

From: Ranier Vilela <ranier(dot)vf(at)gmail(dot)com>
To: Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: [PATCH] Fix ouside scope t_ctid (ItemPointerData)
Date: 2020-05-14 22:55:17
Message-ID: CAEudQAq-SwSh6JPrgv6vjZDNxZwP_8orkOES3JoMfqpUzd28_A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Em qui., 14 de mai. de 2020 às 19:23, Mark Dilger <
mark(dot)dilger(at)enterprisedb(dot)com> escreveu:

>
>
> > On May 14, 2020, at 11:34 AM, Ranier Vilela <ranier(dot)vf(at)gmail(dot)com> wrote:
> >
> > htup->t_ctid = target_tid;
> > htup->t_ctid = newtid;
> > Both target_tid and newtid are local variable, whe loss scope, memory is
> garbage.
>
> Ok, thanks for the concrete example of what is bothering you.
>
> In htup_details, I see that struct HeapTupleHeaderData has a field named
> t_ctid of type struct ItemPointerData. I also see in heapam that
> target_tid is of type ItemPointerData. The
>
> htup->t_ctid = target_tid
>
> copies the contents of target_tid. By the time target_tid goes out of
> scope, the contents are already copied. I would share your concern if
> t_ctid were of type ItemPointer (aka ItemPointerData *) and the code said
>
> htup->t_ctid = &target_tid
>
> but it doesn't say that, so I don't see the issue.
>
Even if the patch simplifies and uses the API to make the assignments.
Really, the central problem does not exist, my fault.
Perhaps because it has never made such use, structure assignment.
And I failed to do research on the subject before.
Sorry.

>
> Also in heapam, I see that newtid is likewise of type ItemPointerData, so
> the same logic applies. By the time newtid goes out of scope, its contents
> have already been copied into t_ctid, so there is no problem.
>
> But maybe you know all that and are just complaining that the name
> "ItemPointerData" sounds like a pointer rather than a struct? I'm still
> unclear whether you believe this is a bug, or whether you just don't like
> the naming that is used.
>
My concerns were about whether attribution really would copy the
structure's content and not just its address.
The name makes it difficult, but that was not the point.

The tool warned about uninitialized variable, which I mistakenly deduced
for loss of scope.

Thank you very much for the clarifications and your time.
We never stopped learning, and using structure assignment was a new
learning experience.

regards
Ranier Vilela

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Ranier Vilela 2020-05-14 22:59:23 Re: [PATCH] Fix ouside scope t_ctid (ItemPointerData)
Previous Message Mark Dilger 2020-05-14 22:49:39 Re: [PATCH] Fix ouside scope t_ctid (ItemPointerData)