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

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

On Mon, Apr 27, 2020 at 6:34 AM Torsten Bögershausen <tboegi@web.de> wrote:

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?

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

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

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

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

On 2020-04-27 at 13:13:12, Carlo Marcelo Arenas Belón wrote:

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.