Re: [PATCH 05/18] convert trivial cases to ALLOC_ARRAY

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Feb 16, 2016, 04:32 UTC
Message-ID: <CAPig+cSn29QtcWKRAaKMDzObuXetEySrVtLogkzfZXKXjPeufQ@mail.gmail.com>
In-Reply-To: <20160216042346.GB27060@sigill.intra.peff.net>

On Mon, Feb 15, 2016 at 11:23 PM, Jeff King <peff@peff.net> wrote:

Show 12 quoted lines

> On Mon, Feb 15, 2016 at 11:22:12PM -0500, Eric Sunshine wrote:
>> On Mon, Feb 15, 2016 at 4:51 PM, Jeff King <peff@peff.net> wrote:
>> > -       path = xmalloc((n+1)*sizeof(char *));
>> > +       ALLOC_ARRAY(path, n+1);
>>
>> Elsewhere in this patch, you've reformatted "x+c" as "x + c"; perhaps
>> do so here, as well.
>
> Will do. I noticed while going over this before sending it out that it
> may also be technically possible for "n+1" to overflow here (and I think
> in a few other places in this patch). I don't know how paranoid we want
> to be.

Yes, I also noticed those and considered mentioning it. There was also some multiplication which might be of concern.

    ALLOC_ARRAY(graph->mapping, 2 * graph->column_capacity);

It would be easy enough to manually call st_add() and st_mult() for those cases, but I haven't examined them closely enough to determine how likely they would be to overflow, nor do I know if the resulting noisiness of code is desirable.