Re: [PATCH 1/5] xdiff: support external hunks via xpparam_t

>> > +      * Clear changed[] arrays.  xdl_prepare_env() may have dirtied
>> > +      * them via xdl_cleanup_records().  The allocation is nrec + 2
>> > +      * elements; changed points one past the start (see xprepare.c).
>> > +      */
>> > +     memset(xe->xdf1.changed - 1, 0,
>> > +            (xe->xdf1.nrec + 2) * sizeof(bool));
>> > +     memset(xe->xdf2.changed - 1, 0,
>> > +            (xe->xdf2.nrec + 2) * sizeof(bool));
>>
>> This, especially the starting offset of -1, looks horrible.  The
>> internal layout of xdfenv_t might happen to match the way the above
>> code expects, which is how xdl_prepare_ctx() may have give you, but
>> it somehow feels brittle.  I guess the assumption that changed[]
>> does not point at the beginning of the allocated area (e.g., it is a
>> no-no to free(xe->xdf1.changed) or realloc() it) is so pervasive that
>> it cannot be helped.  Sigh.
>>
>
> Agreed it is ugly. I wanted to make sure the entire changed[] including
> sentinels were clear as a defensive measure for downstream callers
> (xdl_change_compact). I agree this results in something that is ugly
> and brittle, but in the end I thought it was superior to relying on the
> fact that upstream zeroes the entire changed[] array. Maybe if the
> comment was more explicit about why this is happening it would be
> helpful?

>     /*
>      * Clear changed[] arrays including sentinels.
>      * xdl_prepare_env() may have dirtied them via
>      * xdl_cleanup_records(), and xdl_change_compact() reads
>      * the sentinel at changed[-1] during backward scans.
>      */