Re: [PATCH v3 0/3] Add a couple of string-related functions

[Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2136 bytes --]

On Wed, Sep 30, 2015 at 01:31:32PM +1000, David Gibson wrote:
> On Tue, Sep 29, 2015 at 11:09:05AM +0200, Thierry Reding wrote:
> > From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > 
> > These three patches add a couple of string functions that have proven
> > useful in U-Boot's copy of libfdt, so they are likely to be useful for
> > other users as well.
> > 
> > Patch 1 adds a function to count the number of strings in a property's
> > value. This also adds a new DTS sample along with a small test program
> > to validate the implemented functions.
> > 
> > Patch 2 adds a function to retrieve the index of a given string in any
> > given property's value. This adds code to the test program introduced in
> > the previous patch to exercise the new functionality.
> > 
> > Patch 3 adds a function to retrieve a string by index from a property's
> > value. This extends the test program introduced in patch 1 to validate
> > the new functionality.
> 
> Thanks, I've applied this.
> 
> I did end up making a couple of small changes.
> 
> First, I forgot to say earlier that I also wanted to change the
> testcase name to stringlist* for clarity.

Okay, makes sense.

> Second, Travis CI picked up some -Wshadow warnings: in a bunch of
> places you were using 'index' as a variable name, which shadows the
> libc index(3) function.  I'm not sure why that didn't show up on a
> local "make check", but in any case I've changed thos to 'idx' to fix
> it.

As far as I can tell that's because index(3) is declared in strings.h on
my system (the file ships with the GNU libc 2.22) and that header file
isn't included from anywhere. Perhaps your system differs from that?

Actually there's also a prototype for index(3) in string.h on my system
but it is guarded with an #ifdef __USE_MISC, and upon closer inspection
that does get set, so the index(3) symbol should be available. No idea
why I don't get a warning, though, -Wshadow is definitely getting passed
to the compiler.

Anyway, those changes sound reasonable in either case, thanks for
applying.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]