Re: [PATCH v4 1/7] Add delta-islands.{c,h}

On Mon, Aug 13, 2018 at 01:17:18PM +0100, Ramsay Jones wrote:

quoted

quoted

quoted

+struct island_bitmap {
+     uint32_t refcount;
+     uint32_t bits[];

Use FLEX_ARRAY here? We are slowly moving toward requiring
certain C99 features, but I can't remember a flex array
weather-balloon patch.

This was already discussed by Junio and Peff there:

https://public-inbox.org/git/20180727130229.GB18599@sigill.intra.peff.net/

That is a fine discussion, without a firm conclusion, but I don't
think you can simply do nothing here:

  $ cat -n junk.c
       1	#include <stdint.h>
       2	
       3	struct island_bitmap {
       4		uint32_t refcount;
       5		uint32_t bits[];
       6	};
       7	
  $ gcc --std=c89 --pedantic -c junk.c
  junk.c:5:11: warning: ISO C90 does not support flexible array members [-Wpedantic]
    uint32_t bits[];
             ^~~~
  $ gcc --std=c99 --pedantic -c junk.c

Right, whether we use the FLEX_ALLOC macros or not, this needs to be
declared with FLEX_ARRAY, not an empty "[]".

I'm fine either way on using the FLEX_ALLOC macros.

quoted

quoted

... Ah, OK, trg_ => target.

I am ok to replace "trg" with "target" (or maybe "dst"? or something
else) and "src" with "source" if you think it would make things
clearer.

If it had been dst_ (or target), I would not have had a 'huh?'
moment, but it is not all that important.

FWIW, these are all inherited from try_delta(), etc, in the existing
code. I'm fine with using another term, but we should probably do it
universally. And if we do, probably "base" is a better name than "src",
since the direction depends on which part of the relationship you are
considering. I'm not sure what that makes "dst".

-Peff