0

I'm trying to send an array of a predefined size of user input to the execvp function, however I receive a warning from the compiler. There is ratio of text-code that stackoverflow requires so I'm just trying to fill it up so that I can submit my post. Don't bother reading this, just head down to the code below. Thank you in advance!!!

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>
#define INPUTSIZE 50
#define INPUTCOUNT 10

int parseInput(char string[*][INPUTSIZE]);

void main(int argc, char **argv){
    char prompt = '#', shellInput[INPUTCOUNT][INPUTSIZE];
    pthread_t thread;
    pid_t processID;
    int rc;

    while(1){
        printf("%c", prompt);

        if(parseInput(shellInput)==EOF){
            exit(EXIT_SUCCESS);
        }

        processID = fork();
        if(processID == 0){
            execvp(shellInput[0], shellInput);
            printf("Uknown command\n");
        }


        printf("Nope.gif!\n");
        fflush(stdout);

    }
}

int parseInput(char string[INPUTCOUNT][INPUTSIZE]){
    char *inputString, *tempString;
    int i, status; 
    size_t bytecount = INPUTSIZE+1;

    inputString = (char *)malloc(INPUTCOUNT*INPUTSIZE);
    tempString = (char *)malloc(INPUTSIZE);
    status = getline(&inputString, &bytecount, stdin);

    tempString = strtok(inputString, " ");


    for(i = 0; i < INPUTCOUNT && tempString != NULL; i++){
        strncpy(string[i], tempString, INPUTSIZE);
        tempString = strtok(NULL, " \n\r");
    }
    strncpy(string[i], "NULL", sizeof(NULL));

    return status;
}
9
  • 1
    strncpy(string[i], "NULL", sizeof(NULL)); "NULL" != NULL Commented Mar 26, 2015 at 16:48
  • 1) this line: 'int parseInput(char string[*][INPUTSIZE]); would be much better/clearer written as: 'int parseInput(char **string);' 2) tgurn on all the compiler warnings then the compiler would have told you that this line: 'void main(int argc, char **argv){' is not valid, it should be: 'int main(int argc, char **argv){'. main() ALWAYS returns int. Commented Mar 26, 2015 at 17:28
  • neither argc nor argv[] were used, so the compiler will raise warnings about the two unused parameters. Warnings are where the compiler sees some doubtful code. To fix that warning use: 'int main(void)' Commented Mar 26, 2015 at 17:29
  • the code block beginning with: 'if(processID == 0){' is all in a child process. the process, if not exited, will continue around the loop, doing the same thing as the parent. to avoid that problem, insert a 'exit()' statement just before the closing brace of that code block Commented Mar 26, 2015 at 17:34
  • all calls to malloc (and family) 1) in C, do not cast the returned value 2) always check (!=NULL) the returned value to assure the operation was successful Commented Mar 26, 2015 at 17:36

2 Answers 2

1

Given

char shellInput[INPUTCOUNT][INPUTSIZE];

The type of variable shellInput is "array of INPUTCOUNT arrays of INPUTSIZE chars". In most contexts, shellInput is automatically converted to type "pointer to array of INPUTSIZE chars" (char (*)[INPUTSIZE]). That's not at all the same thing as "array of pointer to char" ((char *)[]), which is the required type of the second argument to execvp().

(Technically, execvp() is actually declared to accept a const array (of non-const pointers), but it is acceptable to pass a non-const one.)

It looks like you want something more like this:

    char *shellInput[INPUTCOUNT + 1];

    /* ... */

int parseInput(char *string[]) {
    char *line = NULL;
    size_t line_len;
    ssize_t n_read = getline(&line, &line_len, stdin);
    char *token;
    int input_count = 0;

    if (n_read < 0) {
        return n_read;
    }

    for (token = strtok(line, " "); token && (input_count < INPUTCOUNT);
            token = strtok(NULL, " ")) {
        string[input_count++] = strdup(token);
    }
    string[input_count] = NULL;
    free(line);

    return n_read;
}

Note, too, that the parent process needs to free the elements of shellInput (up to, but not including the first NULL) after each fork(). Additionally, note that there's no particular reason why you couldn't make shellInput have type char **, and teach parseInput() to arrange to dynamically allocate enough space for any number of command-line words.

Sign up to request clarification or add additional context in comments.

Comments

0

You array should be an array of pointers to char, not array of array of char. This is because the logical end of the array of strings (aka char *) that you must give to exec must end with a nul pointer. If you want to exec to ls -l myfile the array must be equals to:

char *args[] = { "ls", "-l", "myfile", NULL };

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.