Add sys_getopt_r, sys_getopt_long_r and sys_getopt_long_only_r

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/posix/c_lib_ext/getopt/getopt.h

@@ -13,17 +13,12 @@
 extern "C" {
 #endif
 
-extern void getopt_init(void);
-
-#define getopt_state sys_getopt_state
 #define option sys_getopt_option
 
 #define no_argument       0
 #define required_argument 1
 #define optional_argument 2
 
-extern struct sys_getopt_state *getopt_state_get(void);
-
 extern int getopt_long(int nargc, char *const *nargv, const char *options,
 		       const struct option *long_options, int *idx);
 

lib/posix/c_lib_ext/getopt_shim.c

@@ -11,37 +11,52 @@
 char *optarg;
 int opterr, optind, optopt;
 
-void getopt_init(void)
+static struct sys_getopt_state	getopt_state = SYS_GETOPT_STATE_INITIALIZER;
+
+static void getopt_global_state_get(void)
 {
-	sys_getopt_init();
+	getopt_state.opterr = opterr;
+	getopt_state.optind = optind;
+	getopt_state.optopt = optopt;
+	getopt_state.optarg = optarg;
 }
 
-struct getopt_state *getopt_state_get(void)
+static void getopt_global_state_put(void)
 {
-	return sys_getopt_state_get();
+	opterr = getopt_state.opterr;
+	optind = getopt_state.optind;
+	optopt = getopt_state.optopt;
+	optarg = getopt_state.optarg;
 }
 
 int getopt(int argc, char *const argv[], const char *optstring)
 {
-	return sys_getopt(argc, argv, optstring);
-}
+	int ret;
 
-void z_getopt_global_state_update_shim(struct sys_getopt_state *state)
-{
-	opterr = state->opterr;
-	optind = state->optind;
-	optopt = state->optopt;
-	optarg = state->optarg;
+	getopt_global_state_get();
+	ret = sys_getopt_r(argc, argv, optstring, &getopt_state);
+	getopt_global_state_put();
+	return ret;
 }
 
 int getopt_long(int argc, char *const argv[], const char *shortopts,
 		const struct option *longopts, int *longind)
 {
-	return sys_getopt_long(argc, argv, shortopts, longopts, longind);
+	int ret;
+
+	getopt_global_state_get();
+	ret = sys_getopt_long_r(argc, argv, shortopts, longopts, longind, &getopt_state);
+	getopt_global_state_put();
+	return ret;
 }
 
 int getopt_long_only(int argc, char *const argv[], const char *shortopts,
 		     const struct option *longopts, int *longind)
 {
-	return sys_getopt_long_only(argc, argv, shortopts, longopts, longind);
+	int ret;
+
+	getopt_global_state_get();
+	ret = sys_getopt_long_only_r(argc, argv, shortopts, longopts, longind, &getopt_state);
+	getopt_global_state_put();
+	return ret;
 }

lib/posix/shell/uname.c

@@ -42,7 +42,7 @@ static void uname_print_usage(const struct shell *sh)
 
 static int uname_cmd_handler(const struct shell *sh, size_t argc, char **argv)
 {
-	struct sys_getopt_state *state = sys_getopt_state_get();
+	struct sys_getopt_state state = SYS_GETOPT_STATE_INITIALIZER;
 	struct utsname info;
 	unsigned int set;
 	int option;
@@ -53,8 +53,7 @@ static int uname_cmd_handler(const struct shell *sh, size_t argc, char **argv)
 
 	/* Get the uname options */
 
-	optind = 1;
-	while ((option = sys_getopt(argc, argv, "asonrvmpi")) != -1) {
+	while ((option = sys_getopt_r(argc, argv, "asonrvmpi", &state)) != -1) {
 		switch (option) {
 		case 'a':
 			set = UNAME_ALL;
@@ -93,13 +92,13 @@ static int uname_cmd_handler(const struct shell *sh, size_t argc, char **argv)
 
 		case '?':
 		default:
-			badarg = (char)state->optopt;
+			badarg = (char)state.optopt;
 			break;
 		}
 	}
 
-	if (argc != optind) {
-		shell_error(sh, "uname: extra operand %s", argv[optind]);
+	if (argc != state.optind) {
+		shell_error(sh, "uname: extra operand %s", argv[state.optind]);
 		uname_print_usage(sh);
 		return -1;
 	}

lib/utils/getopt/getopt.c

@@ -1,4 +1,4 @@
 /*	$NetBSD: getopt.c,v 1.26 2003/08/07 16:43:40 agc Exp $	*/
 /* SPDX-License-Identifier: BSD-3-Clause */
 /*
  * Copyright (c) 1987, 1993, 1994
@@ -40,12 +40,8 @@
 #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 @@
 	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 @@
 			 */
 			state->place = EMSG;
 			if (strchr(ostr, '-') == NULL) {
-				z_getopt_global_state_update(state);
 				return -1;
 			}
 			state->optopt = '-';
@@ -119,7 +117,6 @@
 		if (state->opterr && *ostr != ':') {
 			LOG_DBG("illegal option -- %c", state->optopt);
 		}
-		z_getopt_global_state_update(state);
 		return BADCH;
 	}
 
@@ -142,18 +139,32 @@
 			/* 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

@@ -38,12 +38,6 @@ static struct sys_getopt_state m_getopt_common_state = {
 #endif
 };
 
-/* Shim function to update global variables in getopt_shim.c if user wants to
- * still use the original non-posix compliant getopt. The shim will be deprecated
- * and eventually removed in the future.
- */
-extern void z_getopt_global_state_update_shim(struct sys_getopt_state *state);
-
 /* This function is not thread safe. All threads using getopt are calling
  * this function.
  */
@@ -54,10 +48,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. */