Tell me more ×
Stack Overflow is a question and answer site for professional and enthusiast programmers. It's 100% free, no registration required.
up vote 0 down vote favorite

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;
}
share|improve this question
 
What's the runtime error? Is there a trace? What do you mean your gdb doesn't work? –  Justin Jasmann Oct 4 at 23:35
3  
If you're not doing this just for exercise you might want to look into the strtok function. It's used for splitting strings. –  jpw Oct 4 at 23:36
 
It is giving segmentation fault. –  haccks Oct 4 at 23:37
 
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. –  Paul Griffiths Oct 4 at 23:38
3  
@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. –  Paul Griffiths Oct 4 at 23:44
show 5 more comments

3 Answers

up vote 1 down vote accepted

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.

share|improve this answer
add comment
up vote 0 down vote

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.

share|improve this answer
add comment
up vote 0 down vote

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.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.