Compilation errors in git.pu/refs/reftable-backend.c

Compilation errors in git.pu/refs/reftable-backend.c

[Torsten Bögershausen]

Folks, after the integration of the reftable branch:
Merge branch 'hn/reftable' into pu

My historic (?) compiler stumbles over this code here:
refs/reftable-backend.c
   for (int i = 0; i < transaction->nr; i++) {

So I think we could easily avoid the (for int i) construction.
The other observation could be, why we use an "int", when "nr" is
defined as a size_t ?

struct ref_transaction {
	struct ref_store *ref_store;
	struct ref_update **updates;
	size_t alloc;
	size_t nr;
	enum ref_transaction_state state;
	void *backend_data;
};

Thanks for working on Git

Re: Compilation errors in git.pu/refs/reftable-backend.c

[Han-Wen Nienhuys]

On Mon, Apr 27, 2020 at 6:34 AM Torsten Bögershausen <tboegi@web.de> wrote:
>
> Folks, after the integration of the reftable branch:
> Merge branch 'hn/reftable' into pu

thanks, I'll fix that.

I don't understand why Git doesn't enforce this, though.  Couldn't
-Wdeclaration-after-statement be added to the Makefile if the compiler
supported it?

-- 
Han-Wen Nienhuys - Google Munich
I work 80%. Don't expect answers from me on Fridays.
--

Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich

Registergericht und -nummer: Hamburg, HRB 86891

Sitz der Gesellschaft: Hamburg

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado

Re: Compilation errors in git.pu/refs/reftable-backend.c

[Jeff King]

On Mon, Apr 27, 2020 at 02:05:12PM +0200, Han-Wen Nienhuys wrote:

> On Mon, Apr 27, 2020 at 6:34 AM Torsten Bögershausen <tboegi@web.de> wrote:
> >
> > Folks, after the integration of the reftable branch:
> > Merge branch 'hn/reftable' into pu
> 
> thanks, I'll fix that.
> 
> I don't understand why Git doesn't enforce this, though.  Couldn't
> -Wdeclaration-after-statement be added to the Makefile if the compiler
> supported it?

Try building with "make DEVELOPER=1" (or putting DEVELOPER=1 in your
config.mak), which will turn on warnings and enable -Werror. See
config.mak.dev for the complete list.

-Peff

Re: Compilation errors in git.pu/refs/reftable-backend.c

[Carlo Marcelo Arenas Belón]

On Mon, Apr 27, 2020 at 02:05:12PM +0200, Han-Wen Nienhuys wrote:
> 
> I don't understand why Git doesn't enforce this, though.  Couldn't
> -Wdeclaration-after-statement be added to the Makefile if the compiler
> supported it?

not sure if that flag will catch it, but from what I recall, that
specific feature was only available in C99 while git's codebase
targets C89 (need to build with --std=gnu89 as some GNU extensions
are already needed).

see the "For C programs" section in Documentation/CodingGuidelines for
details.

Carlo

Re: Compilation errors in git.pu/refs/reftable-backend.c

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1081 bytes --]

On 2020-04-27 at 13:13:12, Carlo Marcelo Arenas Belón wrote:
> On Mon, Apr 27, 2020 at 02:05:12PM +0200, Han-Wen Nienhuys wrote:
> > 
> > I don't understand why Git doesn't enforce this, though.  Couldn't
> > -Wdeclaration-after-statement be added to the Makefile if the compiler
> > supported it?
> 
> not sure if that flag will catch it, but from what I recall, that
> specific feature was only available in C99 while git's codebase
> targets C89 (need to build with --std=gnu89 as some GNU extensions
> are already needed).

I think we had chosen to preserve this behavior because MSVC didn't
support C99.  We may wish to change our approach now that MSVC supports
a reasonable subset of it and GCC and Clang are available for almost
every system out there.

I don't think it's unreasonable to expect folks to use a compiler
supporting a standard that is now more than two decades old.

Of course, that doesn't affect the present series, but it is a thing we
should consider.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]