Jeff King <peff@xxxxxxxx> writes: > Yep, but it's not much different than the hundreds of other function > interfaces we have where sometimes you can pass NULL and sometimes not. > > So anyway. What do we want to do here? The fix I have? Something more > elaborate and reusable? Or perhaps just switch it to: My preference was to take the patch as-is, as it was clear enough, before seeing this one ... > diff --git a/xdiff-interface.c b/xdiff-interface.c > index 3cd2ac2855..4d20069302 100644 > --- a/xdiff-interface.c > +++ b/xdiff-interface.c > @@ -84,8 +84,8 @@ static void trim_common_tail(mmfile_t *a, mmfile_t *b) > { > const int blk = 1024; > long trimmed = 0, recovered = 0; > - char *ap = a->ptr ? a->ptr + a->size : a->ptr; > - char *bp = b->ptr ? b->ptr + b->size : b->ptr; > + char *ap = a->size ? a->ptr + a->size : a->ptr; > + char *bp = b->size ? b->ptr + b->size : b->ptr; > long smaller = (a->size < b->size) ? a->size : b->size; > > while (blk + trimmed <= smaller && !memcmp(ap - blk, bp - blk, blk)) { > > By checking "size" instead of "ptr", then we know that the addition is a > noop. And we'd continue to catch a NULL pointer mixed with a non-zero > length (as a segfault). And a non-NULL pointer with a zero length does > the right thing. which makes quite a lot of sense.