The Muffin Boy
The Muffin Boy

Reputation: 314

2D Pointer to 2D Pointer

I forgot most of my C, so please forgive me if this is a stupid question. Because I need to separate a string of words into individual words.

#include "argsInfo.h"
#include <stdlib.h>

/* Parses string argument which contains words
 * separated by whitespace.  It returns an
 * argsInfo data structure which contains an
 * array of the parsed words and the number
 * of words in the array.
 */

argsInfo getArgsInfo(char * string) {
    argsInfo info;
    char ** temp;
    int nWords=1;
    int i=0;
    int j,k;
    //Test if the the input string is empty
    if (string[0] == '\0'){
        nWords=0;
    }else{
        //First I need to check how long the input String is, as-well as cout how many words are in the string.
        while (string[i] != '\0'){
            if (string[i] == ' '){
               nWords++;
            }
            i++;
        }
    }
    //This allocates enough memory for each word.
    temp = (char**) malloc(nWords*sizeof(char*));

    for (j=0;j<nWords;j++){
        temp[j] = (char*) malloc(i*sizeof(char));
    }
    j=0;
    k=0;
    // If I encounter a white space, it signifies a new word, and I need to move it to the next element
    while (j < i){
        if (string[j] == ' '){
            k++;
        }
        temp[k][j] = string[j];
        j++;
    }
    info.argc = nWords;
    info.argv = temp;
    return info;
}

That 3rd last LINE. THAT'S where I think the problem is. info.argv = temp;

This is what the struct looks like:

 typedef struct {
    int argc;
    char ** argv;
 } argsInfo;

Example Input and Output:

Input: "ax bcd efghij"

Output: ax

If I remove the k++ line, the output becomes: ax bcd efghij

Likewise, if I input a b c. Only 'a' will show up when I run through the array.

Upvotes: 2

Views: 112

Answers (2)

nem035
nem035

Reputation: 35501

First, this part is inefficient but works:

for (j=0;j<nWords;j++){
    temp[j] = (char*) malloc(i*sizeof(char));
}

You are using the value of i which will be equal to the total number of characters in your original input string. This means that for each separate word you are allocation enough room to store the original sentence which is a waste of space. You could, for example, while you are counting words, also remember the longest word seen thus far and use that as your allocation factor which will probably be much less than the whole sentence. We start the length at 1 to include the terminating character '\0'

int longest = 1;
int tempLength = 1;
//Test if the the input string is empty
if (string[0] == '\0'){
    nWords=0;
}else{
    //First I need to check how long the input String is, 
    //as-well as count how many words are in the string.
    while (string[i] != '\0'){
        if (string[i] == ' '){
           if(tempLength > longest) {
               longest = tempLength;
           }
           nWords++;
        } else {
            tempLength++; // count characters of current word
        }
        i++;
    }
}

for (j=0;j<nWords;j++){
    temp[j] = (char*) malloc(longest*sizeof(char));
}

Finally, the last part of your code needs a fix. It doesn't work because you are using j as an index in the overall sentence and as an index in a single word. You never reset j. Let's say the first word is

apple

Once you encounter a space, you will have:

j = 5
temp[0] = "apple"

Now you increment k to 1 but j stays the same so you will start storing characters of the next word from position 5 instead of 0:

temp[1][5] = string[5];

Instead of:

temp[1][0] = string[5];

Therefore, you have 3 indexes to worry about:

  • Index a that iterates over the input string.
  • Index b that iterates over a single word of the string.
  • Index c that iterates over the array of words.

The code:

int a, b, c;
for(a = 0, b = 0, c = 0; a < i; a++) {  // index i holds the total number of chars in input string
    if(string[a] != ' ') {
        temp[c][b] = string[a];
        b++;
    } else {
        temp[c][b] = '/0'; // add terminating character to current word
        b = 0;
        c++;
    }
}
info.argc = nWords;
info.argv = temp;
return info;

Upvotes: 2

WhozCraig
WhozCraig

Reputation: 66254

Pretty sure this is what you were after. This should only require scanning the string once. Your index math has several issues:

  • Your calculation of i is inefficient.
  • The hoops nWords seems to go through questionable
  • You don't seem to be interested in terminating each word, which is very bad.

That said, walk through the following very carefully in a debugger to see how it works.

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

argsInfo getArgsInfo(const char * s)
{
    argsInfo info = {0,NULL};

    while (*s)
    {
        // find start of next word
        while (*s && isspace((unsigned char)*s))
            ++s;

        // find end of next word
        const char *beg = s;
        while (*s && !isspace((unsigned char)*s))
            ++s;

        if ((s - beg) > 0)
        {
            char **tmp = realloc(info.argv, (info.argc+1)*sizeof(*tmp));
            if (tmp)
            {
                info.argv = tmp;
                tmp[info.argc] = malloc((s - beg + 1) * sizeof(char));
                if (tmp[info.argc] != NULL)
                {
                    memcpy(tmp[info.argc], beg, s-beg);
                    tmp[info.argc++][s-beg] = 0; // <<= TERMINATE
                }
                else
                {
                    perror("Failed to allocate string");
                    exit(EXIT_FAILURE);
                }
            }
            else
            {
                perror("Failed to expand string pointer array");
                exit(EXIT_FAILURE);
            }
        }
    }
    return info;
}

Upvotes: 2

Related Questions