arrayscpointersstrtok

clearing an array in preparation for new array values


The function gets an input from the user using read() and breaks it down using strtok and places it into an array. The program loops until it reaches an error (which i won't get into because that isn't the problem here) or if the program is terminated by the user. However, when it loops back around and reads the input from the user, it seems to be hanging onto the previous input from the user.

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

#define BUFFERSIZE 1024

int main(int argc, char* argv[])
{
    argc++;

    char buf[BUFFERSIZE];
    int n;
        printf("Please enter commands: \n");

    while ((n = read(STDIN_FILENO, buf, BUFFERSIZE)) > 0)
        {
        printf("original string: %s:\n", buf);
        buf[strlen(buf)-1] = '\0';
        printf("after change: %s:\n", buf);
    
        int i = 0;

        char* array[100];
        char* token1 = strtok(buf, " ");

        while ((token1 != NULL))
        {
            array[i++] = token1;
            token1 = strtok(NULL, " ");
        }//while

        for (int j = 0; j < i; j++) 
        {
            printf("Array value %d: %s:\n", j, array[j]);
        }//for
        
        if (buf == "exit")
        {
            printf("found it\n");
        }//if

        
            for (int i = 1; i < argc; i++)
            {
            pid_t pid;
            
            if (argc >= 1) 
            {
                if ((pid = fork()) < 0) 
                {
                    perror("fork");
                }//if
                else if (pid == 0) 
                { // child process
                    if (execvp(array[0], array) == -1) 
                    {
                        perror("execvp");
                        return EXIT_FAILURE;
                    } // if
                }//else if
                else 
                {   // parent process
                    int status;
                    wait(&status);
                    printf("Please enter commands again: \n");
                    
                }//else     
            }//if
            else 
            {
                fprintf(stderr, "Please specify the name of the program to exec as a command line argument\n");
                return EXIT_FAILURE;
            }//if
        }//for
    }//while
    if (n == -1) perror("read");
}//main

I've tried to clear the array and clear "buf" but no luck. i have a feeling it has to do with the read() and the fact that "buf" is hanging onto its old value.


Solution

  • Prefaced by my top comments ...

    1. read does not add 0x00 the way fgets does, so we have to do it manually.
    2. execvp needs the array to be terminated with a NULL entry.
    3. child should use exit instead of return.

    Here's the refactored code. It is annotated. Note that this code does not do the split/join of the buffer to guarantee the buffer ending in newline as suggested by my top comments:

    #include <stdio.h>
    #include <unistd.h>
    #include <stdlib.h>
    #include <sys/wait.h>
    #include <fcntl.h>
    #include <string.h>
    
    #define BUFFERSIZE 1024
    
    int
    main(int argc, char *argv[])
    {
        argc++;
    
        char buf[BUFFERSIZE];
        int n;
    
        printf("Please enter commands: \n");
    
    // NOTE/BUG: read does _not_ add 0x00 the way fgets does
    #if 0
        while ((n = read(STDIN_FILENO, buf, BUFFERSIZE)) > 0) {
    #else
        while ((n = read(STDIN_FILENO, buf, BUFFERSIZE - 1)) > 0) {
            buf[n] = 0;
    #endif
            printf("original string: %s:\n", buf);
            buf[strlen(buf) - 1] = '\0';
            printf("after change: %s:\n", buf);
    
            int i = 0;
    
            char *array[100];
            char *token1 = strtok(buf, " ");
    
            while ((token1 != NULL)) {
                array[i++] = token1;
                token1 = strtok(NULL, " ");
            }
    
    // NOTE/BUG: execvp needs a NULL terminator
    #if 1
            array[i] = NULL;
    #endif
    
            for (int j = 0; j < i; j++) {
                printf("Array value %d: %s:\n", j, array[j]);
            }
    
    // NOTE/BUG: wrong way to compare strings
    #if 0
            if (buf == "exit")
    #else
            if (strcmp(buf, "exit") == 0) {
                printf("found it\n");
                break;
            }
    #endif
    
            for (int i = 1; i < argc; i++) {
                pid_t pid;
    
                if (argc >= 1) {
                    if ((pid = fork()) < 0) {
                        perror("fork");
                    }
    
                    // child process
                    else if (pid == 0) {
                        if (execvp(array[0], array) == -1) {
                            perror("execvp");
    #if 0
                            return EXIT_FAILURE;
    #else
                            exit(EXIT_FAILURE);
    #endif
                        }
                    }
    
                    // parent process
                    else {
                        int status;
    
                        wait(&status);
                        printf("Please enter commands again: \n");
                    }
                }
                else {
                    fprintf(stderr, "Please specify the name of the program to exec as a command line argument\n");
                    return EXIT_FAILURE;
                }
            }
        }
        if (n == -1)
            perror("read");
    }
    

    In the above code, I've used cpp conditionals to denote old vs. new code:

    #if 0
    // old code
    #else
    // new code
    #endif
    
    #if 1
    // new code
    #endif
    

    Note: this can be cleaned up by running the file through unifdef -k