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

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

 



On Mon, Feb 15, 2016 at 11:32:25PM -0500, Eric Sunshine wrote:

> On Mon, Feb 15, 2016 at 11:23 PM, Jeff King <peff@xxxxxxxx> wrote:
> > 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@xxxxxxxx> 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.

Yeah, I'm quite sure that one is safe (we set column_capacity to a fixed
integer immediately beforehand). And many of the "+" ones are likely
safe, too.  If "n" is close to wrapping, then allocating "n" structs
will probably fail beforehand (though not always, if you have a ton of
RAM and "n" is a signed int).

But part of the point of this series is that we shouldn't have to wonder
if things are safe. They should just be obviously so, and we should err
on the side of caution. So I think it probably _is_ worth sprinkling
st_add() calls in those places. I'll take a look for the re-roll.

-Peff
--
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]