[GitHub] [incubator-nuttx] pussuw commented on a diff in pull request #6423: sched/env: Fix the return value of unsetenv()

[GitBox <gi...@apache.org>]

pussuw commented on code in PR #6423:
URL: https://github.com/apache/incubator-nuttx/pull/6423#discussion_r895661902


##########
sched/environ/env_unsetenv.c:
##########
@@ -61,23 +61,22 @@ int unsetenv(FAR const char *name)
 {
   FAR struct tcb_s *rtcb = this_task();
   FAR struct task_group_s *group = rtcb->group;
-  int ret = OK;
+  int idx;
 
   DEBUGASSERT(name && group);
 
   /* Check if the variable exists */
 
   sched_lock();
-  if (group && (ret = env_findvar(group, name)) >= 0)
+  if (group && (idx = env_findvar(group, name)) >= 0)
     {
       /* It does!  Remove the name=value pair from the environment. */
 
-      env_removevar(group, ret);
-      ret = OK;
+      env_removevar(group, idx);
     }
 
   sched_unlock();
-  return ret;
+  return OK;

Review Comment:
   Yes we could add check for the incoming parameter too. The man page is a bit ambiguous in my opinion about this.
   
   For `setenv()`it makes perfect sense to check for the incoming parameter. 
   For `unsetenv()` it is ambiguous. The check is for parameter validity, but no such env variable can exist, as setenv() checks for parameter validity. As such name does not exist, should the function return success ?



-- 
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