lkml.org 
[lkml]   [2022]   [Jul]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
SubjectRe: [PATCH 1/2] lib/string_helpers: Introduce strsplit_u32()
From
On 2022-07-08 5:25 PM, Andy Shevchenko wrote:
> On Fri, Jul 8, 2022 at 2:34 PM Péter Ujfalusi
> <peter.ujfalusi@linux.intel.com> wrote:

...

>>> It seems you are missing the (1). The code has checks for the case where you
>>> can do get number upfront, it would just require two passes, but it's nothing
>>> in comparison of heave realloc().
>>>
>>> unsigned int *tokens;
>>> char *p;
>>> int num;
>>>
>>> p = get_options(str, 0, &num);
>>> if (num == 0)
>>> // No numbers in the string!
>>>
>>> tokens = kcalloc(num + 1, sizeof(*tokens), GFP_KERNEL);
>>> if (!tokens)
>>> return -ENOMEM;
>>>
>>> p = get_oprions(str, num, &tokens);
>>> if (*p)
>>> // String was parsed only partially!
>>> // assuming it's not a fatal error
>>>
>>> return tokens;
>
>> This diff is tested and works:
>
> Thanks, Peter!
>
> But at least you can memove() to avoid second allocation.
> ideally to refactor that the result of get_options is consumed as is
> (it may be casted to struct tokens { int n; u32 v[]; })


A long shot, but what if we were to modify get_options() so it takes
additional element-size parameter instead? Something like below - I've
ignored get_range() though. Will re-visit if this option is viable.


diff --git a/lib/cmdline.c b/lib/cmdline.c
index 5546bf588780..272f892b71df 100644
--- a/lib/cmdline.c
+++ b/lib/cmdline.c
@@ -53,7 +53,7 @@ static int get_range(char **str, int *pint, int n)
* for the sake of simplification.
*/

-int get_option(char **str, int *pint)
+int get_num_option(char **str, void *pint, size_t nsize)
{
char *cur = *str;
int value;
@@ -65,7 +65,7 @@ int get_option(char **str, int *pint)
else
value = simple_strtoull(cur, str, 0);
if (pint)
- *pint = value;
+ memcpy(pint, &value, min(nsize, sizeof(value)));
if (cur == *str)
return 0;
if (**str == ',') {
@@ -77,6 +77,12 @@ int get_option(char **str, int *pint)

return 1;
}
+EXPORT_SYMBOL(get_num_option);
+
+int get_option(char **str, int *pint)
+{
+ return get_num_option(str, pint, sizeof(*pint));
+}
EXPORT_SYMBOL(get_option);

/**
@@ -104,15 +110,15 @@ EXPORT_SYMBOL(get_option);
* completely parseable).
*/

-char *get_options(const char *str, int nints, int *ints)
+char *get_num_options(const char *str, int nints, void *ints, size_t nsize)
{
bool validate = (nints == 0);
int res, i = 1;

while (i < nints || validate) {
- int *pint = validate ? ints : ints + i;
+ int *pint = validate ? ints : ints + (i * nsize);

- res = get_option((char **)&str, pint);
+ res = get_num_option((char **)&str, pint, nsize);
if (res == 0)
break;
if (res == 3) {
@@ -133,9 +139,17 @@ char *get_options(const char *str, int nints, int
*ints)
if (res == 1)
break;
}
- ints[0] = i - 1;
+ --i;
+ memcpy(ints, &i, min(nsize, sizeof(i)));
return (char *)str;
}
+EXPORT_SYMBOL(get_num_options);
+
+char *get_options(const char *str, int nints, int *ints)
+{
+ return get_num_options(str, nints, ints, sizeof(*ints));
+}
+
EXPORT_SYMBOL(get_options);

\
 
 \ /
  Last update: 2022-07-08 18:34    [W:0.839 / U:0.328 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site