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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



David Rientjes <rientjes@xxxxxxxxxx> 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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]