use better overflow check #2768
Conversation
|
if we're really committing to this catch overflow idea, don't we want it backing |
nevermind, i think it is by virtue of being in |
| if( size < (size_t) cols || size < (size_t) rows) | ||
| // check if multiplication would overflow | ||
| // calloc will deal with overflow due to sizeof(nccell) | ||
| if( ! rows || (cols > SIZE_MAX / rows)) | ||
| return 0; |
There was a problem hiding this comment.
we'll want a diagnostic. see other uses of LOG_
There was a problem hiding this comment.
It's not easy to see from the way github presents these changes, but the return 0 will leas to:
size_t fbsize = ncplane_sizeof_cellarray(p->leny, p->lenx);
if(!fbsize || (p->fb = calloc(sizeof(struct nccell),fbsize)) == NULL){
logerror("error allocating cellmatrix (r=%u, c=%u)",
p->leny, p->lenx);
free(p);
return NULL;
}
so there is a logerror there and the huge or zero values will provide ample clues as what went wrong. (IMO)
|
|
||
| fbsize = sizeof( struct nccell) * size; | ||
| if( fbsize <= size) | ||
| return 0; |
There was a problem hiding this comment.
Thats deleted code though 😉
|
Just wondering, if I need to add some more comments here, or if everything is clear. |
This is a better mergable #2767, but my fork got out of sync and there were superflous commits in there. Sorry for the inconvenience. Here's my original comment.
Adding to this: The old code caught overflow with large negative numbers, but this one will catch any kind of overflow. Also the code is easier to understand IMO.