0

I am trying to write only to a file when a user specifies which file to write in with ">". The first iteration works fine, but when a user types any command after the one where they specified the file to write to with ">" it still writes to the file. I want my program to only write to a file when specified and after that if the user doesn't specify again with ">" it should print to the terminal. The code I run to write the execv() output to a file is this:

void write_file_if_exists(char *path, char *data){
    int fd = open(path, O_RDWR | O_CREAT, S_IRUSR | S_IWUSR);
        dup2(fd,1);
        close(fd);
    INSERT_PATH[0] = NULL;
}

and my code to then execute the execv command is this:

int run_bin(const char* command_line){
    char **commands_list = commands(command_line);
    char bin[20] = "/bin/";
    strcat(bin,commands_list[0]);

    int pid = fork();

    if(pid == 0){
        if(INSERT_PATH[0] != NULL){
            write_file_if_exists(INSERT_PATH[0],NULL);
        }
        int d = execv(bin,commands_list);
        return d;
    }
    else{
        int status=-1;
        wait(&status);
        return -1;
    }
    return 0;
}

How can I stop writing to the file if the user doesn't specify it again after the first command that specifies a file?

Wragnam
  • 19
  • 2
  • On an unrelated note: If `fork` fails, *or* your program is in the parent process, you unconditionally return `-1`. Why? Why is it always a failure to be in the parent process? – Some programmer dude Aug 17 '23 at 12:24
  • Note that there's no need to record or check the return value from [`execv()`](https://pubs.opengroup.org/onlinepubs/9699919799/functions/execv.html). If it succeeds, it does not return; if it returns, it failed (and the return value is `-1`, and `errno` indicates why). Typically, you should exit after failing to exec. Otherwise, you have two processes (the parent and the child) both trying to execute, which leads to chaos. There might be some wisdom in checking that `fd != 1` before closing it. The `dup2()` call is safe; the `close()` is not. – Jonathan Leffler Aug 17 '23 at 15:08
  • In general, you should leave ALL_CAPS names for macros and enumeration constants. The `INSERT_PATH` variable should not be all-caps. – Jonathan Leffler Aug 17 '23 at 15:13

1 Answers1

2

The call to write_file_if_exists only happens in the child process. Therefore you do the assignment

INSERT_PATH[0] = NULL;

in the child process only.

The parent process will never change the value of INSERT_PATH[0]. You need to change the value in the parent process, not the child process:

int pid = fork();

if (pid == 0)
{
    if(INSERT_PATH[0] != NULL){
        write_file_if_exists(INSERT_PATH[0],NULL);
    }
    execv(bin,commands_list);
    perror("execv");
    _exit(EXIT_FAILURE);
} else if (pid == -1)
    return pid;
} else {
    INSERT_PATH[0] = NULL;  // Do the "clearing" in the parent process
    return wait(NULL);
}

Also note that in the above code I have changed how errors are handled in the child process. If the execv call fails, my code reports the error and then exits the child process.

Since you don't show the code calling your run_bin function, I don't know how it handles possible errors. Especially how it handles errors in the child process (which the caller will know nothing about). If it continues to read input and run commands, then by returning in the child process will lead to other problems.

Better to just exit directly if any exec family of functions fails.

Some programmer dude
  • 400,186
  • 35
  • 402
  • 621