All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lucas De Marchi <lucas.demarchi@intel.com>
To: linux-modules@vger.kernel.org
Cc: Jiri Slaby <jirislaby@kernel.org>, Jessica Yu <jeyu@kernel.org>,
	Giovanni Gherdovich <ggherdovich@suse.cz>
Subject: [PATCH 1/4] libkmod-config: revamp kcmdline parsing into a state machine
Date: Fri, 12 Feb 2021 01:45:21 -0800	[thread overview]
Message-ID: <20210212094524.170861-1-lucas.demarchi@intel.com> (raw)

The handling of spaces and quotes is becoming hard to maintain. Convert
the parser into a state machine so we can check all the states. This
should make it easier to fix a corner case we have right now:
The kernel also accepts a quote before the module name instead of the
value. But this additional is left for later. This is purely an
algorithm change with no behavior change.
---
 libkmod/libkmod-config.c | 86 ++++++++++++++++++++++++----------------
 1 file changed, 52 insertions(+), 34 deletions(-)

diff --git a/libkmod/libkmod-config.c b/libkmod/libkmod-config.c
index 971f20b..d3cd10d 100644
--- a/libkmod/libkmod-config.c
+++ b/libkmod/libkmod-config.c
@@ -499,7 +499,14 @@ static int kmod_config_parse_kcmdline(struct kmod_config *config)
 	char buf[KCMD_LINE_SIZE];
 	int fd, err;
 	char *p, *modname,  *param = NULL, *value = NULL;
-	bool is_quoted = false, is_module = true;
+	bool is_quoted = false, iter = true;
+	enum state {
+		STATE_IGNORE,
+		STATE_MODNAME,
+		STATE_PARAM,
+		STATE_VALUE,
+		STATE_COMPLETE,
+	} state;
 
 	fd = open("/proc/cmdline", O_RDONLY|O_CLOEXEC);
 	if (fd < 0) {
@@ -516,54 +523,65 @@ static int kmod_config_parse_kcmdline(struct kmod_config *config)
 		return err;
 	}
 
-	for (p = buf, modname = buf; *p != '\0' && *p != '\n'; p++) {
-		if (*p == '"') {
+	state = STATE_MODNAME;
+	for (p = buf, modname = buf; iter; p++) {
+		switch (*p) {
+		case '"':
 			is_quoted = !is_quoted;
-
-			if (is_quoted) {
-				/* don't consider a module until closing quotes */
-				is_module = false;
-			} else if (param != NULL && value != NULL) {
+			break;
+		case '\0':
+		case '\n':
+			/* Stop iterating on new chars */
+			iter = false;
+			/* fall-through */
+		case ' ':
+			if (is_quoted && state == STATE_VALUE) {
+				/* no state change*/;
+			} else if (is_quoted) {
+				/* spaces are only allowed in the value part */
+				state = STATE_IGNORE;
+			} else if (state == STATE_VALUE || state == STATE_PARAM) {
+				*p = '\0';
+				state = STATE_COMPLETE;
+			} else {
 				/*
-				 * If we are indeed expecting a value and
-				 * closing quotes, then this can be considered
-				 * a valid option for a module
+				 * go to next option, ignoring any possible
+				 * partial match we have
 				 */
-				is_module = true;
+				modname = p + 1;
+				state = STATE_MODNAME;
 			}
-
-			continue;
-		}
-		if (is_quoted)
-			continue;
-
-		switch (*p) {
-		case ' ':
-			*p = '\0';
-			if (is_module)
-				kcmdline_parse_result(config, modname, param, value);
-			param = value = NULL;
-			modname = p + 1;
-			is_module = true;
 			break;
 		case '.':
-			if (param == NULL) {
+			if (state == STATE_MODNAME) {
 				*p = '\0';
 				param = p + 1;
+				state = STATE_PARAM;
+			} else if (state == STATE_PARAM) {
+				state = STATE_IGNORE;
 			}
 			break;
 		case '=':
-			if (param != NULL)
+			if (state == STATE_PARAM) {
+				/*
+				 * Don't set *p to '\0': the value var shadows
+				 * param
+				 */
 				value = p + 1;
-			else
-				is_module = false;
+				state = STATE_VALUE;
+			} else if (state == STATE_MODNAME) {
+				state = STATE_IGNORE;
+			}
 			break;
 		}
-	}
 
-	*p = '\0';
-	if (is_module)
-		kcmdline_parse_result(config, modname, param, value);
+		if (state == STATE_COMPLETE) {
+			kcmdline_parse_result(config, modname, param, value);
+			/* start over on next iteration */
+			modname = p + 1;
+			state = STATE_MODNAME;
+		}
+	}
 
 	return 0;
 }
-- 
2.30.0


             reply	other threads:[~2021-02-12  9:46 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-12  9:45 Lucas De Marchi [this message]
2021-02-12  9:45 ` [PATCH 2/4] libkmod-config: re-quote option from kernel cmdline Lucas De Marchi
2021-02-15 15:28   ` Jessica Yu
2021-02-15 20:18     ` Lucas De Marchi
2021-02-12  9:45 ` [PATCH 3/4] testsuite: allow to re-use single function for tests Lucas De Marchi
2021-02-12  9:45 ` [PATCH 4/4] test-modprobe: share single function for kcmdline tests Lucas De Marchi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210212094524.170861-1-lucas.demarchi@intel.com \
    --to=lucas.demarchi@intel.com \
    --cc=ggherdovich@suse.cz \
    --cc=jeyu@kernel.org \
    --cc=jirislaby@kernel.org \
    --cc=linux-modules@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.