Re: [PATCH V2 1/8] tools lib: adopt str_has_suffix() from bpftool/gen.c

[Arnaldo Carvalho de Melo <acme@kernel.org>]

On Thu, Apr 11, 2024 at 05:45:25PM +0530, Chaitanya S Prakash wrote:
> On 4/9/24 05:02, Masami Hiramatsu (Google) wrote:
> > On Mon,  8 Apr 2024 11:52:23 +0530
> > Chaitanya S Prakash <ChaitanyaS.Prakash@arm.com> wrote:
> > > and add it to tools/lib/string.c The function is modified to return
> > > strlen(suffix) on success and 0 on failure.
> > > +++ b/tools/include/linux/string.h
> > > @@ -42,6 +42,8 @@ static inline bool strstarts(const char *str, const char *prefix)
> > >   	return strncmp(str, prefix, strlen(prefix)) == 0;
> > >   }
> > > +size_t str_has_suffix(const char *str, const char *suffix);
> > > +
> > >   extern char * __must_check skip_spaces(const char *);
> > >   extern char *strim(char *);
> > > diff --git a/tools/lib/string.c b/tools/lib/string.c
> > > index 8b6892f959ab..9cb515048239 100644
> > > --- a/tools/lib/string.c
> > > +++ b/tools/lib/string.c
> > > @@ -226,3 +226,29 @@ void *memchr_inv(const void *start, int c, size_t bytes)
> > >   	return check_bytes8(start, value, bytes % 8);
> > >   }
> > > +
> > > +/**
> > > + * str_has_suffix - Test is a string has a given suffix
> > > + * @str: The string to be tested
> > > + * @suffix: The string to see if @str ends with
> > > + *
> > > + * Returns:
> > > + * * strlen(@suffix) if @str ends with @suffix
> > > + * * 0 if @str does not end with @suffix
> > I think this should return boolean, because user will never check the
> > return value is strlen(@suffix). (and user always use the return value
> > as a boolean)
 
> This is in accordance with Arnaldo's comments on V1 of the patch series.
> Please find the link for the same below.
 
> https://lore.kernel.org/all/ZdYPf3wMl35VemsL@x1/

Yeah, when I saw str_has_prefix() I thought it too clever, but then, why
not, there are use cases, in the kernel, where that is useful.

And one thing with tools/ that we try hard is to reduce the differences
of programming in the kernel and in there, so that people used to
functions in the kernel use them in tools/ with the same semantics.

So in these "the user will never do things like this" moments I try to
look at the users in the codebase where we're lifting the function, and
in there _there_ are uses of the strlen() return, to my surprise.

- Arnaldo
 
> > > +size_t str_has_suffix(const char *str, const char *suffix)
> > > +{
> > > +	size_t i;
> > > +	size_t str_len = strlen(str);
> > > +	size_t suffix_len = strlen(suffix);
> > > +
> > > +	if (str_len < suffix_len)
> > > +		return 0;
> > > +
> > > +	for (i = 0; i < suffix_len ; i++) {
> > > +		if (str[str_len  - i - 1] != suffix[suffix_len - i - 1])
> > > +			return 0;
> > > +	}
> > > +
> > > +	return suffix_len;