Re: [PATCH v2 01/11] builtin/pack-objects.c: change check_pbase_path() to use ALLOC_GROW()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Feb 28, 2014 at 9:20 PM, Michael Haggerty <mhagger@xxxxxxxxxxxx> wrote:
> Duy,
>
> The example in Documentation/technical/api-allocation-growing.txt does
> it the same way as Dmitry:
>
>     ALLOC_GROW(item, nr + 1, alloc);
>     item[nr++] = value you like;
>
> The alternative,
>
>     nr++;
>     ALLOC_GROW(item, nr, alloc);
>     item[nr] = value you like;
>
> is an extra line, which is at least a small argument for the variant
> shown in the docs.  (Since ALLOC_GROW is a macro, it is not OK to use
> "++nr" as its second argument.)  Personally, I also prefer the shorter
> version.  The line
>
>     item[nr++] = value
>
> is an easy-to-recognize idiom, and
>
>     ALLOC_GROW(item, nr + 1, alloc);
>
> somehow makes it more transparent by how much more space will be needed.
>
> So my vote is that the patches are OK the way Dmitry wrote them (mind, I
> have only read through 05/11 so far).

I'm not saying all patches should do

nr++;
ALLOC_GROW(item, nr, alloc);

only those that do

if (..) realloc...;
nr++;
....

should be reordered. Those changes that do item[nr++] = yyy should be
kept. Anyway it's just an observation, not something that should block
these patches.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]