Re: AW: [RFC/PATCH v2 3/5] libbb: add ends_with() function
I did not see the email you've quoted on the mailing list. Interesting.
I shall respond to it now.
On 2015-08-25 03:00, dietmar.schind...@manroland-web.com wrote:
Von: Tito
Gesendet: Montag, 24. August 2015 21:29
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.
Yes, I will gladly pass the buck to the silly caller. It is the function
caller's responsibility to use the function properly and check for error
conditions the caller actually cares about. There is no way to know what
exactly will be done with a chunk of published code in ten years or in
ten hours. Basic C programming rules say to quietly work with as many
inputs as possible and fail gracefully when something goes wrong. In the
case of asking "ends with?" you get to choose between returning "int:
true/false" or "int: more possible outputs than simple true/false" or
"pointer to a string or NULL on failure." Since increasingly complex
return values tend to require increasingly complex code to handle those
possibilities, the question posed is most simply answered as a
true/false question: "does string X end with Y?" Choosing for Y ==
to return true or false is linguistically nonsensical but
*logical from a byte-for-byte analysis perspective* for the reasons I
discussed in my previous message.
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_.
More meaningful responses (by which we are really discussing "return
values" or--in one of the worst cases--structs passed as pointers and
modified by the called function as the "return value") require more
complicated code to handle those responses. Keeping responses as simple
as possible minimizes code size AND complexity. Adding complexity is
always bad by default unless adding complexity is ultimately the only
"simple" solution. Passing struct pointers around in network packet
handling code is unavoidable due to the information density required of
the task, for example, but it makes no sense to apply such density to
the straightforward question of "does this string end with this other
string?"
We could always perform very complex analysis of such a simple question
and return a series of bit flags for possibilities; for example, "yes,
but the case of the text differs" or "no, but if you offset left by one
character, yes" or "yes, and both substrings are in upper case." How
complex should the answer to the question be? How much time should we
waste trying to make our response more meaningful?
IMO for such a simple question, the calling code should be able to do
something like this if it wants and just be done with it:
if (ends_with(a, b) == 1) do_true_work();
else do_false_work();
In this case, if b == '' and that's a problem for the calling code, the
calling code should be going "if (*b == '\0') handle_empty_case()" and
dealing with it prior to calling ends_with(). Likewise, it's possible
that the author of the calling code knows that the data their code is
working with can safely ignore what happens when empty strings are
passed to ends_with() and attempting to "help" the author will only
frustrate them instead and possibly force them to make more obnoxious
code sort of like this:
x = ends_with(a, b);
switch (x) {
case 0:
do_false_work();
break;
case 1:
case -1:
do_true_work();
break;
default:
do_unknown_retval_error_message();
}
or a much smarter programmer not interested in a different response
might instead do one of these to bypass such "helpfulness" in responses:
if (*b != '\0') {
if (ends_with(a, b) == 0) {
do_false_work();
goto did_false_work;
}
}
do_true_work();
did_false_work:
...
if (ends_with == 0) do_false_work();
else do_true_work();
...
An exercise for the reader: what happens to the code in all these
workarounds if we have 0, 1, and 2 as return values for
{false,true,empty} and then "improve" the function later by changing
various "corner cases" to return negative values, or if we add return
value 3 for {ends_with_if_shifted_left_by_one_char} or any other such
"improvement" of similar type? Unless all *potential* future return
value sets are clearly documented, we could break things in interesting
ways!
Or, we just return "true/false" in the simplest possible way and stop
pretending we know more than the calling programmer does about their code.
This suffix being non-existent is a anomaly that probably the caller
would like to be informed about.
Then let them check for it. The caller knows their input data set. We do
not and we can never know in advance what will be thrown at any of the
reusable functions we write.
If a useless answer for empty strings is a problem, the writer of th
Re: AW: [RFC/PATCH v2 3/5] libbb: add ends_with() function
On 08/25/2015 09:00 AM, dietmar.schind...@manroland-web.com wrote: Von: Tito Gesendet: Montag, 24. August 2015 21:29 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 " 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. To add a data point: I vote for Jody Bruchon's coherent view. 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_. An empty string is a string also; treating it accordingly is meaningful. This suffix being non-existent is a anomaly that probably the caller would like to be informed about. Calling it an anomaly is a personal choice; I'd call it a normal extreme case. 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.). We cannot predict the usefulness of all possible cases, so this question implies an unprovable assumption. 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 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". We see that the strstr() function also does not handle a string with zero length in any special way; the manual just states explicitly what results from normal proceeding according to the description. Hi, that what results is still the opposite of what results in the ends_with() way for basically the same problem. Ciao, Tito -- Best regards, Dietmar Schindler ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [RFC/PATCH v2 3/5] libbb: add ends_with() function
On 08/24/2015 10:58 PM, Bartosz Gołaszewski wrote: 2015-08-24 21:29 GMT+02:00 Tito : 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 " 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 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 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
AW: [RFC/PATCH v2 3/5] libbb: add ends_with() function
> Von: Tito > Gesendet: Montag, 24. August 2015 21:29 > > 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 " 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. To add a data point: I vote for Jody Bruchon's coherent view. > 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_. An empty string is a string also; treating it accordingly is meaningful. > This suffix being non-existent is a anomaly that probably the caller > would like to be informed about. Calling it an anomaly is a personal choice; I'd call it a normal extreme case. > > 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.). We cannot predict the usefulness of all possible cases, so this question implies an unprovable assumption. > > 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 > > 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". We see that the strstr() function also does not handle a string with zero length in any special way; the manual just states explicitly what results from normal proceeding according to the description. -- Best regards, Dietmar Schindler manroland web systems GmbH -- Managing Director: Joern Gosse Registered Office: Augsburg -- Trade Register: AG Augsburg -- HRB-No.: 26816 -- VAT: DE281389840 Confidentiality note: This eMail and any files transmitted with it are confidential and intended solely for the use of the individual or entity to whom they are addressed. If you are not the intended recipient, you are hereby notified that any use or dissemination of this communication is strictly prohibited. If you have received this eMail in error, then please delete this eMail. ! Please consider your environmental responsibility before printing this eMail ! ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [RFC/PATCH v2 3/5] libbb: add ends_with() function
Hello Tito, 2015-08-24 21:29 GMT+02:00 Tito : > 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". uClibc does exactly that, but doesn't treat an empty key in a special way. It simply returns full string as a 'side effect' of the search algorithm used. That said, I have realized now that there's a previous similar function called is_prefixed_with, which finds if a string starts with a key and returns the rest of the string if found (i.e. the string without the prefix). And, as already has said Bartosz, maybe the correct name for ends_with may be is_suffixed_with. But for consistency it should also return the string at the position of the match or something like that. But until it's return value is not consistent with is_prefixed_with, I would *leave* it as ends_with to prevent confusion. Cheers, Xabier Oneca_,,_ ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [RFC/PATCH v2 3/5] libbb: add ends_with() function
Hello Bartosz, 2015-08-24 22:58 GMT+02:00 Bartosz Gołaszewski : > 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. Then you should have made ends_with patch part of the same patch as readahead implementation, so it would have gone unnoticed! ;) Cheers, Xabier Oneca_,,_ ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [RFC/PATCH v2 3/5] libbb: add ends_with() function
2015-08-24 21:29 GMT+02:00 Tito : > > > 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 " 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 > > 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 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [RFC/PATCH v2 3/5] libbb: add ends_with() function
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 " 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 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". Ciao, Tito Since there is no correct answer from a layman's perspective (or even a linguistic interpretation), the technically correct result is the only valid way I can see to resolve the stalemate and avoid making an arbitrary choice. -Jody ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [RFC/PATCH v2 3/5] libbb: add ends_with() function
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 " 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. 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." 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. Since there is no correct answer from a layman's perspective (or even a linguistic interpretation), the technically correct result is the only valid way I can see to resolve the stalemate and avoid making an arbitrary choice. -Jody ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [RFC/PATCH v2 3/5] libbb: add ends_with() function
Am 21.08.2015 22:34, schrieb Xabier Oneca -- xOneca:
> Hello Walter,
>
> 2015-08-21 18:26 GMT+02:00 walter harms :
>>
>> maybe its a bit late but ...
>>
>> the function name is a bit unfortunate whats about has_suffix() ?
>>
>> you can improve readability by returning strcmp directly and calculating
>> the len immediately.
>>
>> int has_suffix(const char *str, const char *key)
>> {
>> size_t len = strlen(str)-strlen(key);
>>
>> if (len < 0 )
>> return -1;
>>
>> return strcmp(str + len, key);
>> }
>>
>> re,
>> wh
>
> strcmp returns 0 when they match. I think it makes more sense if
> has_suffix returns 1 if match, 0 otherwise:
>
> return !strcmp(str + str_len - key_len, key);
>
> Also, can len be < 0? Isn't size_t unsigned?
yep, you are right to much c&p, i should have stayed with int.
my idea was make something compatible to strcmp().
my original also had tests for !key etc. the question is: Is that needed ?
re,
wh
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Re: [RFC/PATCH v2 3/5] libbb: add ends_with() function
On Fri, Aug 21, 2015 at 06:26:27PM +0200, walter harms wrote:
>
>
> Am 21.08.2015 16:23, schrieb Bartosz Golaszewski:
> > This function checks if given key can be found at the end of the string.
> >
> > Signed-off-by: Bartosz Golaszewski
> > ---
> > include/libbb.h | 1 +
> > libbb/compare_string_array.c | 30 ++
> > 2 files changed, 31 insertions(+)
> >
> > diff --git a/include/libbb.h b/include/libbb.h
> > index a56b684..7b41c9b 100644
> > --- a/include/libbb.h
> > +++ b/include/libbb.h
> > @@ -422,6 +422,7 @@ const char *bb_basename(const char *name) FAST_FUNC;
> > char *last_char_is(const char *s, int c) FAST_FUNC;
> > const char* endofname(const char *name) FAST_FUNC;
> > char *is_prefixed_with(const char *string, const char *key) FAST_FUNC;
> > +int ends_with(const char *str, const char *key) FAST_FUNC;
> >
> > int ndelay_on(int fd) FAST_FUNC;
> > int ndelay_off(int fd) FAST_FUNC;
> > diff --git a/libbb/compare_string_array.c b/libbb/compare_string_array.c
> > index e24815a..a2d77c7 100644
> > --- a/libbb/compare_string_array.c
> > +++ b/libbb/compare_string_array.c
> > @@ -23,6 +23,19 @@ char* FAST_FUNC is_prefixed_with(const char *string,
> > const char *key)
> > #endif
> > }
> >
> > +int FAST_FUNC ends_with(const char *str, const char *key)
> > +{
> > + size_t str_len = strlen(str), key_len = strlen(key);
> > +
> > + if (str_len >= key_len) {
> > + if (strcmp(str + str_len - key_len, key) == 0) {
> > + return 1;
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
>
> maybe its a bit late but ...
>
> the function name is a bit unfortunate whats about has_suffix() ?
>
> you can improve readability by returning strcmp directly and calculating
> the len immediately.
>
> int has_suffix(const char *str, const char *key)
> {
> size_t len = strlen(str)-strlen(key);
>
> if (len < 0 )
> return -1;
No you can't. len<0 is always false because len is unsigned. The
original code in the patch was correct; your version is buggy.
Rich
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Re: [RFC/PATCH v2 3/5] libbb: add ends_with() function
Hello, 2015-08-21 23:26 GMT+02:00 Tito : > what sense would it make to check if str\0 ends_with \0 if all strings do > end with \0 ? It can be helpful when key is dynamically created (e.g. extracted from a string) and it comes as an empty string. But it also depends on context: If it is too often checked that (*key == 0) before calling ends_with, it makes sense to embed that check in ends_with. If it doesn't matter, then why make the code (and binary) bigger? Cheers, Xabier Oneca_,,_ ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [RFC/PATCH v2 3/5] libbb: add ends_with() function
On 08/21/2015 10:46 PM, Xabier Oneca -- xOneca wrote: Hello tito, [...] ./test 'testprova' ends_with 'test' = no 'testprova' ends_with 'prova' = yes 'test' ends_with 'prova' = no 'test' ends_with 'testprova' = no '' ends_with 'testprova' = no 'test' ends_with '' = yes and I wonder if the last test result is correct: 'test' ends_with '' = yes Just for fun I've also added a different implementation that seems to do the right thing (at least for me): 'testprova' ends_with 'test' = no 'testprova' ends_with 'prova' = yes 'test' ends_with 'prova' = no 'test' ends_with 'testprova' = no '' ends_with 'testprova' = no 'test' ends_with '' = no Well, The Right Thing(tm) depends on the use case of ends_with()... To me that last line is correct, because "test\0" *ends* wtih "\0". But in a context where key should not be empty your exception may make sense. Cheers, Xabier Oneca_,,_ Hi, what sense would it make to check if str\0 ends_with \0 if all strings do end with \0 ? Ciao, Tito ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [RFC/PATCH v2 3/5] libbb: add ends_with() function
Hello tito, > [...] > > ./test > 'testprova' ends_with 'test' = no > 'testprova' ends_with 'prova' = yes > 'test' ends_with 'prova' = no > 'test' ends_with 'testprova' = no > '' ends_with 'testprova' = no > 'test' ends_with '' = yes > > and I wonder if the last test result is correct: > > 'test' ends_with '' = yes > > Just for fun I've also added a different implementation that > seems to do the right thing (at least for me): > > 'testprova' ends_with 'test' = no > 'testprova' ends_with 'prova' = yes > 'test' ends_with 'prova' = no > 'test' ends_with 'testprova' = no > '' ends_with 'testprova' = no > 'test' ends_with '' = no Well, The Right Thing(tm) depends on the use case of ends_with()... To me that last line is correct, because "test\0" *ends* wtih "\0". But in a context where key should not be empty your exception may make sense. Cheers, Xabier Oneca_,,_ ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [RFC/PATCH v2 3/5] libbb: add ends_with() function
2015-08-21 22:34 GMT+02:00 Xabier Oneca -- xOneca :
> Hello Walter,
>
> 2015-08-21 18:26 GMT+02:00 walter harms :
>>
>> maybe its a bit late but ...
>>
>> the function name is a bit unfortunate whats about has_suffix() ?
>>
>> you can improve readability by returning strcmp directly and calculating
>> the len immediately.
>>
>> int has_suffix(const char *str, const char *key)
>> {
>> size_t len = strlen(str)-strlen(key);
>>
>> if (len < 0 )
>> return -1;
>>
>> return strcmp(str + len, key);
>> }
>>
>> re,
>> wh
>
> strcmp returns 0 when they match. I think it makes more sense if
> has_suffix returns 1 if match, 0 otherwise:
>
> return !strcmp(str + str_len - key_len, key);
Should have been:
return !strcmp(str + len, key);
Sorry, I copied Bartosz's strcmp line, but you get it.. ;)
Cheers,
Xabier Oneca_,,_
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Re: [RFC/PATCH v2 3/5] libbb: add ends_with() function
Hello Walter,
2015-08-21 18:26 GMT+02:00 walter harms :
>
> maybe its a bit late but ...
>
> the function name is a bit unfortunate whats about has_suffix() ?
>
> you can improve readability by returning strcmp directly and calculating
> the len immediately.
>
> int has_suffix(const char *str, const char *key)
> {
> size_t len = strlen(str)-strlen(key);
>
> if (len < 0 )
> return -1;
>
> return strcmp(str + len, key);
> }
>
> re,
> wh
strcmp returns 0 when they match. I think it makes more sense if
has_suffix returns 1 if match, 0 otherwise:
return !strcmp(str + str_len - key_len, key);
Also, can len be < 0? Isn't size_t unsigned?
Cheers,
Xabier Oneca_,,_
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Re: [RFC/PATCH v2 3/5] libbb: add ends_with() function
Hi,
I've written a little test program to look at some corner cases for this new
ends_with function:
#include
#include
int ends_with(const char *str, const char *key)
{
size_t str_len = strlen(str), key_len = strlen(key);
if (str_len >= key_len) {
if (strcmp(str + str_len - key_len, key) == 0) {
return 1;
}
}
return 0;
}
int my_ends_with(const char *str, const char *key)
{
char *p;
const char *s = str;
while((p = strstr(s, key)) != NULL && *key != 0) {
// printf("p = %s s = %s\n", p, s);
if (strcmp(p, key) == 0)
return 1;
s++;
}
return 0;
}
int main(int argc, char **argv)
{
printf("'%s' ends_with '%s' = %s\n", "testprova", "test",
(ends_with("testprova", "test")) ? "yes" : "no");
printf("'%s' ends_with '%s' = %s\n", "testprova", "prova",
(ends_with("testprova", "prova")) ? "yes" : "no");
printf("'%s' ends_with '%s' = %s\n", "test", "prova",
(ends_with("test", "prova")) ? "yes" : "no");
printf("'%s' ends_with '%s' = %s\n", "test", "testprova",
(ends_with("test", "testprova")) ? "yes" : "no");
printf("'%s' ends_with '%s' = %s\n", "", "testprova", (ends_with("",
"testprova")) ? "yes" : "no");
printf("'%s' ends_with '%s' = %s\n", "test", "", (ends_with("test",
"")) ? "yes" : "no");
puts("==");
printf("'%s' ends_with '%s' = %s\n", "testprova", "test",
(my_ends_with("testprova", "test")) ? "yes" : "no");
printf("'%s' ends_with '%s' = %s\n", "testprova", "prova",
(my_ends_with("testprova", "prova")) ? "yes" : "no");
printf("'%s' ends_with '%s' = %s\n", "test", "prova",
(my_ends_with("test", "prova")) ? "yes" : "no");
printf("'%s' ends_with '%s' = %s\n", "test", "testprova",
(my_ends_with("test", "testprova")) ? "yes" : "no");
printf("'%s' ends_with '%s' = %s\n", "", "testprova", (my_ends_with("",
"testprova")) ? "yes" : "no");
printf("'%s' ends_with '%s' = %s\n", "test", "", (my_ends_with("test",
"")) ? "yes" : "no");
return 0;
}
The output is:
./test
'testprova' ends_with 'test' = no
'testprova' ends_with 'prova' = yes
'test' ends_with 'prova' = no
'test' ends_with 'testprova' = no
'' ends_with 'testprova' = no
'test' ends_with '' = yes
and I wonder if the last test result is correct:
'test' ends_with '' = yes
Just for fun I've also added a different implementation that
seems to do the right thing (at least for me):
'testprova' ends_with 'test' = no
'testprova' ends_with 'prova' = yes
'test' ends_with 'prova' = no
'test' ends_with 'testprova' = no
'' ends_with 'testprova' = no
'test' ends_with '' = no
Ciao,
Tito
On 08/21/2015 06:26 PM, walter harms wrote:
>
>
> Am 21.08.2015 16:23, schrieb Bartosz Golaszewski:
>> This function checks if given key can be found at the end of the string.
>>
>> Signed-off-by: Bartosz Golaszewski
>> ---
>> include/libbb.h | 1 +
>> libbb/compare_string_array.c | 30 ++
>> 2 files changed, 31 insertions(+)
>>
>> diff --git a/include/libbb.h b/include/libbb.h
>> index a56b684..7b41c9b 100644
>> --- a/include/libbb.h
>> +++ b/include/libbb.h
>> @@ -422,6 +422,7 @@ const char *bb_basename(const char *name) FAST_FUNC;
>> char *last_char_is(const char *s, int c) FAST_FUNC;
>> const char* endofname(const char *name) FAST_FUNC;
>> char *is_prefixed_with(const char *string, const char *key) FAST_FUNC;
>> +int ends_with(const char *str, const char *key) FAST_FUNC;
>>
>> int ndelay_on(int fd) FAST_FUNC;
>> int ndelay_off(int fd) FAST_FUNC;
>> diff --git a/libbb/compare_string_array.c b/libbb/compare_string_array.c
>> index e24815a..a2d77c7 100644
>> --- a/libbb/compare_string_array.c
>> +++ b/libbb/compare_string_array.c
>> @@ -23,6 +23,19 @@ char* FAST_FUNC is_prefixed_with(const char *string,
>> const char *key)
>> #endif
>> }
>>
>> +int FAST_FUNC ends_with(const char *str, const char *key)
>> +{
>> +size_t str_len = strlen(str), key_len = strlen(key);
>> +
>> +if (str_len >= key_len) {
>> +if (strcmp(str + str_len - key_len, key) == 0) {
>> +return 1;
>> +}
>> +}
>> +
>> +return 0;
>> +}
>> +
>
> maybe its a bit late but ...
>
> the function name is a bit unfortunate whats about has_suffix() ?
>
> you can improve readability by returning strcmp directly and calculating
> the len immediately.
>
> int has_suffix(const char *str, const char *key)
> {
> size_t len = strlen(str)-strlen(key);
>
> if (len < 0 )
> return -1;
>
> return strcmp(str + len, key);
> }
>
> re,
> wh
>
>
>> /* returns the array index of the string */
>> /* (index of first match is returned, or -1) */
>> int
Re: [RFC/PATCH v2 3/5] libbb: add ends_with() function
Am 21.08.2015 16:23, schrieb Bartosz Golaszewski:
> This function checks if given key can be found at the end of the string.
>
> Signed-off-by: Bartosz Golaszewski
> ---
> include/libbb.h | 1 +
> libbb/compare_string_array.c | 30 ++
> 2 files changed, 31 insertions(+)
>
> diff --git a/include/libbb.h b/include/libbb.h
> index a56b684..7b41c9b 100644
> --- a/include/libbb.h
> +++ b/include/libbb.h
> @@ -422,6 +422,7 @@ const char *bb_basename(const char *name) FAST_FUNC;
> char *last_char_is(const char *s, int c) FAST_FUNC;
> const char* endofname(const char *name) FAST_FUNC;
> char *is_prefixed_with(const char *string, const char *key) FAST_FUNC;
> +int ends_with(const char *str, const char *key) FAST_FUNC;
>
> int ndelay_on(int fd) FAST_FUNC;
> int ndelay_off(int fd) FAST_FUNC;
> diff --git a/libbb/compare_string_array.c b/libbb/compare_string_array.c
> index e24815a..a2d77c7 100644
> --- a/libbb/compare_string_array.c
> +++ b/libbb/compare_string_array.c
> @@ -23,6 +23,19 @@ char* FAST_FUNC is_prefixed_with(const char *string, const
> char *key)
> #endif
> }
>
> +int FAST_FUNC ends_with(const char *str, const char *key)
> +{
> + size_t str_len = strlen(str), key_len = strlen(key);
> +
> + if (str_len >= key_len) {
> + if (strcmp(str + str_len - key_len, key) == 0) {
> + return 1;
> + }
> + }
> +
> + return 0;
> +}
> +
maybe its a bit late but ...
the function name is a bit unfortunate whats about has_suffix() ?
you can improve readability by returning strcmp directly and calculating
the len immediately.
int has_suffix(const char *str, const char *key)
{
size_t len = strlen(str)-strlen(key);
if (len < 0 )
return -1;
return strcmp(str + len, key);
}
re,
wh
> /* returns the array index of the string */
> /* (index of first match is returned, or -1) */
> int FAST_FUNC index_in_str_array(const char *const string_array[], const
> char *key)
> @@ -110,3 +123,20 @@ smallint FAST_FUNC yesno(const char *str)
> return ret / 3;
> }
> #endif
> +
> +#if ENABLE_UNIT_TEST
> +
> +BBUNIT_DEFINE_TEST(ends_with)
> +{
> + BBUNIT_ASSERT_TRUE(ends_with("foo bar", "bar"));
> + BBUNIT_ASSERT_TRUE(ends_with("foo", "foo"));
> + BBUNIT_ASSERT_TRUE(ends_with("foo", ""));
> + BBUNIT_ASSERT_TRUE(ends_with("", ""));
> +
> + BBUNIT_ASSERT_FALSE(ends_with("foo", "bar foo"));
> + BBUNIT_ASSERT_FALSE(ends_with("foo foo", "bar"));
> +
> + BBUNIT_ENDTEST;
> +}
> +
> +#endif /* ENABLE_UNIT_TEST */
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[RFC/PATCH v2 3/5] libbb: add ends_with() function
This function checks if given key can be found at the end of the string.
Signed-off-by: Bartosz Golaszewski
---
include/libbb.h | 1 +
libbb/compare_string_array.c | 30 ++
2 files changed, 31 insertions(+)
diff --git a/include/libbb.h b/include/libbb.h
index a56b684..7b41c9b 100644
--- a/include/libbb.h
+++ b/include/libbb.h
@@ -422,6 +422,7 @@ const char *bb_basename(const char *name) FAST_FUNC;
char *last_char_is(const char *s, int c) FAST_FUNC;
const char* endofname(const char *name) FAST_FUNC;
char *is_prefixed_with(const char *string, const char *key) FAST_FUNC;
+int ends_with(const char *str, const char *key) FAST_FUNC;
int ndelay_on(int fd) FAST_FUNC;
int ndelay_off(int fd) FAST_FUNC;
diff --git a/libbb/compare_string_array.c b/libbb/compare_string_array.c
index e24815a..a2d77c7 100644
--- a/libbb/compare_string_array.c
+++ b/libbb/compare_string_array.c
@@ -23,6 +23,19 @@ char* FAST_FUNC is_prefixed_with(const char *string, const
char *key)
#endif
}
+int FAST_FUNC ends_with(const char *str, const char *key)
+{
+ size_t str_len = strlen(str), key_len = strlen(key);
+
+ if (str_len >= key_len) {
+ if (strcmp(str + str_len - key_len, key) == 0) {
+ return 1;
+ }
+ }
+
+ return 0;
+}
+
/* returns the array index of the string */
/* (index of first match is returned, or -1) */
int FAST_FUNC index_in_str_array(const char *const string_array[], const char
*key)
@@ -110,3 +123,20 @@ smallint FAST_FUNC yesno(const char *str)
return ret / 3;
}
#endif
+
+#if ENABLE_UNIT_TEST
+
+BBUNIT_DEFINE_TEST(ends_with)
+{
+ BBUNIT_ASSERT_TRUE(ends_with("foo bar", "bar"));
+ BBUNIT_ASSERT_TRUE(ends_with("foo", "foo"));
+ BBUNIT_ASSERT_TRUE(ends_with("foo", ""));
+ BBUNIT_ASSERT_TRUE(ends_with("", ""));
+
+ BBUNIT_ASSERT_FALSE(ends_with("foo", "bar foo"));
+ BBUNIT_ASSERT_FALSE(ends_with("foo foo", "bar"));
+
+ BBUNIT_ENDTEST;
+}
+
+#endif /* ENABLE_UNIT_TEST */
--
2.1.4
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
