Message ID | 20240317093901.3212684-2-liwang@redhat.com |
---|---|
State | Accepted |
Headers | show |
Series | provide a unified way to parse /proc/cmdline | expand |
Hi Li, > A new structure tst_kcmdline_var is defined to hold a command-line > argument's key and a fixed-size value. Furthermore, function > tst_kcmdline_parse is added to the corresponding .c file, which > reads from /proc/cmdline, parses the command-line arguments, and > populates the tst_kcmdline_var array with the obtained key-value > pairs, ensuring safe file operations and buffer size checks. Nice. Just compiler warning and formatting nits below (just fix them before merge). Thanks a lot for adding a test. Reviewed-by: Petr Vorel <pvorel@suse.cz> > Signed-off-by: Li Wang <liwang@redhat.com> > --- > include/tst_kconfig.h | 29 ++++++++++++++ > lib/newlib_tests/test_kconfig03.c | 40 +++++++++++++++++++ > lib/tst_kconfig.c | 66 +++++++++++++++++++++++++++++++ > 3 files changed, 135 insertions(+) > create mode 100644 lib/newlib_tests/test_kconfig03.c > diff --git a/include/tst_kconfig.h b/include/tst_kconfig.h > index 8b24a8380..bbb899784 100644 > --- a/include/tst_kconfig.h > +++ b/include/tst_kconfig.h > @@ -6,6 +6,8 @@ > #ifndef TST_KCONFIG_H__ > #define TST_KCONFIG_H__ > +#include <stdbool.h> > + > /** > * Initialization helper macro for struct tst_kconfig_var. Requires <string.h> very nic: missing dot at the end. > */ > @@ -64,4 +66,31 @@ void tst_kconfig_read(struct tst_kconfig_var vars[], size_t vars_len); > */ > int tst_kconfig_check(const char *const kconfigs[]); > +/** > + * Macro to initialize a tst_kcmdline_var structure with a specified parameter > + * name and an empty value. This is useful for setting up an array of parameter > + * structures before parsing the actual command-line arguments. > + */ > +#define TST_KCMDLINE_INIT(paraname) { \ > + .key = paraname, \ > + .value = "", \ > + .found = false \ > +} > + > +/** > + * Structure for storing command-line parameter key and its corresponding value and here. > + */ > +struct tst_kcmdline_var { > + const char *key; > + char value[128]; > + bool found; > +}; > + > +/** > + * Parses command-line parameters from /proc/cmdline and stores them in params array. > + * params: The array of tst_kcmdline_var structures to be filled with parsed key-value pairs. > + * params_len: The length of the params array, indicating how many parameters to parse. > + */ > +void tst_kcmdline_parse(struct tst_kcmdline_var params[], size_t params_len); > + > #endif /* TST_KCONFIG_H__ */ > diff --git a/lib/newlib_tests/test_kconfig03.c b/lib/newlib_tests/test_kconfig03.c > new file mode 100644 > index 000000000..937c2b29c > --- /dev/null > +++ b/lib/newlib_tests/test_kconfig03.c > @@ -0,0 +1,40 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Copyright (c) 2024 Li Wang <liwang@redhat.com> > + */ > + > +#include "tst_test.h" > +#include "tst_kconfig.h" > + > +static struct tst_kcmdline_var params[] = { > + TST_KCMDLINE_INIT("BOOT_IMAGE"), > + TST_KCMDLINE_INIT("root"), > + TST_KCMDLINE_INIT("foo") > +}; > + > +static void do_test(void) > +{ > + int i, N; > + > + N = (int) ARRAY_SIZE(params); > + > + tst_kcmdline_parse(params, N); > + > + for (i = 0; i < N; i++) { > + if (params[i].found) { > + if (params[i].value) test_kconfig03.c:25:29: warning: the comparison will always evaluate as ‘true’ for the address of ‘value’ will never be NULL [-Waddress] 25 | if (params[i].value) | ^~~~~~ In file included from test_kconfig03.c:7: ../../include/tst_kconfig.h:85:14: note: ‘value’ declared here 85 | char value[128]; It must be: if (params[i].value[0] != '\0') Or just without else at all: for (i = 0; i < N; i++) { if (params[i].found) { tst_res(TPASS, "params[%d] = {%s, %s}", i, params[i].key, params[i].value); } else { if (!strcmp(params[i].key, "foo")) tst_res(TPASS, "%s is not found in /proc/cmdline", params[i].key); else tst_res(TFAIL, "%s is not found in /proc/cmdline", params[i].key); } Kind regards, Petr > + tst_res(TPASS, "params[%d] = {%s, %s}", i, params[i].key, params[i].value); > + else > + tst_res(TPASS, "params[%d] = {%s, no value}", i, params[i].key); > + } else { > + if (!strcmp(params[i].key, "foo")) > + tst_res(TPASS, "%s is not found in /proc/cmdline", params[i].key); > + else > + tst_res(TFAIL, "%s is not found in /proc/cmdline", params[i].key); > + } > + } > +} ...
On Mon, Mar 18, 2024 at 5:10 AM Petr Vorel <pvorel@suse.cz> wrote: > Hi Li, > > > A new structure tst_kcmdline_var is defined to hold a command-line > > argument's key and a fixed-size value. Furthermore, function > > tst_kcmdline_parse is added to the corresponding .c file, which > > reads from /proc/cmdline, parses the command-line arguments, and > > populates the tst_kcmdline_var array with the obtained key-value > > pairs, ensuring safe file operations and buffer size checks. > > Nice. Just compiler warning and formatting nits below (just fix them > before merge). > Thanks a lot for adding a test. > > Reviewed-by: Petr Vorel <pvorel@suse.cz> > > > Signed-off-by: Li Wang <liwang@redhat.com> > > --- > > include/tst_kconfig.h | 29 ++++++++++++++ > > lib/newlib_tests/test_kconfig03.c | 40 +++++++++++++++++++ > > lib/tst_kconfig.c | 66 +++++++++++++++++++++++++++++++ > > 3 files changed, 135 insertions(+) > > create mode 100644 lib/newlib_tests/test_kconfig03.c > > > diff --git a/include/tst_kconfig.h b/include/tst_kconfig.h > > index 8b24a8380..bbb899784 100644 > > --- a/include/tst_kconfig.h > > +++ b/include/tst_kconfig.h > > @@ -6,6 +6,8 @@ > > #ifndef TST_KCONFIG_H__ > > #define TST_KCONFIG_H__ > > > +#include <stdbool.h> > > + > > /** > > * Initialization helper macro for struct tst_kconfig_var. Requires > <string.h> > very nic: missing dot at the end. > > */ > > @@ -64,4 +66,31 @@ void tst_kconfig_read(struct tst_kconfig_var vars[], > size_t vars_len); > > */ > > int tst_kconfig_check(const char *const kconfigs[]); > > > +/** > > + * Macro to initialize a tst_kcmdline_var structure with a specified > parameter > > + * name and an empty value. This is useful for setting up an array of > parameter > > + * structures before parsing the actual command-line arguments. > > + */ > > +#define TST_KCMDLINE_INIT(paraname) { \ > > + .key = paraname, \ > > + .value = "", \ > > + .found = false \ > > +} > > + > > +/** > > + * Structure for storing command-line parameter key and its > corresponding value > and here. > > > + */ > > +struct tst_kcmdline_var { > > + const char *key; > > + char value[128]; > > + bool found; > > +}; > > + > > +/** > > + * Parses command-line parameters from /proc/cmdline and stores them in > params array. > > + * params: The array of tst_kcmdline_var structures to be filled with > parsed key-value pairs. > > + * params_len: The length of the params array, indicating how many > parameters to parse. > > + */ > > +void tst_kcmdline_parse(struct tst_kcmdline_var params[], size_t > params_len); > > + > > #endif /* TST_KCONFIG_H__ */ > > diff --git a/lib/newlib_tests/test_kconfig03.c > b/lib/newlib_tests/test_kconfig03.c > > new file mode 100644 > > index 000000000..937c2b29c > > --- /dev/null > > +++ b/lib/newlib_tests/test_kconfig03.c > > @@ -0,0 +1,40 @@ > > +// SPDX-License-Identifier: GPL-2.0-or-later > > +/* > > + * Copyright (c) 2024 Li Wang <liwang@redhat.com> > > + */ > > + > > +#include "tst_test.h" > > +#include "tst_kconfig.h" > > + > > +static struct tst_kcmdline_var params[] = { > > + TST_KCMDLINE_INIT("BOOT_IMAGE"), > > + TST_KCMDLINE_INIT("root"), > > + TST_KCMDLINE_INIT("foo") > > +}; > > + > > +static void do_test(void) > > +{ > > + int i, N; > > + > > + N = (int) ARRAY_SIZE(params); > > + > > + tst_kcmdline_parse(params, N); > > + > > + for (i = 0; i < N; i++) { > > + if (params[i].found) { > > + if (params[i].value) > > test_kconfig03.c:25:29: warning: the comparison will always evaluate as > ‘true’ for the address of ‘value’ will never be NULL [-Waddress] > 25 | if (params[i].value) > | ^~~~~~ > In file included from test_kconfig03.c:7: > ../../include/tst_kconfig.h:85:14: note: ‘value’ declared here > 85 | char value[128]; > > It must be: > if (params[i].value[0] != '\0') > > Or just without else at all: > > for (i = 0; i < N; i++) { > if (params[i].found) { > tst_res(TPASS, "params[%d] = {%s, %s}", i, > params[i].key, params[i].value); > } else { > if (!strcmp(params[i].key, "foo")) > tst_res(TPASS, "%s is not found in > /proc/cmdline", params[i].key); > else > tst_res(TFAIL, "%s is not found in > /proc/cmdline", params[i].key); > } > Looks good, Patch applied with your suggestion. Thanks!
diff --git a/include/tst_kconfig.h b/include/tst_kconfig.h index 8b24a8380..bbb899784 100644 --- a/include/tst_kconfig.h +++ b/include/tst_kconfig.h @@ -6,6 +6,8 @@ #ifndef TST_KCONFIG_H__ #define TST_KCONFIG_H__ +#include <stdbool.h> + /** * Initialization helper macro for struct tst_kconfig_var. Requires <string.h> */ @@ -64,4 +66,31 @@ void tst_kconfig_read(struct tst_kconfig_var vars[], size_t vars_len); */ int tst_kconfig_check(const char *const kconfigs[]); +/** + * Macro to initialize a tst_kcmdline_var structure with a specified parameter + * name and an empty value. This is useful for setting up an array of parameter + * structures before parsing the actual command-line arguments. + */ +#define TST_KCMDLINE_INIT(paraname) { \ + .key = paraname, \ + .value = "", \ + .found = false \ +} + +/** + * Structure for storing command-line parameter key and its corresponding value + */ +struct tst_kcmdline_var { + const char *key; + char value[128]; + bool found; +}; + +/** + * Parses command-line parameters from /proc/cmdline and stores them in params array. + * params: The array of tst_kcmdline_var structures to be filled with parsed key-value pairs. + * params_len: The length of the params array, indicating how many parameters to parse. + */ +void tst_kcmdline_parse(struct tst_kcmdline_var params[], size_t params_len); + #endif /* TST_KCONFIG_H__ */ diff --git a/lib/newlib_tests/test_kconfig03.c b/lib/newlib_tests/test_kconfig03.c new file mode 100644 index 000000000..937c2b29c --- /dev/null +++ b/lib/newlib_tests/test_kconfig03.c @@ -0,0 +1,40 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (c) 2024 Li Wang <liwang@redhat.com> + */ + +#include "tst_test.h" +#include "tst_kconfig.h" + +static struct tst_kcmdline_var params[] = { + TST_KCMDLINE_INIT("BOOT_IMAGE"), + TST_KCMDLINE_INIT("root"), + TST_KCMDLINE_INIT("foo") +}; + +static void do_test(void) +{ + int i, N; + + N = (int) ARRAY_SIZE(params); + + tst_kcmdline_parse(params, N); + + for (i = 0; i < N; i++) { + if (params[i].found) { + if (params[i].value) + tst_res(TPASS, "params[%d] = {%s, %s}", i, params[i].key, params[i].value); + else + tst_res(TPASS, "params[%d] = {%s, no value}", i, params[i].key); + } else { + if (!strcmp(params[i].key, "foo")) + tst_res(TPASS, "%s is not found in /proc/cmdline", params[i].key); + else + tst_res(TFAIL, "%s is not found in /proc/cmdline", params[i].key); + } + } +} + +static struct tst_test test = { + .test_all = do_test, +}; diff --git a/lib/tst_kconfig.c b/lib/tst_kconfig.c index 595ea4b09..e16ca1400 100644 --- a/lib/tst_kconfig.c +++ b/lib/tst_kconfig.c @@ -14,6 +14,7 @@ #include "tst_private.h" #include "tst_kconfig.h" #include "tst_bool_expr.h" +#include "tst_safe_stdio.h" static int kconfig_skip_check(void) { @@ -565,3 +566,68 @@ char tst_kconfig_get(const char *confname) return var.choice; } + +void tst_kcmdline_parse(struct tst_kcmdline_var params[], size_t params_len) +{ + char buf[128], line[512]; + size_t b_pos = 0,l_pos =0, i; + int var_id = -1; + + FILE *f = SAFE_FOPEN("/proc/cmdline", "r"); + + if (fgets(line, sizeof(line), f) == NULL) { + SAFE_FCLOSE(f); + tst_brk(TBROK, "Failed to read /proc/cmdline"); + } + + for (l_pos = 0; line[l_pos] != '\0'; l_pos++) { + char c = line[l_pos]; + + switch (c) { + case '=': + buf[b_pos] = '\0'; + for (i = 0; i < params_len; i++) { + if (strcmp(buf, params[i].key) == 0) { + var_id = (int)i; + params[i].found = true; + } + } + + b_pos = 0; + break; + case ' ': + case '\n': + buf[b_pos] = '\0'; + if (var_id >= 0 && var_id < (int)params_len) + strcpy(params[var_id].value, buf); + + var_id = -1; + b_pos = 0; + break; + default: + if (b_pos + 1 >= sizeof(buf)) { + tst_res(TWARN, "Buffer overflowed while parsing /proc/cmdline"); + while (line[l_pos] != '\0' && line[l_pos] != ' ' && line[l_pos] != '\n') + l_pos++; + + var_id = -1; + b_pos = 0; + + if (line[l_pos] != '\0') + l_pos--; + } else { + buf[b_pos++] = c; + } + break; + } + } + + for (i = 0; i < params_len; i++) { + if (params[i].found) + tst_res(TINFO, "%s is found in /proc/cmdline", params[i].key); + else + tst_res(TINFO, "%s is not found in /proc/cmdline", params[i].key); + } + + SAFE_FCLOSE(f); +}
A new structure tst_kcmdline_var is defined to hold a command-line argument's key and a fixed-size value. Furthermore, function tst_kcmdline_parse is added to the corresponding .c file, which reads from /proc/cmdline, parses the command-line arguments, and populates the tst_kcmdline_var array with the obtained key-value pairs, ensuring safe file operations and buffer size checks. Signed-off-by: Li Wang <liwang@redhat.com> --- include/tst_kconfig.h | 29 ++++++++++++++ lib/newlib_tests/test_kconfig03.c | 40 +++++++++++++++++++ lib/tst_kconfig.c | 66 +++++++++++++++++++++++++++++++ 3 files changed, 135 insertions(+) create mode 100644 lib/newlib_tests/test_kconfig03.c