[RFC/PATCH v2 3/5] libbb: add ends_with() function

Tito farmatito at tiscali.it
Tue Aug 25 12:08:51 UTC 2015



On 08/24/2015 10:58 PM, Bartosz Gołaszewski wrote:
> 2015-08-24 21:29 GMT+02:00 Tito <farmatito at tiscali.it>:
>>
>>
>> On 08/24/2015 12:51 AM, Jody Bruchon wrote:
>>>
>>> On 2015-08-21 17:26, Tito wrote:
>>>>
>>>> what sense would it make to check if str\0 ends_with \0 if all strings
>>>> do end with \0 ?
>>>
>>>
>>> In this case, I view this as a problem of handling all possible valid
>>> inputs in a robust manner. It can also make the code more efficient
>>> since \0 is just another byte and is guaranteed to be at the end of any
>>> valid string, thus solving the problem of what to do when passed an
>>> empty (but properly zero-terminated) string without having to add code
>>> for a special case and/or add parameters for string lengths.
>>>
>>> It does not matter that "ends with <empty>" is a comparison that almost
>>> certainly serves no practical purpose in the code calling it; what
>>> matters is that the behavior is consistent if the calling code is silly
>>> enough to pass empty strings and expect a meaningful response.
>>
>>
>> Hi Jody,
>> so you pass the buck to the silly caller to guess what is consistent, but
>> even in the few replies on this thread there were different views about it.
>> In my opinion it would be better to cover such corner cases with
>> meaningful responses by analyzing the semantics of this new function
>> that checks if a _string_ ends with a specific _string_ or as somebody
>> suggested a specific _suffix_.
>> This suffix being non-existent is a anomaly that probably the caller
>> would like to be informed about.
>>
>>> If a useless answer for empty strings is a problem, the writer of the
>>> calling
>>> code needs to handle that themselves, and is in a far more flexible
>>> position to do so than the person writing the called function that only
>>> answers "yes" or "no."
>>
>>
>> Why should a function give a useless answer?
>> Wouldn't it be better for the caller to give him
>> always useful answers (return different values, set errno, etc.).
>>
>>> I also say that we're all C programmers if we're here fiddling with the
>>> guts of BusyBox and we can all easily understand why returning "yes" for
>>> "ends with \0?" is a technically correct result since all C strings end
>>> with \0.
>>
>>
>> This is a somewhat dogmatic argument:
>> "The people of the inner circle know it better."
>>
>> SusV3 instead for the strstr function of which ends_with() could be
>> considered a subset of functionality (just looks for s2 at the end of s1)
>> says:
>>
>> "#include <string.h>
>>
>>      char *strstr(const char *s1, const char *s2);
>>
>> DESCRIPTION
>>
>>      The strstr() function shall locate the first occurrence in the string
>> pointed to by s1 of the sequence of bytes (excluding the terminating null
>> byte) in the string pointed to by s2.
>>
>> RETURN VALUE
>>
>>      Upon successful completion, strstr() shall return a pointer to the
>> located string or a null pointer if the string is not found.
>>
>>      If s2 points to a string with zero length, the function shall return s1.
>>
>> ERRORS
>>
>>      No errors are defined."
>>
>> So as you can see they explicitly request a specific action for the case
>> of a zero length key:
>>
>>      "If s2 points to a string with zero length, the function shall return
>> s1."
>>
>> Not that I like their solution but still this is a fact and makes me
>> think that we should also handle this corner case in better way
>> rather than return a "useless value".
>
> To be honest this is the part of the series I don't really care much
> about as opposed to the actual readahead patch that makes use of this
> new function. Let's call it is_suffixed_with() to stay consistent with
> is_prefixed_with() & make it return true for key="\0". The way it
> works (including key="\0") shall be reflected in the comment
> describing it. What do you think?
>
Hi,
this is a step in the right direction.

Thanks for your time and effort.

Ciao,
Tito


More information about the busybox mailing list