On Mon, Aug 13, 2018 at 01:17:18PM +0100, Ramsay Jones wrote:
> >>> +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@xxxxxxxxxxxxxxxxxxxxx/
> >
>
> 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.
> >> ... 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
