utils/getopt: Add _r variants of the getopt APIs

Add sys_getopt_r, sys_getopt_long_r and sys_getopt_long_only_r variants of
the getopt APIs. This allows applications to explicitly control their
getopt state, rather than storing it in global and/or thread local
variables. The existing APIs remain available and operate as they did
before.

Signed-off-by: Keith Packard <[email protected]>

Author: keith-packard

doc/services/shell/index.rst

@@ -686,12 +686,12 @@ the arguments string, looking for supported options. Typically, this task
 is accomplished by the ``getopt`` family functions.
 
 For this purpose shell supports a variant of the getopt and getopt_long libraries from
-the FreeBSD project called sys_getopt and sys_getopt_long. This feature is activated by:
+the FreeBSD project called sys_getopt_r and sys_getopt_long_r. This feature is activated by:
 :kconfig:option:`CONFIG_GETOPT` set to ``y`` and :kconfig:option:`CONFIG_GETOPT_LONG`
 set to ``y``.
 
-This feature can be used in thread safe as well as non thread safe manner.
-The former is full compatible with regular getopt usage while the latter
+This feature can be used in non thread safe as well as thread safe manner.
+The former is fully compatible with regular getopt usage while the latter
 a bit differs.
 
 An example non-thread safe usage:
@@ -714,21 +714,17 @@ An example thread safe usage:
 .. code-block:: c
 
   char *cvalue = NULL;
-  struct sys_getopt_state *state;
-  while ((char c = sys_getopt(argc, argv, "abhc:")) != -1) {
-        state = sys_getopt_state_get();
+  struct sys_getopt_state state = SYS_GETOPT_STATE_INITIALIZER;
+  while ((char c = sys_getopt_r(argc, argv, "abhc:", &state)) != -1) {
         switch (c) {
         case 'c':
-                cvalue = state->optarg;
+                cvalue = state.optarg;
                 break;
         default:
                 break;
         }
   }
 
-Thread safe sys_getopt functionality is activated by
-:kconfig:option:`CONFIG_SHELL_GETOPT` set to ``y``.
-
 Obscured Input Feature
 **********************
 

include/zephyr/sys/sys_getopt.h

@@ -34,6 +34,9 @@ extern int sys_getopt_opterr;
 extern int sys_getopt_optind;
 extern int sys_getopt_optopt;
 
+/* Initialize local sys_getopt_state with this value */
+#define SYS_GETOPT_STATE_INITIALIZER	{0}
+
 #define sys_getopt_no_argument       0
 #define sys_getopt_required_argument 1
 #define sys_getopt_optional_argument 2
@@ -52,6 +55,9 @@ struct sys_getopt_option {
 	int val;
 };
 
+/* Function initializes getopt_state structure */
+void sys_getopt_init_r(struct sys_getopt_state *state);
+
 /* Function initializes getopt_state structure for current thread */
 void sys_getopt_init(void);
 
@@ -67,16 +73,19 @@ struct sys_getopt_state *sys_getopt_state_get(void);
  * @param[in] nargc	   Arguments count.
  * @param[in] nargv	   Arguments.
  * @param[in] ostr	   String containing the legitimate option characters.
+ * @param[inout] state	   Current argument parsing state.
  *
  * @return		If an option was successfully found, function returns
  *			the option character.
  */
+int sys_getopt_r(int nargc, char *const nargv[], const char *ostr, struct sys_getopt_state *state);
+
 int sys_getopt(int nargc, char *const nargv[], const char *ostr);
 
 /**
  * @brief Parses the command-line arguments.
  *
- * The sys_getopt_long() function works like @ref sys_getopt() except
+ * The sys_getopt_long_r() function works like @ref sys_getopt_r() except
  * it also accepts long options, started with two dashes.
  *
  * @note This function is based on FreeBSD implementation but it does not
@@ -90,17 +99,22 @@ int sys_getopt(int nargc, char *const nargv[], const char *ostr);
  * @param[in] idx	   If idx is not NULL, it points to a variable
  *			   which is set to the index of the long option relative
  *			   to @p long_options.
+ * @param[inout] state	   Current argument parsing state.
  *
  * @return		If an option was successfully found, function returns
  *			the option character.
  */
+int sys_getopt_long_r(int nargc, char *const *nargv, const char *options,
+		      const struct sys_getopt_option *long_options, int *idx,
+		      struct sys_getopt_state *state);
+
 int sys_getopt_long(int nargc, char *const *nargv, const char *options,
 		    const struct sys_getopt_option *long_options, int *idx);
 
 /**
  * @brief Parses the command-line arguments.
  *
- * The sys_getopt_long_only() function works like @ref sys_getopt_long(),
+ * The sys_getopt_long_only_r() function works like @ref sys_getopt_long_r(),
  * but '-' as well as "--" can indicate a long option. If an option that starts
  * with '-' (not "--") doesn't match a long option, but does match a short
  * option, it is parsed as a short option instead.
@@ -116,10 +130,15 @@ int sys_getopt_long(int nargc, char *const *nargv, const char *options,
  * @param[in] idx	   If idx is not NULL, it points to a variable
  *			   which is set to the index of the long option relative
  *			   to @p long_options.
+ * @param[inout] state	   Current argument parsing state.
  *
  * @return		If an option was successfully found, function returns
  *			the option character.
  */
+int sys_getopt_long_only_r(int nargc, char *const *nargv, const char *options,
+			   const struct sys_getopt_option *long_options, int *idx,
+			   struct sys_getopt_state *state);
+
 int sys_getopt_long_only(int nargc, char *const *nargv, const char *options,
 			 const struct sys_getopt_option *long_options, int *idx);
 

lib/utils/getopt/getopt.c

@@ -40,12 +40,8 @@ LOG_MODULE_REGISTER(sys_getopt);
 #define BADARG ((int)':')
 #define EMSG   ""
 
-void sys_getopt_init(void)
+void sys_getopt_init_r(struct sys_getopt_state *state)
 {
-	struct sys_getopt_state *state;
-
-	state = sys_getopt_state_get();
-
 	state->opterr = 1;
 	state->optind = 1;
 	state->optopt = 0;
@@ -58,41 +54,44 @@ void sys_getopt_init(void)
 	state->nonopt_start = -1; /* first non option argument (for permute) */
 	state->nonopt_end = -1;   /* first option after non options (for permute) */
 #endif
+}
 
-	sys_getopt_opterr = 1;
-	sys_getopt_optind = 1;
-	sys_getopt_optopt = 0;
-	sys_getopt_optreset = 0;
-	sys_getopt_optarg = NULL;
+void sys_getopt_init(void)
+{
+	struct sys_getopt_state *state;
+
+	state = sys_getopt_state_get();
+
+	sys_getopt_init_r(state);
+	z_getopt_global_state_update(state);
 }
 
 /*
  * getopt --
  *	Parse argc/argv argument vector.
  */
-int sys_getopt(int nargc, char *const nargv[], const char *ostr)
+int sys_getopt_r(int nargc, char *const nargv[], const char *ostr, struct sys_getopt_state *state)
 {
-	struct sys_getopt_state *state;
 	char *oli; /* option letter list index */
 
-	/* get getopt state of the current thread */
-	state = sys_getopt_state_get();
+	/* Reset if optind is 0 */
+	if (state->optind == 0) {
+		sys_getopt_init_r(state);
+	}
 
 	if (state->optreset || *state->place == 0) { /* update scanning pointer */
 		state->optreset = 0;
 		state->place = nargv[state->optind];
 		if (state->optind >= nargc || *state->place++ != '-') {
 			/* Argument is absent or is not an option */
 			state->place = EMSG;
-			z_getopt_global_state_update(state);
 			return -1;
 		}
 		state->optopt = *state->place++;
 		if (state->optopt == '-' && *state->place == 0) {
 			/* "--" => end of options */
 			++state->optind;
 			state->place = EMSG;
-			z_getopt_global_state_update(state);
 			return -1;
 		}
 		if (state->optopt == 0) {
@@ -101,7 +100,6 @@ int sys_getopt(int nargc, char *const nargv[], const char *ostr)
 			 */
 			state->place = EMSG;
 			if (strchr(ostr, '-') == NULL) {
-				z_getopt_global_state_update(state);
 				return -1;
 			}
 			state->optopt = '-';
@@ -119,7 +117,6 @@ int sys_getopt(int nargc, char *const nargv[], const char *ostr)
 		if (state->opterr && *ostr != ':') {
 			LOG_DBG("illegal option -- %c", state->optopt);
 		}
-		z_getopt_global_state_update(state);
 		return BADCH;
 	}
 
@@ -142,18 +139,32 @@ int sys_getopt(int nargc, char *const nargv[], const char *ostr)
 			/* option-argument absent */
 			state->place = EMSG;
 			if (*ostr == ':') {
-				z_getopt_global_state_update(state);
 				return BADARG;
 			}
 			if (state->opterr) {
 				LOG_DBG("option requires an argument -- %c", state->optopt);
 			}
-			z_getopt_global_state_update(state);
 			return BADCH;
 		}
 		state->place = EMSG;
 		++state->optind;
 	}
-	z_getopt_global_state_update(state);
 	return state->optopt; /* return option letter */
 }
+
+/*
+ * getopt --
+ *	Parse argc/argv argument vector.
+ */
+int sys_getopt(int nargc, char *const nargv[], const char *ostr)
+{
+	struct sys_getopt_state *state;
+	int ret;
+
+	/* Get state of the current thread */
+	state  = sys_getopt_state_get();
+	ret = sys_getopt_r(nargc, nargv, ostr, state);
+
+	z_getopt_global_state_update(state);
+	return ret;
+}

lib/utils/getopt/getopt_common.c

@@ -54,10 +54,6 @@ void z_getopt_global_state_update(struct sys_getopt_state *state)
 	sys_getopt_optopt = state->optopt;
 	sys_getopt_optreset = state->optreset;
 	sys_getopt_optarg = state->optarg;
-
-	if (!IS_ENABLED(CONFIG_NATIVE_LIBC) && IS_ENABLED(CONFIG_POSIX_C_LIB_EXT)) {
-		z_getopt_global_state_update_shim(state);
-	}
 }
 
 /* It is internal getopt API function, it shall not be called by the user. */

lib/utils/getopt/getopt_long.c

@@ -348,6 +348,11 @@ static int getopt_internal(struct sys_getopt_state *state, int nargc, char *cons
 		return -1;
 	}
 
+	/* Reset if optind is 0 */
+	if (state->optind == 0) {
+		sys_getopt_init_r(state);
+	}
+
 	/*
 	 * Disable GNU extensions if options string begins with a '+'.
 	 */
@@ -368,14 +373,6 @@ static int getopt_internal(struct sys_getopt_state *state, int nargc, char *cons
 		options++;
 	}
 
-	/*
-	 * XXX Some GNU programs (like cvs) set optind to 0 instead of
-	 * XXX using optreset.  Work around this braindamage.
-	 */
-	if (state->optind == 0) {
-		state->optind = state->optreset = 1;
-	}
-
 	state->optarg = NULL;
 	if (state->optreset) {
 		state->nonopt_start = state->nonopt_end = -1;
@@ -568,6 +565,13 @@ static int getopt_internal(struct sys_getopt_state *state, int nargc, char *cons
  * getopt_long --
  *	Parse argc/argv argument vector.
  */
+int sys_getopt_long_r(int nargc, char *const *nargv, const char *options,
+		      const struct sys_getopt_option *long_options, int *idx,
+		      struct sys_getopt_state *state)
+{
+	return getopt_internal(state, nargc, nargv, options, long_options, idx, FLAG_PERMUTE);
+}
+
 int sys_getopt_long(int nargc, char *const *nargv, const char *options,
 		    const struct sys_getopt_option *long_options, int *idx)
 {
@@ -588,6 +592,14 @@ int sys_getopt_long(int nargc, char *const *nargv, const char *options,
  * getopt_long_only --
  *	Parse argc/argv argument vector.
  */
+int sys_getopt_long_only_r(int nargc, char *const *nargv, const char *options,
+			   const struct sys_getopt_option *long_options, int *idx,
+			   struct sys_getopt_state *state)
+{
+	return getopt_internal(state, nargc, nargv, options, long_options, idx,
+			       FLAG_PERMUTE | FLAG_LONGONLY);
+}
+
 int sys_getopt_long_only(int nargc, char *const *nargv, const char *options,
 			 const struct sys_getopt_option *long_options, int *idx)
 {