Thread: rename labels in heapam.c?

Hi,

For the umpteenth time I was annoyed by the names of labels in
heapam.c. It's really not useful to see a 'goto l1;' etc.

How about renaming l1 to retry_delete_locked, l2 to retry_update_locked,
l3 to retry_lock_tuple_locked etc? Especially with the subsidiary
functions for updates and locking, it's not always clear from context
where the goto jumps to.

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:
> For the umpteenth time I was annoyed by the names of labels in
> heapam.c. It's really not useful to see a 'goto l1;' etc.

Yeah, those label names are uninformative as can be.

> How about renaming l1 to retry_delete_locked, l2 to retry_update_locked,
> l3 to retry_lock_tuple_locked etc? Especially with the subsidiary
> functions for updates and locking, it's not always clear from context
> where the goto jumps to.

Is it practical to get rid of the goto's altogether?  If not,
renaming would be an improvement.

            regards, tom lane

Hi,

On 2019-03-22 17:09:23 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > For the umpteenth time I was annoyed by the names of labels in
> > heapam.c. It's really not useful to see a 'goto l1;' etc.
> 
> Yeah, those label names are uninformative as can be.
> 
> > How about renaming l1 to retry_delete_locked, l2 to retry_update_locked,
> > l3 to retry_lock_tuple_locked etc? Especially with the subsidiary
> > functions for updates and locking, it's not always clear from context
> > where the goto jumps to.
> 
> Is it practical to get rid of the goto's altogether?  If not,
> renaming would be an improvement.

I don't think it'd be easy. We could probably split
heap_{insert,delete,update} into sub-functions and then have the
toplevel function just loop over invocations of those, but that seems
like a pretty significant refactoring of the code.  Since renaming the
labels isn't going to make that harder, I'm inclined to do that, rather
than wait for a refactoring that, while a good idea, isn't likely to
happen that soon.

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:
> On 2019-03-22 17:09:23 -0400, Tom Lane wrote:
>> Is it practical to get rid of the goto's altogether?  If not,
>> renaming would be an improvement.

> I don't think it'd be easy.

Fair enough.  I just wanted to be sure we considered getting rid of
the pig before we put lipstick on it.  I concur that it's not worth
major refactoring to do so.

            regards, tom lane