Re: nitpick about poor style in MergeAttributes

Michael Paquier <michael(at)paquier(dot)xyz> writes:
> On Wed, May 22, 2019 at 06:20:01PM -0700, Mark Dilger wrote:
>> What to do about this is harder to say. In the following
>> patch, I'm just doing what I think is standard for callers
>> of list_delete_cell, and assigning the return value back
>> to the list (similar to how a call to repalloc should do).
>> But since there is an implicit assumption that the list
>> is never emptied by this operation, perhaps checking
>> against NIL and elog'ing makes more sense?

> Yes, I agree that this is a bit fuzzy, and this code is new as of
> 705d433. As you say, I agree that making sure that the return value
> of list_delete_cell is not NIL is a sensible choice.

Are we sure that's not just a newly-introduced bug, ie it has not
been tested in cases where the tlist could become empty? My first
thought would be to assign the list pointer value back as per usual
coding convention, not to double down on the assumption that this
was well-considered code.

regards, tom lane