Re: [PATCH v3 3/4] fetch-pack: expose fsckObjects configuration logic

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Patrick Steinhardt <ps@pks.im> writes:
> >
> >>> +int fetch_pack_fsck_objects(void)
> >>> +{
> >>> +	fetch_pack_setup();
> >>> +
> >>> +	return fetch_fsck_objects >= 0
> >>> +	       ? fetch_fsck_objects
> >>> +	       : transfer_fsck_objects >= 0
> >>> +	       ? transfer_fsck_objects
> >>> +	       : 0;
> >>> +}
> >>
> >> ... can we maybe rewrite it to something more customary here? The
> >> following is way easier to read, at least for me.
> >>
> >> 	int fetch_pack_fsck_objects(void)
> >> 	{
> >> 		fetch_pack_setup();
> >> 		if (fetch_fsck_objects >= 0 ||
> >> 		    transfer_fsck_objects >= 0)
> >> 			return 1;
> >> 		return 0;
> >> 	}
> >
> > But do they mean the same thing?  In a repository where
> >
> >     [fetch] fsckobjects = no
> >
> > is set, no matter what transfer.fsckobjects says (or left unspecified),
> > we want to return "no, we are not doing fsck".
> 
> The original before it was made into a helper function was written
> as a cascade of ?: operators, because it had to be a single
> expression.  As the body of a helper function, we now can sprinkle
> multiple return statements in it.  I think the way that is easiest
> to understand is
> 
> 	/* the most specific, if specified */
> 	if (fetch_fsck_objects >= 0)
> 		return fetch_fsck_objects;
> 	/* the less specific, catch-all for both directions */
> 	if (transfer_fsck_objects >= 0)
>         	return transfer_fsck_objects;
> 	/* the fallback hardcoded default */
> 	return 0;
> 
> without the /* comments */.