Phillip Wood <phillip.wood123@xxxxxxxxx> writes:
>> +static int cmd_log_deinit(int ret, struct rev_info *rev)
>> +{
>> + release_revisions(rev);
>> + return ret;
>> +}
>
>
>> /*
>> * This gives a rough estimate for how many commits we
>> * will print out in the list.
>> @@ -558,7 +564,7 @@ int cmd_whatchanged(int argc, const char **argv, const char *prefix)
>> cmd_log_init(argc, argv, prefix, &rev, &opt);
>> if (!rev.diffopt.output_format)
>> rev.diffopt.output_format = DIFF_FORMAT_RAW;
>> - return cmd_log_walk(&rev);
>> + return cmd_log_deinit(cmd_log_walk(&rev), &rev);
>
> This is a rather unusual pattern, at first I wondered if there were
> going to be more added to the body of cmd_log_deinit() in later
> commits but there isn't so why not just call release_revisions() here
> to be consistent with the other release_revisions() call that are
> added in other patches?
It is being cute and clever by not requiring a temporary variable
ret, where you would normally say
int ret = 0; /* assume success */
... a lot of code ...
ret = cmd_log_walk(&rev);
release_revisions(&rev);
return ret;
I agree that this looks confusing; if this pattern can become
majority locally in the file, I guess it would be OK---at that point
we can claim that it is the (new) usual pattern ;-).
