Re: [PATCH 00/28] clean-ups of static functions and returns

>From: Junio C Hamano <junkio@cox.net>
>To: David Rientjes <rientjes@google.com>
>CC: git@vger.kernel.org
>Subject: Re: [PATCH 00/28] clean-ups of static functions and returns
>Date: Mon, 14 Aug 2006 18:04:24 -0700
>
>David Rientjes <rientjes@google.com> writes:
>
> > On Mon, 14 Aug 2006, Junio C Hamano wrote:
> >
> >> Interesting.  Did you use some automated tool to spot them?
> >>
> >
> > No, these changes are from my own personal tree that optimizes 
>everything for
> > speed since I am working with terabytes of data.  I only submitted 
>changes that
> > I thought would be beneficial for your project as well.
> >
> >>  * Making stricter error checking in the future harder.  There
> >>    are three classes, but the lines between them are fuzzy.
> >>
> >>         [PATCH 04/28] builtin-diff.c cleanup
> >>...
> >>         [PATCH 23/28] makes peek_remote void
> >>
> >>    The callers of the first group check their return values, so
> >>    we could make error checking of these functions stricter in
> >>    the future without affecting the rest of the code.  The ones
> >>    that currently die() (or usage()) could be made into more
> >>    libified form to return error codes.
> >>
> >>    So I do not think it is worth doing these.
> >
> > I disagree.  Having static functions return ints that are the
> > same for _every_ code path and are checked against upon return
> > is never good style.  It implies that error checking is
> > already done and the return value is of importance.
>
>I would agree with your above statement about the second group,
>but not the ones listed in the first group, whose callers are
>prepared to receive error returns.  It just happens that these
>callees do not currently detect errors, but some of them
>certainly could be improved to return errors, instead of just
>calling die().
>
>-
>To unsubscribe from this list: send the line "unsubscribe git" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html