Re: [PATCH v5 01/14] Add data structures and basic functions for commit trailers

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

 



On Sun, Feb 9, 2014 at 8:48 AM, Christian Couder
<chriscool@xxxxxxxxxxxxx> wrote:
> From: Junio C Hamano <gitster@xxxxxxxxx>
>>
>> Christian Couder <chriscool@xxxxxxxxxxxxx> writes:
>>
>>> +static inline int same_trailer(struct trailer_item *a, struct trailer_item *b, int alnum_len)
>>> +{
>>> +    return same_token(a, b, alnum_len) && same_value(a, b);
>>> +}
>>
>> All these "inlines" look premature optimization that can be
>> delegated to any decent compiler, don't they?
>
> Yeah, but as Eric suggested to add them like in header files and you
> did not reply to him, I thought you agreed with him.
> I will remove them.

If these functions are used only by trailer.c, then it would make
sense to move them from trailer.h to trailer.c so that they don't get
defined unnecessarily by each .c file which includes trailer.h.

>>> +/* Get the length of buf from its beginning until its last alphanumeric character */
>>> +static inline size_t alnum_len(const char *buf, int len)
>>> +{
>>> +    while (--len >= 0 && !isalnum(buf[len]));
>>> +    return (size_t) len + 1;
>>
>> This is somewhat unfortunate.  if the caller wants to receive
>> size_t, perhaps it should be passing in size_t (or ssize_t) to the
>> function?  Hard to guess without an actual caller, though.
>
> Ok, I will make it return an int.

Why not adjust the loop condition slightly instead so that you can
continue to accept and return size_t?
--
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]