You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/06/10 09:00:15 UTC

[GitHub] [incubator-nuttx-apps] jlaitine opened a new pull request, #1194: nshlib/nsh_envcmd.c: Change cmd_unset to only unset existing env vari…

jlaitine opened a new pull request, #1194:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1194

   …ables
   
   Correct the problem, where "unset" always gives an error message if the
   variable is not exported (promoted to group-wide environment variable)
   
   This fixes the issue by unsetting the exported env variable only if it exists.
   
   This seems coherent with the way how cmd_set and cmd_export work currently.
   
   Signed-off-by: Jukka Laitinen <ju...@ssrc.tii.ae>
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx-apps] jlaitine commented on a diff in pull request #1194: nshlib/nsh_envcmd.c: Change cmd_unset to only unset existing env vari…

Posted by GitBox <gi...@apache.org>.
jlaitine commented on code in PR #1194:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1194#discussion_r894412003


##########
nshlib/nsh_envcmds.c:
##########
@@ -533,29 +533,45 @@ int cmd_unset(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
 {
 #if defined(CONFIG_NSH_VARS) || !defined(CONFIG_DISABLE_ENVIRON)
   int status;
+  FAR char *oldvalue;

Review Comment:
   this is inside wrong ifdefs. should be:
   #if defined(CONFIG_NSH_VARS) && !defined(CONFIG_DISABLE_ENVIRON)
   
   will fix this



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx-apps] jlaitine commented on pull request #1194: nshlib/nsh_envcmd.c: Change cmd_unset to only unset existing env vari…

Posted by GitBox <gi...@apache.org>.
jlaitine commented on PR #1194:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1194#issuecomment-1153485498

   Closing in favor of better solution from @xiaoxiang781216 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx-apps] jlaitine closed pull request #1194: nshlib/nsh_envcmd.c: Change cmd_unset to only unset existing env vari…

Posted by GitBox <gi...@apache.org>.
jlaitine closed pull request #1194: nshlib/nsh_envcmd.c: Change cmd_unset to only unset existing env vari…
URL: https://github.com/apache/incubator-nuttx-apps/pull/1194


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx-apps] jlaitine commented on pull request #1194: nshlib/nsh_envcmd.c: Change cmd_unset to only unset existing env vari…

Posted by GitBox <gi...@apache.org>.
jlaitine commented on PR #1194:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1194#issuecomment-1153824111

   Just one more comment on this; unsetenv is a posix interface, which should return "success" if the environment variable is not set (i.e. it already doesn't exist).
   
   So Implementation of "unsetenv" has been broken at some point in NuttX, since it now return error if unsetting an non-existing variable
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a diff in pull request #1194: nshlib/nsh_envcmd.c: Change cmd_unset to only unset existing env vari…

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #1194:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1194#discussion_r894997128


##########
nshlib/nsh_envcmds.c:
##########
@@ -533,29 +533,49 @@ int cmd_unset(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
 {
 #if defined(CONFIG_NSH_VARS) || !defined(CONFIG_DISABLE_ENVIRON)
   int status;
+#endif
+#if defined(CONFIG_NSH_VARS) && !defined(CONFIG_DISABLE_ENVIRON)
+  FAR char *oldvalue;
 #endif
   int ret = OK;
 
 #if defined(CONFIG_NSH_VARS)
-  /* Unset NSH variable */
+#ifndef CONFIG_DISABLE_ENVIRON
+  /* Check if the NSH variable has already been promoted to an group-
+   * wide environment variable. In this case, remove the exported
+   * variable
+   */
 
-  status = nsh_unsetvar(vtbl, argv[1]);
-  if (status < 0 && status != -ENOENT)
+  oldvalue = getenv(argv[1]);
+  if (oldvalue == NULL)
+#endif
     {
-      nsh_error(vtbl, g_fmtcmdfailed, argv[0], "nsh_unsetvar",
-                NSH_ERRNO_OF(-status));
-      ret = ERROR;
+      /* Unset NSH variable */
+
+      status = nsh_unsetvar(vtbl, argv[1]);
+      if (status < 0 && status != -ENOENT)
+        {
+          nsh_error(vtbl, g_fmtcmdfailed, argv[0], "nsh_unsetvar",
+                    NSH_ERRNO_OF(-status));
+          ret = ERROR;
+        }
     }
 #endif
 
 #if !defined(CONFIG_DISABLE_ENVIRON)
-  /* Unset environment variable */
-
-  status = unsetenv(argv[1]);

Review Comment:
   Event so, We can simplify the code like this:
   ```
   status = nsh_unsetvar(vtbl, argv[1]);
   #if !defined(CONFIG_DISABLE_ENVIRON)
   if (status == -ENOENT)
     {
        status = unsetenv(argv[1]);
         if (status < 0)
           {
             nsh_error(vtbl, g_fmtcmdfailed, argv[0], "unsetenv",
                       NSH_ERRNO);
             ret = ERROR;
           }
     }
   else 
   #endif
   if (status < 0)
     {
         nsh_error(vtbl, g_fmtcmdfailed, argv[0], "nsh_unsetvar",
                   NSH_ERRNO_OF(-status));
         ret = ERROR;
     }
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a diff in pull request #1194: nshlib/nsh_envcmd.c: Change cmd_unset to only unset existing env vari…

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #1194:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1194#discussion_r894798569


##########
nshlib/nsh_envcmds.c:
##########
@@ -533,29 +533,49 @@ int cmd_unset(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
 {
 #if defined(CONFIG_NSH_VARS) || !defined(CONFIG_DISABLE_ENVIRON)
   int status;
+#endif
+#if defined(CONFIG_NSH_VARS) && !defined(CONFIG_DISABLE_ENVIRON)
+  FAR char *oldvalue;
 #endif
   int ret = OK;
 
 #if defined(CONFIG_NSH_VARS)
-  /* Unset NSH variable */
+#ifndef CONFIG_DISABLE_ENVIRON
+  /* Check if the NSH variable has already been promoted to an group-
+   * wide environment variable. In this case, remove the exported
+   * variable
+   */
 
-  status = nsh_unsetvar(vtbl, argv[1]);
-  if (status < 0 && status != -ENOENT)
+  oldvalue = getenv(argv[1]);
+  if (oldvalue == NULL)
+#endif
     {
-      nsh_error(vtbl, g_fmtcmdfailed, argv[0], "nsh_unsetvar",
-                NSH_ERRNO_OF(-status));
-      ret = ERROR;
+      /* Unset NSH variable */
+
+      status = nsh_unsetvar(vtbl, argv[1]);
+      if (status < 0 && status != -ENOENT)
+        {
+          nsh_error(vtbl, g_fmtcmdfailed, argv[0], "nsh_unsetvar",
+                    NSH_ERRNO_OF(-status));
+          ret = ERROR;
+        }
     }
 #endif
 
 #if !defined(CONFIG_DISABLE_ENVIRON)
-  /* Unset environment variable */
-
-  status = unsetenv(argv[1]);

Review Comment:
   it's more simple to skip the error message if  errno set by unsetenv equals ENOENT.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a diff in pull request #1194: nshlib/nsh_envcmd.c: Change cmd_unset to only unset existing env vari…

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #1194:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1194#discussion_r894997128


##########
nshlib/nsh_envcmds.c:
##########
@@ -533,29 +533,49 @@ int cmd_unset(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
 {
 #if defined(CONFIG_NSH_VARS) || !defined(CONFIG_DISABLE_ENVIRON)
   int status;
+#endif
+#if defined(CONFIG_NSH_VARS) && !defined(CONFIG_DISABLE_ENVIRON)
+  FAR char *oldvalue;
 #endif
   int ret = OK;
 
 #if defined(CONFIG_NSH_VARS)
-  /* Unset NSH variable */
+#ifndef CONFIG_DISABLE_ENVIRON
+  /* Check if the NSH variable has already been promoted to an group-
+   * wide environment variable. In this case, remove the exported
+   * variable
+   */
 
-  status = nsh_unsetvar(vtbl, argv[1]);
-  if (status < 0 && status != -ENOENT)
+  oldvalue = getenv(argv[1]);
+  if (oldvalue == NULL)
+#endif
     {
-      nsh_error(vtbl, g_fmtcmdfailed, argv[0], "nsh_unsetvar",
-                NSH_ERRNO_OF(-status));
-      ret = ERROR;
+      /* Unset NSH variable */
+
+      status = nsh_unsetvar(vtbl, argv[1]);
+      if (status < 0 && status != -ENOENT)
+        {
+          nsh_error(vtbl, g_fmtcmdfailed, argv[0], "nsh_unsetvar",
+                    NSH_ERRNO_OF(-status));
+          ret = ERROR;
+        }
     }
 #endif
 
 #if !defined(CONFIG_DISABLE_ENVIRON)
-  /* Unset environment variable */
-
-  status = unsetenv(argv[1]);

Review Comment:
   Event so, We can simplify the code like this:
   ```
   status = nsh_unsetvar(vtbl, argv[1]);
   #if !defined(CONFIG_DISABLE_ENVIRON)
   if (status == -ENOENT)
     {
        status = unsetenv(argv[1]);
         if (status < 0)
           {
             nsh_error(vtbl, g_fmtcmdfailed, argv[0], "unsetenv",
                       NSH_ERRNO);
             ret = ERROR;
           }
     }
   else if (status < 0)
     {
         nsh_error(vtbl, g_fmtcmdfailed, argv[0], "nsh_unsetvar",
                   NSH_ERRNO_OF(-status));
         ret = ERROR;
     }
   #endif
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a diff in pull request #1194: nshlib/nsh_envcmd.c: Change cmd_unset to only unset existing env vari…

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #1194:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1194#discussion_r894393603


##########
nshlib/nsh_envcmds.c:
##########
@@ -533,30 +533,46 @@ int cmd_unset(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
 {
 #if defined(CONFIG_NSH_VARS) || !defined(CONFIG_DISABLE_ENVIRON)
   int status;
+  FAR char *oldvalue;
 #endif
   int ret = OK;
 
 #if defined(CONFIG_NSH_VARS)
-  /* Unset NSH variable */
+#ifndef CONFIG_DISABLE_ENVIRON
+      /* Check if the NSH variable has already been promoted to an group-

Review Comment:
   why make extra indent?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a diff in pull request #1194: nshlib/nsh_envcmd.c: Change cmd_unset to only unset existing env vari…

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #1194:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1194#discussion_r894798569


##########
nshlib/nsh_envcmds.c:
##########
@@ -533,29 +533,49 @@ int cmd_unset(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
 {
 #if defined(CONFIG_NSH_VARS) || !defined(CONFIG_DISABLE_ENVIRON)
   int status;
+#endif
+#if defined(CONFIG_NSH_VARS) && !defined(CONFIG_DISABLE_ENVIRON)
+  FAR char *oldvalue;
 #endif
   int ret = OK;
 
 #if defined(CONFIG_NSH_VARS)
-  /* Unset NSH variable */
+#ifndef CONFIG_DISABLE_ENVIRON
+  /* Check if the NSH variable has already been promoted to an group-
+   * wide environment variable. In this case, remove the exported
+   * variable
+   */
 
-  status = nsh_unsetvar(vtbl, argv[1]);
-  if (status < 0 && status != -ENOENT)
+  oldvalue = getenv(argv[1]);
+  if (oldvalue == NULL)
+#endif
     {
-      nsh_error(vtbl, g_fmtcmdfailed, argv[0], "nsh_unsetvar",
-                NSH_ERRNO_OF(-status));
-      ret = ERROR;
+      /* Unset NSH variable */
+
+      status = nsh_unsetvar(vtbl, argv[1]);
+      if (status < 0 && status != -ENOENT)
+        {
+          nsh_error(vtbl, g_fmtcmdfailed, argv[0], "nsh_unsetvar",
+                    NSH_ERRNO_OF(-status));
+          ret = ERROR;
+        }
     }
 #endif
 
 #if !defined(CONFIG_DISABLE_ENVIRON)
-  /* Unset environment variable */
-
-  status = unsetenv(argv[1]);

Review Comment:
   it's more simple to skip the error message if  errno set by unsetenv equals ENOENT like what have be done for nsh_unsetvar.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx-apps] jlaitine commented on a diff in pull request #1194: nshlib/nsh_envcmd.c: Change cmd_unset to only unset existing env vari…

Posted by GitBox <gi...@apache.org>.
jlaitine commented on code in PR #1194:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1194#discussion_r895020166


##########
nshlib/nsh_envcmds.c:
##########
@@ -533,29 +533,49 @@ int cmd_unset(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
 {
 #if defined(CONFIG_NSH_VARS) || !defined(CONFIG_DISABLE_ENVIRON)
   int status;
+#endif
+#if defined(CONFIG_NSH_VARS) && !defined(CONFIG_DISABLE_ENVIRON)
+  FAR char *oldvalue;
 #endif
   int ret = OK;
 
 #if defined(CONFIG_NSH_VARS)
-  /* Unset NSH variable */
+#ifndef CONFIG_DISABLE_ENVIRON
+  /* Check if the NSH variable has already been promoted to an group-
+   * wide environment variable. In this case, remove the exported
+   * variable
+   */
 
-  status = nsh_unsetvar(vtbl, argv[1]);
-  if (status < 0 && status != -ENOENT)
+  oldvalue = getenv(argv[1]);
+  if (oldvalue == NULL)
+#endif
     {
-      nsh_error(vtbl, g_fmtcmdfailed, argv[0], "nsh_unsetvar",
-                NSH_ERRNO_OF(-status));
-      ret = ERROR;
+      /* Unset NSH variable */
+
+      status = nsh_unsetvar(vtbl, argv[1]);
+      if (status < 0 && status != -ENOENT)
+        {
+          nsh_error(vtbl, g_fmtcmdfailed, argv[0], "nsh_unsetvar",
+                    NSH_ERRNO_OF(-status));
+          ret = ERROR;
+        }
     }
 #endif
 
 #if !defined(CONFIG_DISABLE_ENVIRON)
-  /* Unset environment variable */
-
-  status = unsetenv(argv[1]);

Review Comment:
   Looks good to me, go ahead and make the PR, as you have the gode ready.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx-apps] jlaitine commented on a diff in pull request #1194: nshlib/nsh_envcmd.c: Change cmd_unset to only unset existing env vari…

Posted by GitBox <gi...@apache.org>.
jlaitine commented on code in PR #1194:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1194#discussion_r894995689


##########
nshlib/nsh_envcmds.c:
##########
@@ -533,29 +533,49 @@ int cmd_unset(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
 {
 #if defined(CONFIG_NSH_VARS) || !defined(CONFIG_DISABLE_ENVIRON)
   int status;
+#endif
+#if defined(CONFIG_NSH_VARS) && !defined(CONFIG_DISABLE_ENVIRON)
+  FAR char *oldvalue;
 #endif
   int ret = OK;
 
 #if defined(CONFIG_NSH_VARS)
-  /* Unset NSH variable */
+#ifndef CONFIG_DISABLE_ENVIRON
+  /* Check if the NSH variable has already been promoted to an group-
+   * wide environment variable. In this case, remove the exported
+   * variable
+   */
 
-  status = nsh_unsetvar(vtbl, argv[1]);
-  if (status < 0 && status != -ENOENT)
+  oldvalue = getenv(argv[1]);
+  if (oldvalue == NULL)
+#endif
     {
-      nsh_error(vtbl, g_fmtcmdfailed, argv[0], "nsh_unsetvar",
-                NSH_ERRNO_OF(-status));
-      ret = ERROR;
+      /* Unset NSH variable */
+
+      status = nsh_unsetvar(vtbl, argv[1]);
+      if (status < 0 && status != -ENOENT)
+        {
+          nsh_error(vtbl, g_fmtcmdfailed, argv[0], "nsh_unsetvar",
+                    NSH_ERRNO_OF(-status));
+          ret = ERROR;
+        }
     }
 #endif
 
 #if !defined(CONFIG_DISABLE_ENVIRON)
-  /* Unset environment variable */
-
-  status = unsetenv(argv[1]);

Review Comment:
   Agreed. Maybe we can assume that the variable doesn't exist in both nsh_vars and environment variables (i.e. Local variable can't shadow an environment variable). Since in that case the implementation would be wrong; unsetting a local variable (in script) would also delete the global one. This was the thinking behind, why I wrote it like that. But I can change it as you suggested for sure!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx-apps] jlaitine commented on a diff in pull request #1194: nshlib/nsh_envcmd.c: Change cmd_unset to only unset existing env vari…

Posted by GitBox <gi...@apache.org>.
jlaitine commented on code in PR #1194:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1194#discussion_r894995689


##########
nshlib/nsh_envcmds.c:
##########
@@ -533,29 +533,49 @@ int cmd_unset(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
 {
 #if defined(CONFIG_NSH_VARS) || !defined(CONFIG_DISABLE_ENVIRON)
   int status;
+#endif
+#if defined(CONFIG_NSH_VARS) && !defined(CONFIG_DISABLE_ENVIRON)
+  FAR char *oldvalue;
 #endif
   int ret = OK;
 
 #if defined(CONFIG_NSH_VARS)
-  /* Unset NSH variable */
+#ifndef CONFIG_DISABLE_ENVIRON
+  /* Check if the NSH variable has already been promoted to an group-
+   * wide environment variable. In this case, remove the exported
+   * variable
+   */
 
-  status = nsh_unsetvar(vtbl, argv[1]);
-  if (status < 0 && status != -ENOENT)
+  oldvalue = getenv(argv[1]);
+  if (oldvalue == NULL)
+#endif
     {
-      nsh_error(vtbl, g_fmtcmdfailed, argv[0], "nsh_unsetvar",
-                NSH_ERRNO_OF(-status));
-      ret = ERROR;
+      /* Unset NSH variable */
+
+      status = nsh_unsetvar(vtbl, argv[1]);
+      if (status < 0 && status != -ENOENT)
+        {
+          nsh_error(vtbl, g_fmtcmdfailed, argv[0], "nsh_unsetvar",
+                    NSH_ERRNO_OF(-status));
+          ret = ERROR;
+        }
     }
 #endif
 
 #if !defined(CONFIG_DISABLE_ENVIRON)
-  /* Unset environment variable */
-
-  status = unsetenv(argv[1]);

Review Comment:
   Agreed. Maybe wr can assume that the variable doesn't exist in both nsh_vars and environment variables (i.e. Local variable can't shadow an environment variable). Since in that case the implementation would be wrong; unsetting a local variable (in script) would also delete the global one. This was the thinking behind, why I wrote it like that. But I can change it as you suggested for sure!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx-apps] jlaitine commented on a diff in pull request #1194: nshlib/nsh_envcmd.c: Change cmd_unset to only unset existing env vari…

Posted by GitBox <gi...@apache.org>.
jlaitine commented on code in PR #1194:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1194#discussion_r894397551


##########
nshlib/nsh_envcmds.c:
##########
@@ -533,30 +533,46 @@ int cmd_unset(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
 {
 #if defined(CONFIG_NSH_VARS) || !defined(CONFIG_DISABLE_ENVIRON)
   int status;
+  FAR char *oldvalue;
 #endif
   int ret = OK;
 
 #if defined(CONFIG_NSH_VARS)
-  /* Unset NSH variable */
+#ifndef CONFIG_DISABLE_ENVIRON
+      /* Check if the NSH variable has already been promoted to an group-

Review Comment:
   mistake, will fix



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org