From 6c507730bd21e71dda15b70ac3e4e053be49e0cd Mon Sep 17 00:00:00 2001
From: Gregory Nutt <gnutt@nuttx.org>
Date: Sat, 3 Apr 2021 07:08:44 -0600
Subject: [PATCH] Fix Use of Variable before NULL check

Found a place in getopt_common() where the option string is used before it is checked if it is NULL.  This can happen because the short option string is optional for getopt_long() and getopt_long_only()

If optstring is NULL, that would be an ERROR for getopt(), but not for the getopt_long() versions.

Should effect only the getopt() APIs

Tested on the simulator using apps/testing/ostest.
---
 libs/libc/unistd/lib_getopt_common.c | 52 ++++++++++++++++++++--------
 1 file changed, 37 insertions(+), 15 deletions(-)

diff --git a/libs/libc/unistd/lib_getopt_common.c b/libs/libc/unistd/lib_getopt_common.c
index 86d21592dc..c1a5d6b909 100644
--- a/libs/libc/unistd/lib_getopt_common.c
+++ b/libs/libc/unistd/lib_getopt_common.c
@@ -281,16 +281,6 @@ int getopt_common(int argc, FAR char * const argv[],
           go->go_binitialized = true;  /* Now we are initialized */
         }
 
-      /* If the first character of opstring s ':', then ':' is in the event
-       * of a missing argument. Otherwise '?' is returned.
-       */
-
-      if (*optstring == ':')
-        {
-           noarg_ret = ':';
-           optstring++;
-        }
-
       /* Are we resuming in the middle, or at the end of a string of
        * arguments? optptr == NULL means that we are started at the
        * beginning of argv[optind]; *optptr == \0 means that we are
@@ -426,9 +416,43 @@ int getopt_common(int argc, FAR char * const argv[],
 
       if (optstring == NULL)
         {
-          goto errout;
+          /* Not an error with getopt_long() */
+
+          if (GETOPT_HAVE_LONG(mode))
+            {
+              /* Return '?'.  optptr is reset to the next argv entry,
+               * discarding everything else that follows in the argv string
+               * (which could be another single character command).
+               */
+
+              go->go_optopt = *go->go_optptr;
+              go->go_optptr = NULL;
+              go->go_optind++;
+              return '?';
+            }
+          else
+            {
+              /* Restore the initial, uninitialized state, and return an
+               * error.
+               */
+
+              go->go_binitialized = false;
+              return ERROR;
+            }
         }
 
+      /* If the first character of opstring s ':', then ':' is in the event
+       * of a missing argument. Otherwise '?' is returned.
+       */
+
+      if (*optstring == ':')
+        {
+          noarg_ret = ':';
+          optstring++;
+        }
+
+      /* Check if the option appears in 'optstring' */
+
       optchar = strchr(optstring, *go->go_optptr);
       if (!optchar)
         {
@@ -439,7 +463,7 @@ int getopt_common(int argc, FAR char * const argv[],
           return '?';
         }
 
-      /* Yes, the character is in the list of valid options.  Does it have an
+      /* Yes, the character is in the list of valid options.  Does it have a
        * required argument?
        */
 
@@ -489,9 +513,7 @@ int getopt_common(int argc, FAR char * const argv[],
       return (optchar[2] == ':') ? *optchar : noarg_ret;
     }
 
-errout:
-
-  /* Restore the initial, uninitialized state */
+  /* Restore the initial, uninitialized state, and return an error. */
 
   go->go_binitialized = false;
   return ERROR;