Re: [PATCH V2 0/8] perf tools: Fix test "perf probe of function from different CU"
From: Arnaldo Carvalho de Melo
Date: Wed Apr 17 2024 - 09:41:34 EST
On Wed, Apr 17, 2024 at 02:24:33PM +0100, James Clark wrote:
> On 14/04/2024 12:41, Alexey Dobriyan wrote:
> > On Thu, Apr 11, 2024 at 05:40:04PM +0530, Chaitanya S Prakash wrote:
> >> On 4/9/24 11:02, Alexey Dobriyan wrote:
> >>> On Mon, Apr 08, 2024 at 11:52:22AM +0530, Chaitanya S Prakash wrote:
> >>>> - Add str_has_suffix() and str_has_prefix() to tools/lib/string.c
> >>>> - Delete ends_with() and replace its usage with str_has_suffix()
> >>>> - Delete strstarts() from tools/include/linux/string.h and replace its
> >>>> usage with str_has_prefix()
> >>> It should be the other way: starts_with is normal in userspace.
> >>> C++, Python, Java, C# all have it. JavaScript too!
> >>
> >> This is done in accordance with Ian's comments on V1 of this patch
> >> series. Please find the link to the same below.
> >
> > Yes, but str_has_suffix() doesn't make sense in the wider context.
> >
> >> https://lore.kernel.org/all/CAP-5=fUFmeoTjLuZTgcaV23iGQU1AdddG+7Rw=d6buMU007+1Q@xxxxxxxxxxxxxx/
> >
> > The naming ends_with makes sense but there is also strstarts and
> > str_has_prefix, perhaps str_has_suffix would be the most consistent
> > and intention revealing name?
> From a brief check it looks like str_has_prefix() is already quite
> common with 94 uses. So the path of least resistance is to make
> everything self consistent and add str_has_suffix().
> I agree it's a bit of a mouthful and not so common in other languages.
> Once this more complicated set gets through we could always do a simple
> search and replace change it to anything we like. But it would touch
> _lots_ of different drivers and trees, so it would be hard to justify.
Right, we try to follow the kernel APIs to make tools/perf more familiar
to kernel developers, this return strlen() thing on str_has_prefix()
looked too clever for me at first, but since we need to do it anyway,
and return !0 to indicate it has the prefix and there are usecase...
The bpftool people agreed as well, so in tools/ it seems we're in
general agreement about using str_has_{prefix,suffix}().
- Arnaldo