On 2014-12-22 18:17:56 +0100, Tomas Vondra wrote:
> On 22.12.2014 17:47, Alvaro Herrera wrote:
> > Tomas Vondra wrote:
> >> On 22.12.2014 07:36, Tatsuo Ishii wrote:
> >>> On 22.12.2014 00:28, Tomas Vondra wrote:
> >
> >>>> (8) Also, I think it's not necessary to define function prototypes for
> >>>> executeStatement2 and is_table_exists. It certainly is not
> >>>> consistent with the other functions defined in pgbench.c (e.g.
> >>>> there's no prototype for executeStatement). Just delete the two
> >>>> prototypes and move is_table_exists before executeStatement2.
> >>>
> >>> I think not having static function prototypes is not a good
> >>> custom. See other source code in PostgreSQL.
> >>
> >> Yes, but apparently pgbench.c does not do that. It's strange to have
> >> prototypes for just two of many functions in the file.
> >
> > Whenever a function is defined before its first use, a prototype is
> > not mandatory, so we tend to omit them, but I'm pretty sure there are
> > cases where we add them anyway. I my opinion, rearranging code so
> > that called functions appear first just to avoid the prototype is not
> > a very good way to organize things, though. I haven't looked at this
> > patch so I don't know whether this is what's being done here.
>
> I'm not objecting to prototypes in general, but I believe the principle
> is to respect how the existing code is written. There are almost no
> other prototypes in pgbench.c - e.g. there are no prototypes for
> executeStatement(), init() etc. so adding the prototypes in this patch
> seems inconsistent. But maybe that's nitpicking.
I don't see a point in avoiding prototypes if the author thinks it makes
his patch clearer. And it's not like pgbench is the pinnacle of beauty
that would be greatly disturbed by any changes to its form.
Greetings,
Andres Freund
-- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services
