Parsing character array to words held in pointer array (C-programming)

Question

I am trying to separate each word from a character array and put them into a pointer array, one word for each slot. Also, I am supposed to use isspace() to detect blanks. But if there is a better way, I am all ears. At the end of the code I want to print out the content of the parameter array.

Let's say the line is: "this is a sentence". What happens is that it prints out "sentence" (the last word in the line, and usually followed by some random character) 4 times (the number of words). Then I get "Segmentation fault (core dumped)".

Where am I going wrong?

int split_line(char line[120])
{
    char *param[21]; // Here I want to put one word for each slot
    char buffer[120]; // Word buffer
    int i; // For characters in line
    int j = 0; // For param words   
    int k = 0; // For buffer chars

    for(i = 0; i < 120; i++)    
    {
        if(line[i] == '\0')
            break;
        else if(!isspace(line[i]))   
        {    
            buffer[k] = line[i];
            k++;    
        }    
        else if(isspace(line[i]))
        {
            buffer[k+1] = '\0';
            param[j] = buffer; // Puts word into pointer array   
            j++;    
            k = 0;    
        }   
        else if(j == 21)    
        {
            param[j] = NULL;    
            break;    
        }    
    }    

    i = 0;
    while(param[i] != NULL)    
    {    
        printf("%s\n", param[i]);    
        i++;    
    }    
    return 0;    
}

param[j]=NULL; is an overflow for j==21, you want either a larger array or stop earlier. (It did not trigger with your short test sentence.) — Jongware
– Jongware, Commented Oct 8, 2014 at 20:57
You are re-using the same buffer for each of your pointers. — Jongware
– Jongware, Commented Oct 8, 2014 at 20:58
This line: int split_line(char line[120]) should be int split_line(char* pLine) — user3629249
– user3629249, Commented Oct 10, 2014 at 7:33
This line: for(i = 0; i < 120; i++) is using a hardcoded value, which is almost always a bad idea. Rather use this line: for(i = 0; i < strlen(pLine); i++) see above comment for where pLine came from — user3629249
– user3629249, Commented Oct 10, 2014 at 7:35
this line: param[j] = buffer; // Puts word into pointer array has an incorrect comment, as a pointer to the buffer array is all that is being moved. In every case, the same pointer is all that is being moved. also, the buffer must be cleared before each saving/extracting sequence. To actually save the extracted words, try param[j] = malloc( strlen(buffer) +1 ); strcpy( param[j], buffer ); memset( buffer, '\0', sizeof(buffer) ); — user3629249
– user3629249, Commented Oct 10, 2014 at 7:42

Serge Ballesta · Accepted Answer · 2014-10-08 21:35:03Z

There are many little problems in this code :

param[j] = buffer; k = 0; : you rewrite at the beginning of buffer erasing previous words
if(!isspace(line[i])) ... else if(isspace(line[i])) ... else ... : isspace(line[i]) is either true of false, and you always use the 2 first choices and never the third.
if (line[i] == '\0') : you forget to terminate current word by a '\0'
if there are multiple white spaces, you currently (try to) add empty words in param

Here is a working version :

int split_line(char line[120])

{

    char *param[21]; // Here I want to put one word for each slot
    char buffer[120]; // Word buffer
    int i; // For characters in line
    int j = 0; // For param words
    int k = 0; // For buffer chars
    int inspace = 0;

    param[j] = buffer;

    for(i = 0; i < 120; i++) {
        if(line[i] == '\0') {
            param[j++][k] = '\0';
            param[j] = NULL;
            break;
        }
        else if(!isspace(line[i])) {
            inspace = 0;
            param[j][k++] = line[i];
        }
        else if (! inspace) {
            inspace = 1;
            param[j++][k] = '\0';
            param[j] = &(param[j-1][k+1]);
            k = 0;
            if(j == 21) {
                param[j] = NULL;
                break;
            }
        }
    }



    i = 0;

    while(param[i] != NULL)

    {

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

        i++;

    }

    return 0;

}

I only fixed the errors. I leave for you as an exercise the following improvements :

the split_line routine should not print itself but rather return an array of words - beware you cannot return an automatic array, but it would be another question
you should not have magic constants in you code (120), you should at least have a #define and use symbolic constants, or better accept a line of any size - here again it is not simple because you will have to malloc and free at appropriate places, and again would be a different question

Anyway good luck in learning that good old C :-)

CS Pei · Accepted Answer · 2014-10-08 21:04:44Z

1

This line does not seems right to me

param[j] = buffer;

because you keep assigning the same value buffer to different param[j] s .

I would suggest you copy all the char s from line[120] to buffer[120], then point param[j] to location of buffer + Next_Word_Postition.

edited Oct 8, 2014 at 21:04

answered Oct 8, 2014 at 20:47

CS Pei

11.1k1 gold badge29 silver badges46 bronze badges

2 Comments

StraightUpBusta Over a year ago

Thank you, but I don't understand exactly why I should do "param[j] = buffer + i;" ? That line just puts the word into the pointer array.

CS Pei Over a year ago

you want to put different words into param[j], right? param[j] = buffer; keep assign param[0], param[1], ... the same value buffer which is buffer + 0;

user2105505 · Accepted Answer · 2014-10-08 20:45:43Z

0

You may want to look at strtok in string.h. It sounds like this is what you are looking for, as it will separate words/tokens based on the delimiter you choose. To separate by spaces, simply use:

dest = strtok(src, " ");

Where src is the source string and dest is the destination for the first token on the source string. Looping through until dest == NULL will give you all of the separated words, and all you have to do is change dest each time based on your pointer array. It is also nice to note that passing NULL for the src argument will continue parsing from where strtok left off, so after an initial strtok outside of your loop, just use src = NULL inside. I hope that helps. Good luck!

answered Oct 8, 2014 at 20:45

user2105505

7081 gold badge9 silver badges19 bronze badges

2 Comments

StraightUpBusta Over a year ago

Thank you. I did look into strtok, and I guess it will work, but I think am supposed to use isspace().

Serge Ballesta Over a year ago

Well isspace() recognize all white space characters : at least " \t\r\n"

Collectives™ on Stack Overflow

Parsing character array to words held in pointer array (C-programming)

3 Answers 3

Comments

2 Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

2 Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related