Implementing argv/argc in C

We were recently assigned a problem where the goal was to develop behavior that simulates parsing argv/argc. The only requirement was that the prototype for the function that does this MUST be:

Code:

---

int makearg(char *s, char ***args);

---

I managed to get the program to work using this (probably less than elegant solution):

Code:

---

int makearg(char string[], char ***args) {
    char *bufferNext;                                      // pointer to the lines after the space
    int static count = 0;                                  // static count (recurssion)
   
    bufferNext = strchr(string, ' ');                      // find a space in string


    if (bufferNext != NULL) {   
       
        *bufferNext++ = '\0';                              // replace the space with a nul char
        args = malloc(sizeof(char)*strlen(string));        // allocate enough space to store the string
        *args = string;                                    // store the string   
       
        if (count == 0)   
            printf("You called the command %s\n", *args);  // tell user the command called
        else
            printf("Argument %d: %s\n", count, *args);    // tell user the arguments
   
        count++;                                          // increment the counter


        makearg(bufferNext, args++);                      // more spaces possible, continue
    } else {
        args = malloc(sizeof(char)*strlen(string));        // allocate for the last argument
        *args = string;                                    // store the last argument
        printf("Argument %d: %s\n", count, *args);        // print the last argument
    }
    args = args - count;                                  // reset the args pointer to the beggining
    return count;
}

---

Everything works as expected, with one exception: if I try to output the values of *args in main I get a segfault:

Code:

---

int main(int argc, char **argv)
{
    int argcount;
    char **args, str[] = "ls -l file";
    argcount = makearg(str, &args);
    printf("There were a total of %d arguments.\n", argcount);
   
    int i = 0;
    for (i = 0; i < argcount; i++) {
            printf("%s\n", *args[i]);
    }
   
    return 0;
}

---

I'm guessing it's an error with my pointers, but whatever the issue is I've hit a brick wall with it.

Try writing one that doesn't call malloc, with the following prototype: size_t makeargv_aux(char *str, char **args, size_t n);

Which may be called like so:

Code:

---

char *args[10];
char buf[] = "abc def ghi";

makeargv_aux(buf, args, sizeof args / sizeof args[0]);

---

Then, within makeargv, you can simply allocate all the memory you need beforehand, optionally make a copy of the string that gets passed, then make the call to makeargv_aux. Much easier to write multiple functions that do single tasks, instead of cramming multiple tasks into a single function.

Quote:

---

Originally Posted by Justin Hall

Everything works as expected,

---

Are you sure? What happens if there are 3 parameters to ls? Note, to handle any number of parameters, you need a loop. I might suggest using strtok instead of strchr, since it will better handle multiple spaces between parameters.

EDIT: Also note, typically argv should contain the program itself, i.e. the "ls" in your example, in argv[0], which your program does not do. Ultimately it depends on what the teacher wants, but it's something for you to consider.

Quote:

---

Originally Posted by Justin Hall

with one exception:

---

That's not the only exception. Compile with warnings turned all the way up. You have several places where you're using the wrong type of pointer:

Code:

---

$ make args
gcc -Wall -Wunreachable-code -ggdb3 -std=c99 -pedantic  -lm -lpthread  args.c  -o args
args.c: In function ‘makearg’:
args.c:16: warning: assignment from incompatible pointer type
args.c:19: warning: format ‘%s’ expects type ‘char *’, but argument 2 has type ‘char **’
args.c:21: warning: format ‘%s’ expects type ‘char *’, but argument 3 has type ‘char **’
args.c:29: warning: assignment from incompatible pointer type
args.c:30: warning: format ‘%s’ expects type ‘char *’, but argument 3 has type ‘char **’
args.c: In function ‘main’:
args.c:45: warning: format ‘%s’ expects type ‘char *’, but argument 2 has type ‘int’

---

Fix all of those.

Quote:

---

Originally Posted by Justin Hall

if I try to output the values of *args in main I get a segfault:

---

I call shenanigans :p. You aren't trying to print *args, you're trying to print *args[i], which is something else all together. That is the cause of one of the above warnings. args is a char **. Thus, args[i] is a char *, and so *args[i] is just a char. That is not suitable for %s.

Quote:

---

Originally Posted by anduril462

Are you sure? What happens if there are 3 parameters to ls? Note, to handle any number of parameters, you need a loop. I might suggest using strtok instead of

---

I don't need a loop, makearg is recursive until bufferNext is NULL (end of string).

Quote:

---

That's not the only exception. Compile with warnings turned all the way up. You have several places where you're using the wrong type of pointer:

Code:

---

$ make args
gcc -Wall -Wunreachable-code -ggdb3 -std=c99 -pedantic  -lm -lpthread  args.c  -o args
args.c: In function ‘makearg’:
args.c:16: warning: assignment from incompatible pointer type
args.c:19: warning: format ‘%s’ expects type ‘char *’, but argument 2 has type ‘char **’
args.c:21: warning: format ‘%s’ expects type ‘char *’, but argument 3 has type ‘char **’
args.c:29: warning: assignment from incompatible pointer type
args.c:30: warning: format ‘%s’ expects type ‘char *’, but argument 3 has type ‘char **’
args.c: In function ‘main’:
args.c:45: warning: format ‘%s’ expects type ‘char *’, but argument 2 has type ‘int’

---

Fix all of those.

---

True, I have several warnings. I said it works with that exception, but it's FAR from works "well" :p . If you run the program, the output of makearg works fine, even with the warnings about typecasting.

Quote:

---

Originally Posted by Justin Hall

I call shenanigans :p. You aren't trying to print *args, you're trying to print *args[i], which is something else all together. That is the cause of one of the above warnings. args is a char **. Thus, args[i] is a char *, and so *args[i] is just a char. That is not suitable for %s.

---

Hmmm... that makes sense. I'm still trying to wrap my brain around the purpose of having a pointer to this depth (passing ***char to makearg). In either case, I'm still getting a segfault if I change the line to:

Code:

---

printf("%c\n", *args[i]);

---

This isn't expected behavior since args[i] has been allocated (within makearg)... unless I'm missing something obvious.

warning C4047: '=' : 'char **' differs in levels of indirection from 'char *'
warning C4047: '=' : 'char **' differs in levels of indirection from 'char *'

you declare 'makearg::args' as char ***args but you malloc it as if it were and array of char, but then assign a pointer to it. draw a picture of the pointers involved and what points to what.

i think this is what you want

Code:

---

makearg::args -> main::args -> [char *] -> [string]
                                            [char *] -> [string]
                                            [char *] -> [string]
                                                  ...

---

I think what you want is to tokenize your string into arguments, malloc an array of char * (not char) and assign each token to the array of char *

Quote:

---

Originally Posted by Justin Hall

I don't need a loop, makearg is recursive until bufferNext is NULL (end of string).

---

Duh! Sorry, totally missed that. I see no reason for a recursive solution here, which is probably why I glossed over that. Also, when I find myself using/needing static vars to make a recursive solution work, I generally reconsider my approach. Note, you don't really need count to be static in this case, there are ways around it (i.e. using the return value of makeargs in your recursive call).

Also, there is a loop solution that is not too difficult. It would involve two loops. The first would scan the string forward, counting all the words (and possibly replacing space characters with null characters). Then you would allocate an array with the right number of char *, storing it in *args. Then a second loop could work backwards, finding the start of each word and populating (*args)[i] with the right strings, replacing space chars with '\0' if you didn't do it in the first pass. You can use strtok for the first loop.

Quote:

---

Originally Posted by Justin Hall

True, I have several warnings. I said it works with that exception, but it's FAR from works "well" :p . If you run the program, the output of makearg works fine, even with the warnings about typecasting.

---

Actually, it does not work. Not really. All your pointer mishandling, pointed out by those warnings, results in undefined behavior. It appears to work, but that's sheer dumb luck. You're getting away with it, but it could very well blow up in your face at any time.

Quote:

---

Originally Posted by Justin Hall

Hmmm... that makes sense. I'm still trying to wrap my brain around the purpose of having a pointer to this depth (passing ***char to makearg). In either case, I'm still getting a segfault if I change the line to:

Code:

---

printf("%c\n", *args[i]);

---

This isn't expected behavior since args[i] has been allocated (within makearg)... unless I'm missing something obvious.

---

You could have done:

Code:

---

printf("%s\n", args[i]);

---

Which would be the right way to print the parameter contained in the i'th position in args. However, that still would not fix your seg faults. Your problem is with how you allocate args.

Remember, if you want to change an int parameter to a function, and have that change persist after the function is done, you need to do something like the following:

Code:

---

void foo(int *bar)
{
    *bar = 42;  // now, whatever int 'bar' points to will have the value 42
}
// call it
int x;
foo(&x);

---

Thus, if you want to change args, you must do

Code:

---

int makeargs(char string[], char ***args)
{
    *args = malloc(num_args * sizeof(*args));
    (*args)[0] = "foo";  // store a string in the first spot in args
    ...
}
// call it
char **args;
num_args = makeargs(string, &args);
// print it
for i from 0; <= num_args
    print args[i]  // this is a string, you can use puts or printf with %s.

---

Two things to note about the code here, in particular the malloc call
1. You may want to use (num_args + 1) * sizeof(*args), if you want to have a NULL pointer as the last element, to terminate the list (similar to the null character '\0' used to terminate a string). The real argv for main has this.
2. I use *args in the sizeof. If you assign malloc to some pointer foo, then it's a good idea to use *foo in the sizeof parameter, instead of the actual type name. That way, you always allocate the right amount of space, and if the type of foo ever changes, you don't have to find all the malloc calls and change them too, they will still be correct.

There really isn't any point in trying to fix it. If it takes you hours to figure out what's causing it to crash then the solution is obvious: Choose a simpler design and rewrite it. If you follow the advice I gave in my previous post, it probably wouldn't take you any more than five minutes to come up with something that works without performing memory allocations. After realising that it's quite similar to counting words, I decided to try it myself and I came up with the following.

Code:

---

size_t makeargs(char **ap, size_t n, char *s){
        int c, inarg = 0;
        size_t save;


        for (save = n--; (c = (unsigned char) *s) && n; s++)
                if (inarg && isspace(c)) {
                        *s = '\0';
                        n--;
                        inarg = 0;
                } else if (!inarg && !isspace(c)) {
                        *ap++ = s;
                        inarg = 1;
                }
        *ap = 0;
        return save - n;
}

---

When you have that working all you need to do is add a layer on top of it that handles memory allocation. If there's a maximum number of arguments you'll need to handle in your program, then depending on how large it is, you might simply allocate an array of that many pointers to char and be done with it. Otherwise, you can write a function that computes the number of arguments in a string like so:

Code:

---

size_t countargs(const char *s)
{
        int inarg = 0, c;
        size_t n = 0;


        while ((c = (unsigned char) *s++))
                if (inarg && isspace(c)) {
                        inarg = 0;
                } else if (!inarg && !isspace(c)) {
                        n++;
                        inarg = 1;
                }
        return n;
}

---

After that, performing the allocation is about as straightforward as it gets:

Code:

---

char **makeargv(char *s)
{
        size_t len = countargs(s) + 1;
        char **r;


        if (!(r = malloc(len * sizeof *r)))
                return 0;
        makeargs(r, len, s);
        return r;
}

---

Of course, this uses the storage pointed at by s to fill the array. Since your program makes a copy of the string and splits up its own copy, it's a bit more difficult since you need to pass two pointers back to free(), but that can be accounted for in a clever way, as I'll show below.

Code:

---

char **makeargv(const char *s)
{
        size_t len = countargs(s) + 1;
        char **r;


        if (!(r = malloc((len + 1) * sizeof *r)))
                return 0;
        if (!(*r = malloc(strlen(s) + 1))) {
                free(r);
                return 0;
        }
        makeargs(r + 1, len, strcpy(*r, s));
        return r + 1;
}


void freeargv(char **argv)
{
        free(*--argv);  // Free the copy of the string
        free(argv);  // Free the array of pointers to char
}

---

This method's much easier than calling malloc() for every single argument you pass, and you can use the first function with memory that has already been allocated which may be beneficial in certain situations. Programming really is only as hard as you make it; if you need to design a large function that does multiple things then all you need to do is figure out what those multiple things are and deal with them individually.