Re: [PATCH 3/5] streaming.c: remove {open,close,read}_method_decl() macros

From: Jeff King <peff@peff.net>
Date: May 5, 2021, 13:44 UTC
Message-ID: <YJKhKdWlzVjHTKjl@coredump.intra.peff.net>
In-Reply-To: <patch-3.5-06961ee52bb-20210505T122816Z-avarab@gmail.com>

On Wed, May 05, 2021 at 02:33:30PM +0200, Ævar Arnfjörð Bjarmason wrote:

Show 10 quoted lines

> Remove the {open,close,read}_method_decl() macros added in
> 46bf043807c (streaming: a new API to read from the object store,
> 2011-05-11) in favor of inlining the definition of the arguments of
> these functions.
> 
> Since we'll end up using them via the "{open,close,read}_istream_fn"
> types we don't gain anything in the way of compiler checking by using
> these macros, and as of preceding commits we no longer need to declare
> these argument lists twice. So declaring them at a distance just
> serves to make the code less readable.

Heh. I have a very similar patch pending. In addition to readability, my reasons there are:

  - you can't find the functions with ctags, etc, when they're hidden
    behind macros

  - you can't annotate the function interfaces to avoid
    -Wunused-parameter warnings. :)

So I am very much in favor of this (and patch 1 is nice here, too, because it skips an extra time you'd have to repeat the interface in the forward declaration).

-Peff