linux-kernel - Re: [PATCH 1/2] lib: add __sysfs_match_string_with

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 23 Apr 2019 06:38:44 +0000
From:   "Ardelean, Alexandru" <alexandru.Ardelean@...log.com>
To:     "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>
CC:     "namhyung@...nel.org" <namhyung@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "andriy.shevchenko@...ux.intel.com" 
        <andriy.shevchenko@...ux.intel.com>,
        "linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
        "mingo@...nel.org" <mingo@...nel.org>
Subject: Re: [PATCH 1/2] lib: add __sysfs_match_string_with_gaps() helper

On Mon, 2019-04-22 at 23:06 +0200, Greg KH wrote:
> 
> 
> On Mon, Apr 22, 2019 at 11:32:56AM +0300, Alexandru Ardelean wrote:
> > This helper is similar to __sysfs_match_string() with the exception
> > that it
> > ignores NULL elements within the array.
> 
> sysfs is "one value per file", why are you trying to write multiple
> things on a single line to a single sysfs file?
> 
> Is IIO really that messy?  :)
> 

Hmm, I don't think I understood the comment/question, or maybe I did not
formulate the comment properly.

Maybe Jonathan can pitch-in here if I'm saying something wrong.

So, in IIO there is `struct iio_enum` which is essentially a sysfs wrapper
for exposing an "enum" type to userspace via sysfs (which takes only one
value). This iio_enum type is basically a string-to-int mapping.

Some example in C:

enum {
    ENUM0,
    ENUM1,
    ENUM5 = 5,
    ENUM6,
    ENUM7
};

/* Notice the gaps in the elements */
static const char * const item_strings[] = {
        [ENUM0] = "mode0",
        [ENUM1] = "mode1",
        [ENUM5] = "mode5",
        [ENUM6] = "mode6", 
        [ENUM7] = "mode7",
};

static const struct iio_enum iio_enum1 = {
        .items = item_strings,
        .num_items = ARRAY_SIZE(item_strings),
        .set = iio_enum1_set,
        .get = iio_enum1_get,
};

The signature of the iio_enum1_set / iio_enum1_get is below:

static int iio_enum1_set(struct iio_dev *indio_dev,
        const struct iio_chan_spec *chan, unsigned int val);

static int iio_enum1_get(struct iio_dev *indio_dev,
        const struct iio_chan_spec *chan)

IIO core resolves the string-to-int mapping.
It uses __sysfs_match_string() to do that, but it requires that the list of
strings (and C enums) be contiguous.
This change [and V2 of this patch] introduces a
__sysfs_match_string_with_gaps() helper that ignores gaps (represented as
NULLs).

For reference, __sysfs_match_string() returns -EINVAL on the first NULL in
the array of strings (regardless of the given array size).

__sysfs_match_string_with_gaps() is typically helpful when C enums refer to
bitfields, or have some equivalence in HW.

Thanks
Alex

> thanks,
> 
> greg k-h