Re: [PATCH 1/2] pkt-line: fix declaration of `set_packet_header()`

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: May 14, 2019, 12:57 UTC
Message-ID: <nycvar.QRO.7.76.6.1905141454510.44@tvgsbejvaqbjf.bet>
In-Reply-To: <xmqqo9469buw.fsf@gitster-ct.c.googlers.com>

Hi Junio,

On Tue, 14 May 2019, Junio C Hamano wrote:

Show 12 quoted lines

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > When this function was changed in a97d00799a19 (remote-curl: use
> > post_rpc() for protocol v2 also, 2019-02-21) from file-local to global,
> > the declaration was incorrectly missing the `const` qualifier.
>
> I do not quite get it.  Back when the function was file-scope
> static, it did not even have a separate declaration, and the
> definition the said commit added looks correct to me.

Right, I should have said "the new declaration" instead of "the declaration".

Show 15 quoted lines

> Having "const int size" parameter in the definition of a function
> does help the compilers and the developers by making sure any
> earlier reference to the parameter in the function would not modify
> it and cause later reference to obtain a different value.
>
> But the parameter treated as a constant without getting modified
> during the invocation of the function is an implementation detail of
> the function; there is no point exposing that implementation detail
> to its callers.  It does not even help the compilers handling the
> caller's compilation unit---the parameter is passed by value, so the
> caller knows that the callee would not modify it without "const"
> there.
>
> Does the language even allow flagging "const int in the definition,
> int in the declaration" as a warning-worthy discrepancy?

Apparently it does, as MS Visual C does issue a warning (and with `/Wall`, it fails).

In any case, I don't think that it makes sense to have a function declaration whose signature differs from the definition's.

Ciao, Dscho

> > -void set_packet_header(char *buf, int size);
> > +void set_packet_header(char *buf, const int size);
>