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

[Bartosz Gołaszewski]

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?

-- 
Best regards,
Bartosz Golaszewski