Re: [PATCH 1/2] lib/string_helpers: Introduce strsplit_u32()

From: Cezary Rojewski
Date: Tue Jul 12 2022 - 09:51:27 EST




On 2022-07-09 10:42 PM, Andy Shevchenko wrote:
On Sat, Jul 09, 2022 at 10:45:49AM +0200, Cezary Rojewski wrote:
On 2022-07-08 6:49 PM, Andy Shevchenko wrote:
On Fri, Jul 8, 2022 at 6:32 PM Cezary Rojewski
<cezary.rojewski@xxxxxxxxx> wrote:

On 2022-07-08 5:25 PM, Andy Shevchenko wrote:
On Fri, Jul 8, 2022 at 2:34 PM Péter Ujfalusi
<peter.ujfalusi@xxxxxxxxxxxxxxx> wrote:

A long shot, but what if we were to modify get_options() so it takes
additional element-size parameter instead?

But why? int / unsigned int, u32 / s32 are all compatible in the current cases.

I'd like to avoid any additional operations, so that the retrieved payload
can be provided to the IPC handler directly. The IPC handlers for AudioDSP
drivers are expecting payload in u32s.

// u32 **tkns, size_t *num_tkns as foo() arguments
// u32 *ints, int nints as locals

get_options(buf, 0, &nints);
if (!nints) {
ret = -ENOENT;
goto free_buf;
}

ints = kcalloc(nints + 1, sizeof(*ints), GFP_KERNEL);
if (!ints) {
ret = -ENOMEM;
goto free_buf;
}

get_num_options(buf, nints + 1, ints, sizeof(*ints));

*tkns = ints;
*num_tkns = nints;

No additional operations in between. The intermediate IPC handler can later
refer to the actual payload via &tkns[1] before passing it to the generic
one.

Casting int array into u32 array does not feel right, or perhaps I'm missing
something like in the doc case.

C standard.

int to unsigned int is not promoted. And standard says that "The rank of any
unsigned integer type shall equal the rank of the corresponding signed integer
type, if any."

I don't know why one needs to have an additional churn here. int and unsigned
int are interoperable with the adjustment to the sign when the other argument
is signed or lesser rank of.


I still believe that casting blindly is not the way to go. I did explicitly ask about int vs u32, not int vs unsigned int. Please note that these values are later passed to the IPC handlers, and this changes the context a bit. If hw expects u32, then u32 it shall be.

Please correct me if I'm wrong, but there is no guarantee that int is always 32bits long. What is guaranteed though, is that int holds at least -/+ 32,767. Also, values larger than INT_MAX are allowed in the IPC payload.


Regards,
Czarek