Return Array of Strings with String Input

Question

I'm trying to take a string and break it into "word" components and store that in an array of strings. "Hello my name is Bill." should give back a char** with elements, "Hello", "my", "name", "is", and "Bill."

My code will compile however I keep encountering a runtime error (I don't get warnings anymore and my debugger gdb doesn't work)>

I'm running on minGW on Window 8.

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

char** words(char* string)
{
    int i = 0;
    int j = 0;
    int k =0;
    int count = 0;

    char** stringArray = (char**) malloc(sizeof(char)*30*30);

    while( string[i] != '\0' )
    {
        if(string[i] != ' ')
        {
            j =0;
            while(string[i+j+1] != ' ')
            {
                j++;
            }
            i = i+j;
            for(k=0; k<=j; k++)
            {
                stringArray[count][k] = string[i+k];
            }
            count++;        
        }
        i++;
    }

    return stringArray;

}   
int main()
{   
    char message[20] = "abcd efgh ijkl mno";

    char** wordArray = words(message);

    printf("%c\n\n", wordArray[0][0]);

    int i =0;
    while(wordArray[i])
    {
        printf("%s\n", wordArray[i]);
        i++;
    }
    printf("\nThe problem is not with the words function");

    return 0;
}

What's the runtime error? Is there a trace? What do you mean your gdb doesn't work? — Justin Jasmann, Oct 04 '13 at 23:35
If you're not doing this just for exercise you might want to look into the [strtok](http://en.cppreference.com/w/c/string/byte/strtok) function. It's used for splitting strings. — jpw, Oct 04 '13 at 23:36
You return a `char **` but it doesn't actually contain any pointers to `char *`. You're almost certainly dereferencing an invalid address because of this. `string` and `stringArray` are also reserved identifiers. — Crowman, Oct 04 '13 at 23:38
@JustinJasmann It won't tell me a specific error code, window's will just tell me that the program stopped working. — DavidRC, Oct 04 '13 at 23:42
@PaulGriffiths So not only should I change the names of string and stringArray (even if I'm trying not to use the string library), but my array of strings is incorrect? — DavidRC, Oct 04 '13 at 23:43
not sure if mingw has wordexp, but... if it does, it may work — technosaurus, Oct 04 '13 at 23:44
@DavidRC: Yes, and yes. You don't have an array of strings at all, you just have one big `char` array and incorrectly point a `char **` at it. You should `malloc()` an array of `char *`, and then for each of them, `malloc()` space for your individual strings. I also can't see that you ever put any terminating `\0` in your strings. You should also get a better compiler if all your stuff isn't working. — Crowman, Oct 04 '13 at 23:44
Take a look at [Q: Using strtok in c](http://stackoverflow.com/questions/8106765/using-strtok-in-c/8106894#8106894)… :) — Vojtěch Hauser, Oct 04 '13 at 23:51
@PaulGriffiths is right. The other option is to leave it as a 2D array, but you'll need to change the type so it isn't `char**` in that case, and then in main you need to access it like it's a 2D array (not like it's an array of pointers, as you're doing now). Either solution would work fine. I'd say the advantage of Paul's suggestion is that it's easier to reason about accessing it properly (espeically for strings), but the down side is that you have to do some messier memory management. — Dave Lillethun, Oct 04 '13 at 23:57

score 1 · Accepted Answer · answered Oct 05 '13 at 00:09

There are couple of issues that have been mentioned in the comments. The allocation should look something like:

#include <ctype.h>    // for isspace()    

#define MAXSTRLEN 30  // using a symbolic constant

char **stringArray;
int i, j, k;

stringArray = malloc(sizeof(char*) * MAXSTRLEN); // don't cast from malloc
for (i = 0; i < 30; ++i) {
  stringArray[i] = malloc(sizeof(char) * MAXSTRLEN);
}
// TODO error checking: malloc could return NULL

while copying the substrings would look like:

i = 0;
j = 0;
while( string[i] != '\0')  // go through the whole string
{
    while (string[i] != '\0' && isspace(string[i])) {
     i++; // skip whitespaces
    }

    k = 0;
    while (string[i] != '\0' && !isspace(string[i])) { // copy word until whitepace or end of string
        stringArray[j][k++] = string[i++];
    }
    stringArray[j][k] = '\0'; // EOS !!!
    j++;
}

and printing (j is number of words actually read):

for (i = 0; i < j/*30*/; ++i) {  // (!) how to print
    printf("%s\n", stringArray[i]);
}

And, yes strtok would also do the job.

Dave Lillethun · Answer 2 · 2013-10-04T23:58:57.387

0

In words() you're assigning values to stringArray as a two-dimensional array, and in main() you're reading values from it as an array of pointers. Those are not the same thing.

So you need to change it so that you're consistently treating it as a 2D array, or so that you're consistently treating it as an array of pointers (char* to be exact). Either will work... see the comments above for elaboration.

edited Oct 04 '13 at 23:58

answered Oct 04 '13 at 23:52

Dave Lillethun

2,978
3
20
24

score 0 · Answer 3 · answered Oct 05 '13 at 00:08

This code is all wrong.

char** stringArray = (char**) malloc(sizeof(char)*30*30);

First of all, sizeof(char) is always one, second, you don't need to cast a void. So:

char **stringArray = malloc(30 * 30);

But that doesn't make any sense because it's an array of char *, so you should allocate in terms of that:

char **stringArray = malloc(sizeof(char *) * 30);

Or even better:

char **stringArray = malloc(sizeof(*stringArray) * 30);

So now you have an array with 30 char *, but each of those is not initialized, so you need to do that:

for (i = 0; i < 30; i++)
    stringArray[i] = malloc(sizeof(**stringArray) * 30);

If you don't do that, you can't access stringArray[count][k].

And then you assume the last element in the array is NULL, but you never set it, so you either do stringArray[count] = NULL at the end of words(), or you do calloc() instead of malloc().

I'm not analyzing the code beyond that; it's just all wrong.

Return Array of Strings with String Input

3 Answers3