Error when setting a character to a string in C

Question

hey guys I need your help. I'm trying to extract a character from a string and set it as the 2nd element of an array of strings. Yet as usual C is giving me segmentation faults, Ive tried sprintf, strcpy, and still segmentation fault the code is:

int getwords(char *line, char *words[])
{    
    int nwords=2;  
    char *result= NULL;
    char TABS[]="\t";
    char spaces[]=" ";
    char commas[]=","; 
    result = strtok(line,TABS);
    words[1]=result[strlen(result)-1];//setting the 2nd element of the array to a char
    result[strlen(result)-1]='\0';//removing the extracted char from the string
    words[0]=result;//setting 1st element to the new modified word
    printf("the opcode is:%s and the type is:%c\n",words[0],result[strlen(result)-1]);

    return nwords;

}

e.g. If I give it "bye." it should return 2 and an array having 2 elements: 1st elem="bye" 2nd elem="." I ran some tests and found out that the error is from the statement: words[1]=result[strlen(result)-1]; Any help is welcom

This looks like PHP rather than C. You have multiple errors here. `result` isn't an array. — mingos, Dec 18 '10 at 13:15
@mingos: pointers can be indexed exactly as arrays. `pointer[index]` is exactly the same as `*(pointer+index)`. — Matteo Italia, Dec 18 '10 at 13:17
Ack, my bad! Thank you for the correction, I mistook `strtok` for `strcspn`, which returns a `size_t`. Sorry, please disregard my initial remark. — mingos, Dec 18 '10 at 13:21
/almost/ exactly as arrays... The sizeof operator will have different behaviour. :) — Arafangion, Dec 18 '10 at 13:23

score 5 · Accepted Answer · answered Dec 18 '10 at 13:12

5

Are you sure words is a modifiable string?

Literal strings are unmodifiable strings. For example: this gives segmentation fault:

char *test = "forty two";
test[6] = 'T'; /* make two uppercase */

You need to show how you call getwords and the definitions of the variables involved.
I'm guessing you're passing pointers to string literals.

answered Dec 18 '10 at 13:12

pmg

106,608
13
126
198

+1, although this is implementation defined, it's bad practice to assume that they're modifiable. Also note that strtok itself will modify the line. – Arafangion Dec 18 '10 at 13:14
2

@Arafangion: Modifying a string literal is actually undefined behaviour. – Martin v. Löwis Dec 18 '10 at 13:20
Indeed, this is not implementation-defined. Attempting to modify a literal string is an error. – R.. GitHub STOP HELPING ICE Dec 18 '10 at 13:24
@Martin: I thought it was implementation defined? – Arafangion Dec 18 '10 at 13:24
C99, 6.4.5p6: If the program attempts to modify such an array, the behavior is undefined. – Martin v. Löwis Dec 18 '10 at 13:27
@Arafangion: the standard can't *stop* an implementation from defining what happens in a particular case of UB, so perhaps some particular implementation has confused you by defining it. But modifying a string literal is certainly UB (6.4.5/6), and just because some implementations define it doesn't mean it's "implementation-defined" in the meaning of the standard. – Steve Jessop Dec 18 '10 at 13:28
What does C89 say about this? The Microsoft C Compiler is still C89 (at best) – Arafangion Dec 18 '10 at 13:28
C89 3.1.4 **semantics** "If the program attempts to modify a string literal of either form, the behavior is undefined." The either form refers to regular ( `"regular"` ) or wide ( `L"wide"` ) string literals. – pmg Dec 18 '10 at 13:34
@pmg: I was calling it with variable test, and test is declared as char *test[10];....First I declared it as char *test[10]={"Empty"}; but after i read your comment I changed it and it still gave segmentation fault. and line is actually "hello\t." – Syntax_Error Dec 18 '10 at 15:45
@Syntax_Error: `char *test[10] = {"Empty"};` defines an array of 10 pointers to char of which all but the first point to NULL. The first pointer, `test[0]` points to the literal string `"Empty"`. Maybe you need `char test[10][1042] = {"Empty", "", "", "", ""};` (see [codepad](http://codepad.org/HHzU72by)) but replace the 1042 with the maximum size your strings will ever have. – pmg Dec 18 '10 at 18:02

score 2 · Answer 2 · answered Dec 18 '10 at 13:22

There are two, perhaps four mistakes in the code below, I explain two of the mistakes in the code comments:

If we assume that "line", for the purposes of explaining what happens, is "hey\tthere"...
We also assume that "words" is an array of two pointers to char.

// Find the first token, in this case "hey", and return it in "result".
result = strtok(line,TABS); // NOTE: 'line' has been modified by the function!

// Take the last character in the returned statement, which is 'y', and
// copy it to the second cell in the 'words' array, however we are supposed
// to be copying a *pointer* to char there...
words[1]=result[strlen(result)-1];

Additionally, if "line" is static and can not be changed, the first line above will crash. If "words" is not allocated or doesn't reference an array of at least two pointers to char, then the second line will crash.

If code execution gets past this point, any code that uses the "words" array will crash because the code will expect pointers, but is getting chars!

@Aragangion: any suggestions on how I might solve this issue? — Syntax_Error, Dec 18 '10 at 15:47
@Syntax_Error: There are so many things wrong with that code, you need to learn C, and rewrite the function entirely. When rewriting that function, I suggest first sitting down and describe *exactly* how that function should work. Does it return all the words, or only split the line into two? Does it expect an array to store the words in? If so, how big is that array? What if it's too small? What if it's too big? If the function is expected to allocate its own array, how will it return that to the caller? — Arafangion, Dec 19 '10 at 00:38

Error when setting a character to a string in C

2 Answers2