Initialize array of cstrings within a method in C using malloc()

Question

I'm trying to create a simple shell program in C as an assignment. Unfortunately I've not dabbled in C, only C++ so I don't have a great understanding of how these things work.

It should hold at least 5 tokens (1 commands and 4 args).

#include <stdio.h>
#include <unistd.h>
#include <stdlib.h>
#include <stdbool.h>
#include <string.h>

int main(int argc, const char * argv[]) {
    bool moreCommands = false;

    char * in = "ls -l";

    char * parsedInput[6];

    parseUserInput(in, parsedInput);

    return 0;
}

void parseUserInput(const char * input, char * userInput[]) {


    int i = 0;
    int j = 0; // Indicates current index of userInput
    userInput[j] = malloc(8); // 8 is arbitrary

    while (input[i] != '\0') {

        printf("Char %d: %c\n", i, input[i]); // Debug information

        if (input[i] == ' ') { // If space is encountered, start new token in next index of 
                               // string array
            j++;
            userInput[j] = malloc(8);
        } else {
            strlcat(userInput[j], &input[i], 1); // Concat char to end of token in input

        }
        i++;
    }
}

The debugger clearly shows 6 items in the parsedInput array of type char *, but the local variable userInput (which should be the same array since it's passed by reference) has no indices in the debugger.

I don't understand what's going on.

p.s. I have tried searching for a solution but have not been able to find one.

I want the method to initialize the array parsedInput with strings corresponding to each "token".

Thanks!

Does changing the type to char** in the function fix it? Since it might not be by reference if it’s an array of pointers. Can’t remember exactly how that worked. — Sami Kuhmonen
– Sami Kuhmonen, Commented Feb 26, 2020 at 2:39
@SamiKuhmonen I originally had the type set to that, and the problem still existed. I actually changed it to what it is now in an effort to fix it haha — Josh
– Josh, Commented Feb 26, 2020 at 2:53
Also you try to concatenate characters to memory that has not been initialized, so anything can happen since there’s no null specified at the memory. — Sami Kuhmonen
– Sami Kuhmonen, Commented Feb 26, 2020 at 2:55
@SamiKuhmonen I thought using malloc initializes the string. You can see that before I enter the while loop, I initialize the first element in the array with malloc(8) — Josh
– Josh, Commented Feb 26, 2020 at 2:58
It only allocates. What that memory contains is not specified. — Sami Kuhmonen
– Sami Kuhmonen, Commented Feb 26, 2020 at 3:15

David C. Rankin · Accepted Answer · 2020-02-26 06:30:12Z

You can do exactly what you are attempting, but if you declare an array of pointers with automatic storage duration, then you must be sure to limit the number of tokens you allocate to number you declare to protect your array bounds. You should also consider allocating 1 additional pointer to allow you to store NULL as the next pointer after your last valid token to serve as a sentinel NULL marking the end of valid pointers in your array. (just as is done with *argv[], and is needed by the execv, execvp and execve functions)

To operate on a string-literal, no modification of the input string can occur. The easiest way to handle separating words on a ' ' (space), is simply to use a pair of pointers (a start and end-pointer) where you move the end-pointer down the string until you find a space, then allocate storage for the number of characters between start and end (+1 for the nul-terminating character) and then just copy the characters to your newly allocated storage and nul-terminate the string.

As your loop advances your end-pointer by one so it points to the next character after the space, set your start pointer to your end pointer so it is at the beginning of the next token. Advance your array of pointer index to the next pointer and set to NULL for the sentinel after you copy each word.

Additionally, you need to keep a state variable, a simple int to use as a 1 (true)/0 (false) flag to track if you are in-word reading characters, or out reading spaces. That allows you to skip over leading spaces, multiple included spaces between tokens, or trailing spaces after the last token. This allows you to split "ls -l" or " ls -l " into the same two tokens "ls" and "-l".

Your parseUserInput() can be written as:

#define MAXARG 6

void parseUserInput (char **userInput, const char *input)
{
    int i = 0, in = 0;                      /* index, and in/out of word flag */
    const char *p = input, *ep = p;         /* pointer and end-pointer */

    while (i < MAXARG) {                    /* loop while pointers remain */
        if (!*ep || *ep == ' ') {           /* if at nul-char or space */
            size_t len = ep - p;            /* get length of token */
            if (in && len) {                /* in-word and chars in token */
                /* allocate/validate storage for token */
                if (!(userInput[i] = malloc (len + 1))) {
                    perror ("malloc-userInput[i]");
                    break;
                }
                memcpy (userInput[i], p, len);  /* copy len chars to storage */
                userInput[i++][len] = 0;        /* nul-terminate, advance index */
                userInput[i] = NULL;        /* set next pointer NULL */
            }
            if (!*ep)                       /* if at end, break */
                break;
            in = 0;                         /* set in-word flag 0 (false) */
        }
        else {  /* normal word char */
            if (!in)                        /* if not in-word */
                p = ep;                     /* update start to end-pointer */
            in = 1;                         /* set in-word flag 1 (true) */
        }
        ep++;   /* advance to next character */
    }
}

(note: the order of the parameters are switched to make the order consistent with strcpy, memcpy, etc...)

Adding a short main() with your example you can do:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define MAXARG 6

void parseUserInput (char **userInput, const char *input)
{
    int i = 0, in = 0;                      /* index, and in/out of word flag */
    const char *p = input, *ep = p;         /* pointer and end-pointer */

    while (i < MAXARG) {                    /* loop while pointers remain */
        if (!*ep || *ep == ' ') {           /* if at nul-char or space */
            size_t len = ep - p;            /* get length of token */
            if (in && len) {                /* in-word and chars in token */
                /* allocate/validate storage for token */
                if (!(userInput[i] = malloc (len + 1))) {
                    perror ("malloc-userInput[i]");
                    break;
                }
                memcpy (userInput[i], p, len);  /* copy len chars to storage */
                userInput[i++][len] = 0;        /* nul-terminate, advance index */
                userInput[i] = NULL;        /* set next pointer NULL */
            }
            if (!*ep)                       /* if at end, break */
                break;
            in = 0;                         /* set in-word flag 0 (false) */
        }
        else {  /* normal word char */
            if (!in)                        /* if not in-word */
                p = ep;                     /* update start to end-pointer */
            in = 1;                         /* set in-word flag 1 (true) */
        }
        ep++;   /* advance to next character */
    }
}

int main (void) {

    char *in = "ls -l",
        *parsedInput[MAXARG + 1] = { NULL };    /* add +1 for sentinel NULL at end */

    parseUserInput (parsedInput, in);           /* note: parameter order change */

    for (char **p = parsedInput; *p; p++) {     /* loop over filled pointers */
        puts (*p);
        free (*p);      /* don't forget to free what you allocate */
    }
}

(note: don't forget to free() the storage you have allocated)

Example Use/Output

$ ./bin/splitinput
ls
-l

Look things over and let me know if you have further questions.

I think you should reference the execv(), execve() and execvp() functions rather than the l-equivalents. The l-equivalents can't be used in a general-purpose shell because the number of arguments passed to them must be known at compile time and the shell doesn't know how many arguments will be present. You can sometimes get away with execl() or execlp() by passing extra parameters as NULL pointers, but execle() will be confused; it expects the environment after the first NULL.
You are most correct. I had my hurry, check the man-page hat on and typed the wrong variants. Thank you!
Because there is no '\0' at the end-pointer. (for a straight strcpy) You are just inch-worming your way down input bracketing characters between your start pointer p and end-pointer ep and copying the character between them. You could use a strncpy, but strlcpy isn't standard C. For portability, everybody has memcpy and you have already computed the length with ep - p, so no need to pass to another function to recount.

Collectives™ on Stack Overflow

Initialize array of cstrings within a method in C using malloc()

1 Answer 1

4 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

4 Comments

Your Answer

Sign up or log in

Post as a guest

Related