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);