Issue with Executing Commands in C Shell Program

Question

I'm currently working on a C shell program that takes user commands, searches for the command executable in the system's PATH, and then executes the command using execve. While the program seems to work correctly for the first execution, I'm encountering an issue when trying to execute another command in the same session.
Here is the code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/types.h>
#include <sys/wait.h>
#include <unistd.h>
#include <sys/stat.h>

extern char **environ;

char *getcommand(char *command)
{
    char *path = getenv("PATH");
    char *token;
    char *execution;
    struct stat st;
    
    token = strtok(path, ":");
    
    while (token)
    {
        execution = malloc(strlen(token) + strlen(command) + 2);
        if (!execution)
        {
            // Handle memory allocation failure
            perror("malloc");
            return (1);
        }
        strcpy(execution, token);
        strcat(execution, "/");
        strcat(execution, command);
        if (stat(execution, &st) == 0)
        {
            return (execution);
        }
        free(execution);
        token = strtok(NULL, ":");
    }
    return (NULL);
}

int main() {
    char *prompt = "$ ";
    ssize_t toread;
    char *command;
    size_t n = 0;
    const char *delim = " ";
    char *token, *atoken;
    int count = 0;
    char **argv;
    pid_t pid;
    int status;
    char *execution;
    int i = 0;
    
    while (1)
    {
        write(1, prompt, strlen(prompt));
        
        command = malloc(sizeof(char) * n);
        if (command == NULL)
        {
            perror("Error");
            exit(1);
        }
        
        toread = getline(&command, &n, stdin);

        // Remove the newline character if present
        if (toread == -1)
        {
            printf("\n");
            exit(1);
        }
        
        if (toread > 0 && command[toread - 1] == '\n') {
            command[toread - 1] = '\0';
            toread--;
        }
        
        /* get the first token */
        token = strtok(command, delim);
   
       /* count tokens */
        argv = malloc(sizeof(char *) * (count + 1));
        while (token != NULL) {
            argv[count] = strdup(token);
            printf("2)- Token: %s\n", token);
            printf("-----------------------------\n");
            token = strtok(NULL, delim);
            count++;
        }
        argv[count] = NULL;
        printf("3)- Argv 0: %s\n", argv[0]);
        printf("-----------------------------\n");
        
        execution = getcommand(argv[0]);
        printf("execution: %s\n", execution);
        pid = fork();
        
        if (pid == -1)
        {
            perror("Fork Failed");
            return (1);
        }
        
        if (pid == 0)
        {
            if (execve(execution, argv, environ) == -1)
            {
                perror("Error in execution");
                free(execution);
                return (1);
            }
        }
        else
        {
            wait(&status);
            count = 0;
        }
        for (i = 0; i < count; i++)
        {
            free(argv[i]);
        }
        free(execution);
        free(command);
        free(argv);
    }
        return 0;
}

I've been trying to debug this issue by examining the output and code, but I'm unable to pinpoint the root cause. The path search in the getcommand function seems to be working correctly, but something is going wrong when trying to execute the command. When I first type a command, for example ls, execveworks just fine, but when I try for the second time in the same session, it gives this: Error in execution: Bad address and it seems like the problem is in execution.

Answer 1

One likely issue is, that strtok modifies the underlying string (the path argument in your case). What's more, getenv gives you a pointer to the same underlying buffer each time it is called. The first call of getcommand succeeds (and returns some value), but by side-effect via strtok, "poisons" the environment for all subsequent invocations.

Either use a different (non-destructive) method to split the path, or make a copy of what getenv returns (and free it properly afterwards), use getenv_s if available, etc.