Re: log_heap_visible(): remove unused parameter and update comment

Hi,

On 9/30/22 1:32 PM, Bharath Rupireddy wrote:
> On Fri, Sep 30, 2022 at 1:07 PM Drouvot, Bertrand
> <bertranddrouvot(dot)pg(at)gmail(dot)com> wrote:
>>
>> Hi,
>>
>> While resuming the work on [1] I noticed that:
>>
>> - there is an unused parameter in log_heap_visible()
>> - the comment associated to the function is not in "sync" with the
>> current implementation (referring a "block" that is not involved anymore)
>>
>> Attached a tiny patch as an attempt to address the above remarks.
>>
>> [1]: https://commitfest.postgresql.org/39/3740/
>
> It looks like that parameter was originally introduced and used in PG
> 9.4 where xl_heap_visible structure was having RelFileNode, which was
> later removed in PG 9.5, since then the RelFileNode rnode parameter is
> left out. This parameter got renamed to RelFileLocator rlocator by the
> commit b0a55e43299c4ea2a9a8c757f9c26352407d0ccc in HEAD.
>
> The attached patch LGTM.

Thanks for looking at it!

>
> We recently committed another patch to remove an unused function
> parameter - 65b158ae4e892c2da7a5e31e2d2645e5e79a0bfd.
>
> It makes me think that why can't we remove the unused function
> parameters once and for all, say with a compiler flag such as
> -Wunused-parameter [1]? We might have to be careful in removing
> certain parameters which are not being used right now, but might be
> used in the near future though.
>
> [1] https://man7.org/linux/man-pages/man1/gcc.1.html
>
> -Wunused-parameter
> Warn whenever a function parameter is unused aside from its
> declaration.
>
> To suppress this warning use the "unused" attribute.
>

That's right. I have the feeling this will be somehow time consuming and
I'm not sure the added value is worth it (as compare to fix them when
"accidentally" cross their paths).

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com