srdja / Collections-C

A library of generic data structures for the C language.
http://srdja.github.io/Collections-C
GNU Lesser General Public License v3.0
2.8k stars 328 forks source link

When the length of the array is 0, the function `array_reverse` is wrong. #127

Closed ZirconXi closed 4 years ago

ZirconXi commented 4 years ago

When the size of the array is 0, the variable j will become very large.

When called, a segmentation fault will be triggered.

Code

ZirconXi commented 4 years ago
/* My `array_reverse ` */
void array_reverse(Array *array) {
    size_t size = array->size;

    if (size != 0) {
        for (size_t i = 0, j = size - 1; i < j; i++, j--) {
            void *temp = array->buffer[i];
            array->buffer[i] = array->buffer[j];
            array->buffer[j] = temp;
        }
    }
}
stefanct commented 4 years ago

I can confirm: ar->size - 1 will underflow because the calculation is done as size_t which is unsigned. array_remove_last() underflows too but passes the value on to array_remove_at() where it is checked before use (I don't see a good reason to change anything in that case). There is one place I could not easily verify and that's in array_filter_mut(). Everything else looks clear to me. I'll push a fix similar to above to my existing patch set (#129) shortly.

stefanct commented 4 years ago

OP has also posted the actual fix as a PR in #128 (that also limits the scope of the loop variables which I appreciate but left out because it's unrelated).

ZirconXi commented 4 years ago

Solved.