| Message ID | 20240309090113.3121235-2-liwang@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | provide a unified way to parse /proc/cmdline | expand |
Hi Li,
It looks to me you introduced new warnings, could you please fix them before
merged?
tst_kconfig.c:398: ERROR: code indent should use tabs where possible
tst_kconfig.c:398: WARNING: please, no spaces at the start of a line
tst_kconfig.c:570: ERROR: open brace '{' following function definitions go on the next line
tst_kconfig.c:600: ERROR: trailing statements should be on next line
Also nits (to be changed before merge) below.
> 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 | 23 ++++++++++++++++++++++
> lib/tst_kconfig.c | 45 +++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 68 insertions(+)
> diff --git a/include/tst_kconfig.h b/include/tst_kconfig.h
> index 8b24a8380..a8cbfb786 100644
> --- a/include/tst_kconfig.h
> +++ b/include/tst_kconfig.h
> @@ -64,4 +64,27 @@ 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 = "" \
> +}
> +
> +/* Structure for storing command-line parameter key and its corresponding value */
nit: I guess /** is for autogenerated docs (maybe Andrea uses it one day with
sphinx :). Therefore maybe here also /* ? Also missing dot at the end of the
sentence.
> +struct tst_kcmdline_var {
> + const char *key;
> + char value[128];
> +};
> +
> +/**
> + * 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
nit: @param params, missing dot at the end of the sentence.
> + * params_len: The length of the params array, indicating how many parameters to parse
nit: @param params_len, missing dot at the end of the sentence.
> + */
> +void tst_kcmdline_parse(struct tst_kcmdline_var params[], size_t params_len);
> +
> #endif /* TST_KCONFIG_H__ */
> diff --git a/lib/tst_kconfig.c b/lib/tst_kconfig.c
> index 595ea4b09..c200dd261 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,47 @@ 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];
> + size_t buf_pos = 0, i;
> + int var_id = -1, c;
> +
> + FILE *f = SAFE_FOPEN("/proc/cmdline", "r");
> +
> + while ((c = fgetc(f)) != EOF) {
I hope fgetc() does not explode on ppc64le the same way as it did on
libs/libltpswap/libswap.c (see 6f82542fc ("libswap.c: Improve calculate swap dev
number")), where for some reason fgets() had to be used. But here it's actually
processing chars, not just counting EOF, so it should be ok. I'll try to retest it,
but please have look as well.
> + switch (c) {
> + case '=':
> + buf[buf_pos] = '\0';
> + for (i = 0; i < params_len; i++) {
> + if (strcmp(buf, params[i].key) == 0)
> + var_id = (int)i;
> + }
> +
> + buf_pos = 0;
> + break;
> + case ' ':
> + case '\n':
> + buf[buf_pos] = '\0';
> + if (var_id >= 0 && var_id < (int)params_len)
> + strcpy(params[var_id].value, buf);
> +
> + var_id = -1;
> + buf_pos = 0;
> + break;
> + default:
> + if (buf_pos + 1 >= sizeof(buf)) {
> + tst_res(TWARN, "Buffer overflowed while parsing /proc/cmdline");
> + while ((c = fgetc(f)) != EOF && c != ' ' && c != '\n');
> +
> + var_id = -1;
> + buf_pos = 0;
> + }
> +
> + buf[buf_pos++] = (char)c;
> + break;
> + }
> + }
LGTM.
Reviewed-by: Petr Vorel <pvorel@suse.cz>
BTW it would be great to add a test for this functionality (there are
already some tests in lib/newlib_tests/test_kconfig*.c).
Kind regards,
Petr
> +
> + SAFE_FCLOSE(f);
> +}
diff --git a/include/tst_kconfig.h b/include/tst_kconfig.h index 8b24a8380..a8cbfb786 100644 --- a/include/tst_kconfig.h +++ b/include/tst_kconfig.h @@ -64,4 +64,27 @@ 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 = "" \ +} + +/* Structure for storing command-line parameter key and its corresponding value */ +struct tst_kcmdline_var { + const char *key; + char value[128]; +}; + +/** + * 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/tst_kconfig.c b/lib/tst_kconfig.c index 595ea4b09..c200dd261 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,47 @@ 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]; + size_t buf_pos = 0, i; + int var_id = -1, c; + + FILE *f = SAFE_FOPEN("/proc/cmdline", "r"); + + while ((c = fgetc(f)) != EOF) { + switch (c) { + case '=': + buf[buf_pos] = '\0'; + for (i = 0; i < params_len; i++) { + if (strcmp(buf, params[i].key) == 0) + var_id = (int)i; + } + + buf_pos = 0; + break; + case ' ': + case '\n': + buf[buf_pos] = '\0'; + if (var_id >= 0 && var_id < (int)params_len) + strcpy(params[var_id].value, buf); + + var_id = -1; + buf_pos = 0; + break; + default: + if (buf_pos + 1 >= sizeof(buf)) { + tst_res(TWARN, "Buffer overflowed while parsing /proc/cmdline"); + while ((c = fgetc(f)) != EOF && c != ' ' && c != '\n'); + + var_id = -1; + buf_pos = 0; + } + + buf[buf_pos++] = (char)c; + break; + } + } + + 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 | 23 ++++++++++++++++++++++ lib/tst_kconfig.c | 45 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+)