On Mon, Feb 15, 2016 at 11:32:25PM -0500, Eric Sunshine wrote: > On Mon, Feb 15, 2016 at 11:23 PM, Jeff King <p...@peff.net> 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 <p...@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.
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
