Messages in this thread Patch in this message | | | Date | Fri, 8 Jul 2022 18:32:38 +0200 | Subject | Re: [PATCH 1/2] lib/string_helpers: Introduce strsplit_u32() | From | Cezary Rojewski <> |
| |
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);
| |