Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
>> void diff_free_filespec_data(struct diff_filespec *s)
>> {
>> + if (!s)
>> + return;
>> +
>> diff_free_filespec_blob(s);
>> FREE_AND_NULL(s->cnt_data);
>
> We can write this in a more canonical way without wasting all that many
> lines:
>
> if (s) {
> diff_free_filespec_blob(s);
> FREE_AND_NULL(s->cnt_data);
> }
On only this part. Please do not use this one, as what was posted
is better.
By making it clear that we do not do anything when given NULL
upfront, it lets readers concentrate on the main part of the
function---"now we are done with NULL, what happens on a real case?"
And when readers are doing so, one extra indentation level is just
an extra noise that does not help.
In this particular case, it is important more from coding discipline
than anything else---for a small enough function like this one whose
main part fits on less than fourth of a screen, extra indentation
does not matter, but all code tends to grow and it starts to matter
if/when we need to clean more things in the function.
Everything else said in the review was excellent and very helpful.
Thanks.
