Return an array of strings from a function

Question

I need to return a char** but when I try to do this, the compiler tells me that I want to return the address of a local variable. How can I do that? I know that I should allocate space for this variable but how? Here is my code, but the second printf doesn't appear and the function returns nothing:

char** parse_cmd(const char* cmdline) {
char** arguments = (char**)malloc(100);
int i;
int j=0, k=0;
printf("%s\n", cmdline);

for(i=0; i<100; i++) {
    arguments[i] = malloc(100);
}

for(i = 0; i < strlen(cmdline); i ++) {
    if(cmdline[i] != ' ') {
        arguments[j][k] = cmdline[i];
        k++;
    } else {
        arguments[j][k] = '\0';
        j++;
        k = 0;
    }
}

printf("%s\n", arguments[1]);

return arguments;
}

Casting the return value of `malloc` in C is considered bad practice by many (like me) for a variety of reasons. — Chris Lutz, May 30 '11 at 05:11
Don't call `strlen()` each time around the loop; it gets expensive. However, that's not the main problem here... — Jonathan Leffler, May 30 '11 at 05:53
What happens if you get 3 spaces in a row? How do you mark the end of the returned argument list? I see no null pointer, and no count. — Jonathan Leffler, May 30 '11 at 06:48

score 4 · Answer 1 · answered May 30 '11 at 05:03

4

You need to do multiple allocations. The first for the char** and then for each of the char*. E.g. something like

  char **args = (char**)malloc(100);
  int i;
  for (i=0; i<100; i++) 
    args[i] = malloc(100);

  // Rest of program

  return args;

answered May 30 '11 at 05:03

Dov Grobgeld

4,783
1
25
36

Now works, but if my `cmdline` is `/ test /' `arguments[0]` is `/***` * is strange symbol. – nyanev May 30 '11 at 05:11
@n_yanev - This is because you haven't terminated your string for each argument. In the else clause, you need to add a '\0' to the end before you iterate j. Like arguments[j][k] = '\0' – sickgemini May 30 '11 at 06:01
You can see my edited code in my first post. But I have the same error. – nyanev May 30 '11 at 06:30

Jonathan Leffler · Accepted Answer · 2011-05-30T15:08:15.950

Here's the code I assembled - and tested. It uses dynamic memory allocation for both the argv argument list and for each argument as it is assembled. The function release_cmd() releases the allocated space. The function cleanup() is internal and releases allocated space on a failure, before returning a null double-pointer. This simplifies the error handling. There's a minimal test harness in the prompt() function and main(). I haven't run in under valgrind, but the MacOS X implementation of malloc() quite often spots problems so I'm moderately confident there's no gross memory abuse - but there could be an off-by-one leak. I haven't tested the cleanup() code by faking an allocation failure.

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

void release_cmd(char **argv)
{
    for (size_t i = 0; argv[i] != 0; i++)
        free(argv[i]);
    free(argv);
}

static char **cleanup(size_t argc, char **argv)
{
    argv[argc] = 0;
    release_cmd(argv);
    return 0;
}

char **parse_cmd(const char* cmdline)
{
    size_t argc = 2;
    char **argv = malloc(argc * sizeof(char *));

    if (argv == 0)
        return 0;

    size_t j = 0;  // Index into argv
    size_t len = strlen(cmdline);

    for (size_t i = 0; i < len; i++)
    {
        while (isspace(cmdline[i]))
            i++;
        if (cmdline[i] == '\0')
            break;
        if (j > argc - 2)
        {
            size_t newc = (argc * 2);
            char **newv = realloc(argv, newc * sizeof(char *));
            if (newv == 0)
                return cleanup(argc, argv);
            argv = newv;
            argc = newc;
        }
        size_t argl = 2;    // Length of argument string
        argv[j] = malloc(argl);
        size_t k = 0;       // Index into argv[j]
        while (cmdline[i] != '\0' && !isspace(cmdline[i]))
        {
            if (k > argl - 2)
            {
                size_t newl = argl * 2;
                char  *news = realloc(argv[j], newl);
                if (news == 0)
                    return cleanup(argc, argv);
                argv[j] = news;
                argl    = newl;
            }
            argv[j][k++] = cmdline[i++];
        }
        argv[j][k] = '\0';
        argv[j] = realloc(argv[j], k+1);    // Shrink to fit!
        j++;
    }
    argv[j] = 0;
    argv = realloc(argv, (j+1)*sizeof(*argv));  // Shrink to fit!

    return argv;
}

static int prompt(const char *prompt, char *buffer, size_t bufsiz)
{
    printf("%s", prompt);
    return (fgets(buffer, bufsiz, stdin) != 0);
}

int main(void)
{
    char line[1024];

    while (prompt("cmd? ", line, sizeof(line)) != 0)
    {
        char **argv = parse_cmd(line);
        char **args = argv;
        while (*args)
            puts(*args++);
        release_cmd(argv);
    }
    putchar('\n');
    return 0;
}

The bits I'm not quite happy with are the outer `for` loop, since the inner code goes tampering with the outer loop index `i`, and it might be cleaner to package the two `realloc()` chunks into separate functions. The loop could be replaced with just `while (1)` (you have to declare `i` and do not need `len` any more). I've now checked with `valgrind` and have fixed the `argv = realloc(argv, (j+1)*sizeof(*argv));` in the code. — Jonathan Leffler, May 30 '11 at 15:07
(Just for the record, the buggy code was `argv = realloc(argv, j+1);`, created by careless analogy with the similar `realloc()` for over-long character strings. The 'shrink to fit' `realloc()` operations may save space if the doubling algorithm allocated 128 bytes, say, as the string grew longer than 64 bytes. — Jonathan Leffler, May 30 '11 at 15:18

Return an array of strings from a function

2 Answers2