<div dir="ltr">Ugh, sorry about that, I forgot to run the tests after the last patch fixing darray_remove(). I missed a backslash in that patch (darray: Fix bug in the darray_remove() macro). I'll resend it with the missing parentheses around the macro params. Thanks.</div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Aug 28, 2017 at 4:48 AM, David Gibson <span dir="ltr"><<a href="mailto:david@gibson.dropbear.id.au" target="_blank">david@gibson.dropbear.id.au</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Sun, Aug 27, 2017 at 11:26:24PM +0200, Damien Grassart wrote:<br>
> The memmove() call should be using the index argument to determine the<br>
> number of bytes to copy. To be consistent with the rest of the code,<br>
> we should also not evaluate the index parameter multiple<br>
> times. Calling this with rand() % arr.size would otherwise generally<br>
> segfault.<br>
><br>
> Finally, we want to avoid using "index" as an identifier so as to not<br>
> shadow index(3) in the C library.<br>
><br>
> Signed-off-by: Damien Grassart <<a href="mailto:damien@grassart.com">damien@grassart.com</a>><br>
<br>
</span>This breaks compile for me, though I can't quickly see why.<br>
<br>
$ make<br>
cc -g3 -ggdb -Wall -Wstrict-prototypes -Wold-style-definition -Wundef -Wmissing-prototypes -Wmissing-declarations -Wpointer-arith -Wwrite-strings -DCCAN_STR_DEBUG=1 -I.  -MMD -MP -MFccan/strgrp/strgrp.o.d -MTccan/strgrp/strgrp.o -c ccan/strgrp/strgrp.c -o ccan/strgrp/strgrp.o<br>
In file included from ccan/strgrp/strgrp.c:26:0:<br>
./ccan/darray/darray.h:235:2: error: expected identifier or ‘(’ before ‘if’<br>
  if (index_ < arr.size-1)    \<br>
  ^~<br>
./ccan/darray/darray.h:237:7: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘.’ token<br>
  (arr).size--;  \<br>
       ^<br>
./ccan/darray/darray.h:238:2: error: expected identifier or ‘(’ before ‘}’ token<br>
  } while(0)<br>
  ^<br>
./ccan/darray/darray.h:238:4: error: expected identifier or ‘(’ before ‘while’<br>
  } while(0)<br>
    ^~~~~<br>
<br>
I also noticed that both this and the earlier patches use:<br>
        size_t index_ = i;<br>
in the macro, without parentheses around the 'i' macro paramater.<br>
That's not the cause of the error above, but it's not good practice as<br>
a rule.<br>
<br>
So, I've backed out these darray patches for now.  Can you debug the<br>
compile problem above and resend the whole lot as a single series.<br>
<div class="HOEnZb"><div class="h5"><br>
> ---<br>
>  ccan/darray/darray.h | 11 ++++++-----<br>
>  1 file changed, 6 insertions(+), 5 deletions(-)<br>
><br>
> diff --git a/ccan/darray/darray.h b/ccan/darray/darray.h<br>
> index 82726c05..6787f14c 100644<br>
> --- a/ccan/darray/darray.h<br>
> +++ b/ccan/darray/darray.h<br>
> @@ -170,8 +170,8 @@ typedef darray(unsigned long)  darray_ulong;<br>
>               memmove((arr).item+1, (arr).item, ((arr).size-1)*sizeof(*(arr).<wbr>item)); \<br>
>               (arr).item[0] = (__VA_ARGS__); \<br>
>       } while(0)<br>
> -#define darray_insert(arr, index, ...) do { \<br>
> -             size_t index_ = index; \<br>
> +#define darray_insert(arr, i, ...) do { \<br>
> +             size_t index_ = i; \<br>
>               darray_resize(arr, (arr).size+1); \<br>
>               memmove((arr).item+index_+1, (arr).item+index_, ((arr).size-index_-1)*sizeof(*<wbr>(arr).item)); \<br>
>               (arr).item[index_] = (__VA_ARGS__); \<br>
> @@ -230,9 +230,10 @@ typedef darray(unsigned long)  darray_ulong;<br>
>  #define darray_pop(arr) ((arr).item[--(arr).size])<br>
>  #define darray_pop_check(arr) ((arr).size ? darray_pop(arr) : NULL)<br>
>  /* Warning, slow: Requires copying all elements after removed item. */<br>
> -#define darray_remove(arr, index) do { \<br>
> -     if (index < arr.size-1)    \<br>
> -             memmove(&(arr).item[index], &(arr).item[index+1], ((arr).size-1-i)*sizeof(*(arr)<wbr>.item)); \<br>
> +#define darray_remove(arr, i) do { \<br>
> +     size_t index_ = i;<br>
> +     if (index_ < arr.size-1)    \<br>
> +             memmove(&(arr).item[index_], &(arr).item[index_+1], ((arr).size-1-index_)*sizeof(*<wbr>(arr).item)); \<br>
>       (arr).size--;  \<br>
>       } while(0)<br>
><br>
<br>
--<br>
</div></div><div class="HOEnZb"><div class="h5">David Gibson                    | I'll have my music baroque, and my code<br>
david AT <a href="http://gibson.dropbear.id.au" rel="noreferrer" target="_blank">gibson.dropbear.id.au</a>  | minimalist, thank you.  NOT _the_ _other_<br>
                                | _way_ _around_!<br>
<a href="http://www.ozlabs.org/~dgibson" rel="noreferrer" target="_blank">http://www.ozlabs.org/~dgibson</a><br>
</div></div></blockquote></div><br></div>