Re: [PATCH 1/2] fsck_commit_buffer(): do not special case the last validation

[Jeff King <peff@peff.net>]

On Thu, Apr 14, 2016 at 11:07:08AM -0700, Junio C Hamano wrote:

> The pattern taken by all the validations in this function is:
> 
> 	if (notice a violation exists) {
> 		err = report(... VIOLATION_KIND ...);
> 		if (err)
> 			return err;
> 	}
> 
> where report() returns zero if specified kind of violation is set to
> be ignored, and otherwise shows an error message and returns non-zero.
> 
> The last validation in the function immediately after the function
> returns 0 to declare "all good" can cheat and directly return the
> return value from report(), and the current code does so, i.e.
> 
> 	if (notice a violation exists)
> 		return report(... VIOLATION_KIND ...);
> 	return 0;
> 
> But that is a selfish code that declares it is the ultimate and
> final form of the function, never to be enhanced later.  To allow
> and invite future enhancements, make the last test follow the same
> pattern.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  fsck.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)

Patch looks good, and nicely explained.

-Peff