Hi,

On 9/30/22 1:32 PM, Bharath Rupireddy wrote:
On Fri, Sep 30, 2022 at 1:07 PM Drouvot, Bertrand
<bertranddrouvot...@gmail.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


Reply via email to