5

I am working on a custom shell for a systems programming class. We were instructed to implement the built-in setenv() and unsetenv() commands with a hint of check man pages for putenv().

My issue is that setenv(char*, char*, int) and putenv(char*) do not seem to be working at all. My code for executing a command entered is as follows:

//... skipping past stuff for IO redirection
pid = fork();
if(pid == 0){
    //child
    if(!strcmp(_simpleCommands[0]->_arguments[0],"printenv")){
        //check if command is "printenv"
        extern char **environ;
        int i;
        for(i = 0; environ[i] != NULL; i++){
            printf("%s\n",environ[i]);
        }
        exit(0);
    }
    if(!strcmp(_simpleCommands[0]->_arguments[0],"setenv")){
        //if command is "setenv" get parameters char* A, char* B
        char * p = _simpleCommands[0]->_arguments[1];
        char * s = _simpleCommands[0]->_arguments[2];

        //putenv(char* s) needs to be formatted A=B; A is variable B is value
        char param[strlen(p) + strlen(s) + 1];
        strcat(param,p);
        strcat(param,"=");
        strcat(param,s);
        putenv(param);
        //setenv(p,s,1);
        exit(0);
    }
    if(!strcmp(_simpleCommands[0]->_arguments[0],"unsetenv")){
        //remove environment variable
        unsetenv(_simpleCommands[0]->_arguments[0]);
        exit(0);
    }
    //execute command
    execvp(_simpleCommands[0]->_arguments[0],_simpleCommands->_arguments);
    perror("-myshell");
    _exit(1);
}

//omitting restore IO defaults...

If I run printenv it works properly, but if I try to set a new variable using either putenv() or setenv() my printenv() command returns the exact same thing, so it does not appear to be working.

As a side note, the problem may not be with the functions or how I called them, because my shell is executing the commands as though it had to format a wildcard (* or ?) which I am not sure should happen.

roschach
  • 8,390
  • 14
  • 74
  • 124
AChrapko
  • 166
  • 2
  • 2
  • 13

2 Answers2

6

You appear to be calling fork unconditionally before examining the command line. But some shell built-in commands need to run in the parent process, so that their effect persists. All the built-ins that manipulate the environment fall in this category.

As an aside, I wouldn't try to use the C library's environment manipulation functions if I were writing a shell. I'd use three-argument main, copy envp into a data structure under my full control, and then feed that back into execve. This is partially because I'm a control freak, and partially because it's nigh-impossible to do anything complicated with setenv and/or putenv and not have a memory leak. See this older SO question for gory details.

Community
  • 1
  • 1
zwol
  • 135,547
  • 38
  • 252
  • 361
  • Okay, that makes sense. From all the research I have done for the project, a lot examples use 3-argument main() but as this is a project I didn't want to make too many changes to the original code they gave us. I really dislike when they give us skeletons of something and we have to hunt down all the places we need to add to, especially when there are comments like "this may contain bugs, you are responsible for fixing them" I believe that if I just move the whole if(!strcmp()...) up above the fork() it should fix it – AChrapko Oct 06 '11 at 05:22
  • Yes, moving all that code up looks like it should do the trick. I'm not a fan of that style of homework assignment myself, but I don't know what I would do instead if I actually had to teach this kind of course, so maybe I shouldn't grumble. – zwol Oct 06 '11 at 05:24
  • Okay, so that didn't do the trick. It still exists if I print immediately after setting the new variable, but if I use printenv (as command in the shell) it didn't persist. I have a recitation tomorrow so I'll ask my TA then as well – AChrapko Oct 06 '11 at 05:31
  • You did take the `exit` calls out, yes? I know it's obvious, but sometimes those are the things you forget to do, *because* they're so obvious. – zwol Oct 06 '11 at 05:40
  • Hehe yeah I took that out and also tried with it in. Uhm, one thing that comes to mind. Should it be an if-else where the else holds the entire for-loop that runs all the fork() and exec*() commands? – AChrapko Oct 06 '11 at 05:45
  • I don't understand why there's a for-loop, but yes, you should not fork at all in this case. – zwol Oct 06 '11 at 05:50
  • Ah, sorry. The entire premise of our shell project was to also learn Lex and Yacc. The for loop inside execute() runs for the number of commands that are entered at once (ex "echo < cat file.txt" = 2 commands) and depending on whether or not you are executing the last command it handles the pipes. Also, I don't know why I didn't just try it and find out for myself, but it didn't seem to work either. – AChrapko Oct 06 '11 at 05:54
  • You might want to think a little about what you'd have to do to support `printenv | grep APPLE`. – zwol Oct 06 '11 at 06:14
0

What make you think it is not working? I wrote a simple test case below...and it worked as expected.

Making sure you setevn and prientevn are called in the same process.

#include <stdlib.h>
#include <assert.h>
int main()
{


 char * s= "stack=overflow";

 int ret =  putenv(s);

 assert(ret == 0);

 //printout all the env
 extern char **environ;
        int i;
        for(i = 0; environ[i] != NULL; i++){
            printf("%s\n",environ[i]);
        }


 return 0; 

}
pierr@ubuntu:~/workspace/so/c/env$ ./test | grep stack
stack=overflow
pierrotlefou
  • 39,805
  • 37
  • 135
  • 175
  • Okay, so I called my print environment variables function right after setenv(p,s,1) and that does display it. I am a little confused as to why the environment variables change back once the child process finishes though, and I think that is what is happening. – AChrapko Oct 06 '11 at 05:17
  • 1
    Every process has its own copy of the environment variables. That they inherit from parent to child is just a convention - if you use `execve` (which is the real system call that all the other `exec*` functions are implemented in terms of) you can set them to whatever you want. Manipulation of one process's copy never affects any other existing process, and in particular, *nothing* a child process does can change its parent's copy of the environment. – zwol Oct 06 '11 at 05:27