| Message ID | 20240308045230.3106549-1-liwang@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/3] kconfig: add funtion to parse /proc/cmdline | expand |
Hi Li, > In tst_kconfig.c, it adds the tst_kcmdline_parse function to > read and parse command-line parameters from /proc/cmdline. > This function tokenizes the command line, matches keys with > the provided parameter array, and stores the associated values. > The update enhances the testing framework's ability to handle > kernel configuration parameters dynamically. > Signed-off-by: Li Wang <liwang@redhat.com> > --- > include/tst_kconfig.h | 23 +++++++++++++++++++++++ > lib/tst_kconfig.c | 41 +++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 64 insertions(+) > diff --git a/include/tst_kconfig.h b/include/tst_kconfig.h > index 8b24a8380..0ecf2c0d1 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_param 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_param { > + 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_param 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_param params[], size_t params_len); > + > #endif /* TST_KCONFIG_H__ */ > diff --git a/lib/tst_kconfig.c b/lib/tst_kconfig.c > index 595ea4b09..f79dea5c6 100644 > --- a/lib/tst_kconfig.c > +++ b/lib/tst_kconfig.c > @@ -565,3 +565,44 @@ char tst_kconfig_get(const char *confname) > return var.choice; > } > + > +void tst_kcmdline_parse(struct tst_kcmdline_param params[], size_t params_len) { > + FILE *proc_cmdline; > + char cmdline[4096]; > + char *token, *key, *value; > + > + proc_cmdline = fopen("/proc/cmdline", "r"); Why not SAFE_FOPEN() ? > + if (proc_cmdline == NULL) > + tst_brk(TBROK | TERRNO, "Failed to open /proc/cmdline for reading"); > + > + if (fgets(cmdline, sizeof(cmdline), proc_cmdline) == NULL) { > + fclose(proc_cmdline); Maybe SAFE_FCLOSE() ? > + > + if (feof(proc_cmdline)) tst_kconfig.c:581:21: warning: pointer ‘proc_cmdline’ used after ‘fclose’ [-Wuse-after-free] 581 | if (feof(proc_cmdline)) | ^~~~~~~~~~~~~~~~~~ tst_kconfig.c:579:17: note: call to ‘fclose’ here 579 | fclose(proc_cmdline); | ^~~~~~~~~~~~~~~~~~~~ > + tst_brk(TBROK, "End-of-File reached on /proc/cmdline without reading any data"); > + else > + tst_brk(TBROK | TERRNO, "Failed to read from /proc/cmdline"); > + } > + fclose(proc_cmdline); Maybe SAFE_FCLOSE() ? Kind regards, Petr > + > + token = strtok(cmdline, " "); > + while (token != NULL) { > + key = token; > + value = strchr(token, '='); > + > + if (value != NULL) { > + /* Split the token into key and value at '=' */ > + *value++ = '\0'; > + > + for (size_t i = 0; i < params_len; i++) { > + if (strcmp(params[i].key, key) == 0) { > + strncpy(params[i].value, value, sizeof(params[i].value) - 1); > + params[i].value[sizeof(params[i].value) - 1] = '\0'; > + break; > + } > + } > + } > + > + token = strtok(NULL, " "); > + } > +}
Hi! > +/** > + * Macro to initialize a tst_kcmdline_param 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_param { ^ maybe var as short for variable would be better name > + 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_param 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_param params[], size_t params_len); > + > #endif /* TST_KCONFIG_H__ */ > diff --git a/lib/tst_kconfig.c b/lib/tst_kconfig.c > index 595ea4b09..f79dea5c6 100644 > --- a/lib/tst_kconfig.c > +++ b/lib/tst_kconfig.c > @@ -565,3 +565,44 @@ char tst_kconfig_get(const char *confname) > > return var.choice; > } > + > +void tst_kcmdline_parse(struct tst_kcmdline_param params[], size_t params_len) { > + FILE *proc_cmdline; > + char cmdline[4096]; > + char *token, *key, *value; > + > + proc_cmdline = fopen("/proc/cmdline", "r"); > + if (proc_cmdline == NULL) > + tst_brk(TBROK | TERRNO, "Failed to open /proc/cmdline for reading"); > + > + if (fgets(cmdline, sizeof(cmdline), proc_cmdline) == NULL) { > + fclose(proc_cmdline); > + > + if (feof(proc_cmdline)) > + tst_brk(TBROK, "End-of-File reached on /proc/cmdline without reading any data"); > + else > + tst_brk(TBROK | TERRNO, "Failed to read from /proc/cmdline"); > + } > + fclose(proc_cmdline); Uff, this is ugly. We have FILE and then use it as if we had fd and read it as a whole. The whole point of FILE is that glibc manages the buffers and reads so that we can access it character by character without being slow. It would be way cleaner if we just read the file character by character building up the key and value while we do that. Something as (bevare untested): char buf[128]; size_t buf_pos = 0, i; int var_id = -1; f = fopen("/proc/cmdline", "r"); while ((c = fgetc(f)) != EOF) { switch (c) { case '=': buf[buf_pos] = 0; for (i = 0; i < vars_len; i++) var_id = i; break; case ' ': buf[buf_pos] = 0; if (var_id >= 0) strcpy(vars[var_id].val, buf); var_id = -1; break; default: if (buf_pos+1 >= sizeof(buf)) //warning? buf[buf_pos++] = c; break; } }
On Sat, Mar 9, 2024 at 4:46 AM Cyril Hrubis <chrubis@suse.cz> wrote: > Hi! > > +/** > > + * Macro to initialize a tst_kcmdline_param 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_param { > ^ > maybe var as short for variable would be better > name > > + 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_param 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_param params[], size_t > params_len); > > + > > #endif /* TST_KCONFIG_H__ */ > > diff --git a/lib/tst_kconfig.c b/lib/tst_kconfig.c > > index 595ea4b09..f79dea5c6 100644 > > --- a/lib/tst_kconfig.c > > +++ b/lib/tst_kconfig.c > > @@ -565,3 +565,44 @@ char tst_kconfig_get(const char *confname) > > > > return var.choice; > > } > > + > > +void tst_kcmdline_parse(struct tst_kcmdline_param params[], size_t > params_len) { > > + FILE *proc_cmdline; > > + char cmdline[4096]; > > + char *token, *key, *value; > > + > > + proc_cmdline = fopen("/proc/cmdline", "r"); > > + if (proc_cmdline == NULL) > > + tst_brk(TBROK | TERRNO, "Failed to open /proc/cmdline for > reading"); > > + > > + if (fgets(cmdline, sizeof(cmdline), proc_cmdline) == NULL) { > > + fclose(proc_cmdline); > > + > > + if (feof(proc_cmdline)) > > + tst_brk(TBROK, "End-of-File reached on > /proc/cmdline without reading any data"); > > + else > > + tst_brk(TBROK | TERRNO, "Failed to read from > /proc/cmdline"); > > + } > > + fclose(proc_cmdline); > > Uff, this is ugly. We have FILE and then use it as if we had fd and read > it as a whole. The whole point of FILE is that glibc manages the buffers > and reads so that we can access it character by character without being > slow. It would be way cleaner if we just read the file character by > character building up the key and value while we do that. > Indeed, this is much better than my way. Thanks!
diff --git a/include/tst_kconfig.h b/include/tst_kconfig.h index 8b24a8380..0ecf2c0d1 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_param 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_param { + 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_param 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_param params[], size_t params_len); + #endif /* TST_KCONFIG_H__ */ diff --git a/lib/tst_kconfig.c b/lib/tst_kconfig.c index 595ea4b09..f79dea5c6 100644 --- a/lib/tst_kconfig.c +++ b/lib/tst_kconfig.c @@ -565,3 +565,44 @@ char tst_kconfig_get(const char *confname) return var.choice; } + +void tst_kcmdline_parse(struct tst_kcmdline_param params[], size_t params_len) { + FILE *proc_cmdline; + char cmdline[4096]; + char *token, *key, *value; + + proc_cmdline = fopen("/proc/cmdline", "r"); + if (proc_cmdline == NULL) + tst_brk(TBROK | TERRNO, "Failed to open /proc/cmdline for reading"); + + if (fgets(cmdline, sizeof(cmdline), proc_cmdline) == NULL) { + fclose(proc_cmdline); + + if (feof(proc_cmdline)) + tst_brk(TBROK, "End-of-File reached on /proc/cmdline without reading any data"); + else + tst_brk(TBROK | TERRNO, "Failed to read from /proc/cmdline"); + } + fclose(proc_cmdline); + + token = strtok(cmdline, " "); + while (token != NULL) { + key = token; + value = strchr(token, '='); + + if (value != NULL) { + /* Split the token into key and value at '=' */ + *value++ = '\0'; + + for (size_t i = 0; i < params_len; i++) { + if (strcmp(params[i].key, key) == 0) { + strncpy(params[i].value, value, sizeof(params[i].value) - 1); + params[i].value[sizeof(params[i].value) - 1] = '\0'; + break; + } + } + } + + token = strtok(NULL, " "); + } +}
In tst_kconfig.c, it adds the tst_kcmdline_parse function to read and parse command-line parameters from /proc/cmdline. This function tokenizes the command line, matches keys with the provided parameter array, and stores the associated values. The update enhances the testing framework's ability to handle kernel configuration parameters dynamically. Signed-off-by: Li Wang <liwang@redhat.com> --- include/tst_kconfig.h | 23 +++++++++++++++++++++++ lib/tst_kconfig.c | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+)