Pointers and strings, failing on EOF

Question

Code rewritten to be more clear

void indexe(char * line, unsigned ref) {
  unsigned i = 0;
  char word[128];  //(1)
  //char * word;   //(2)
  while(*line) {
    if(isalpha(*line))
      word[i++] = *line;  //(1)
      //*word++ = *line;  //(2)
    *line++;
  }
}

int main(int argc, const char * argv[]) {
  char line[128];
  FILE * f = fopen(argv[1], "r");
  unsigned x = 0;
  while (fgets(line, 128, f)){
    indexe(line, ++x);
  }
  fclose(f);
  return 0;
}

Hello, I have tried the two above combinations:

```
word[] -> word[i++]
```
```
*word -> *word++
```

The whole thing works flawlessly, except when reaching EOF, in which case the pointers syntax fails with a segmentation fault, but not the array syntax.

I am a C total beginner, could someone explain in beginner terms what happens here, and maybe suggest a solution to fix the pointer syntax? (But mostly explain, please)

str and idx are what? This isn't your actual code. It's also missing the closing } for main. — Randy Howard, Mar 14 '13 at 20:51
charles, it's either the two commented lines, or the two uncommented lines directly above these. It works flawlessly both ways... except when reading EOF with the pointer syntax. — pouzzler, Mar 14 '13 at 21:07
So you mean you are replacing the array definition with _only_ the pointer declaration? Are you initializing the pointer to point at any place in particular? — CB Bailey, Mar 14 '13 at 21:11
nope, I am not. When I initialized it, I got an instant segfault. When I didn't, it failed on EOF. I tried initializing with '\0', NULL, and "". — pouzzler, Mar 14 '13 at 21:12
Well, if you're writing to random memory locations then if it ever works it is a complete fluke. You must initialize a pointer to point at memory that is allocated for for programs use (whether dynamic or static) before you write to locations that it points at. — CB Bailey, Mar 14 '13 at 21:14
@carl norum this is my exact actual code (I took the smallest part of my actual code that replicated the problem). — pouzzler, Mar 14 '13 at 21:16
@Charles Bailey. I would I go about allocating memory in this case, and why would it fail at EOF, and nowhere else? — pouzzler, Mar 14 '13 at 21:19
Are you sure the code you've posted here fails? I can't see any reason it should. — teppic, Mar 14 '13 at 21:19
segfault on EOF, @teppic, when using the commented out //(2) lines instead of the //(1) lines. — pouzzler, Mar 14 '13 at 21:21
You could allocate another array and pointer `word` at that, but the array version is fine as it is. (e.g. `char buffer[128]; char *word = buffer;`) — CB Bailey, Mar 14 '13 at 21:25
If you use the //(2) lines it's crashing because you're not allocating any memory for the array. — teppic, Mar 14 '13 at 21:26
I think you need to get yourself a good introductory text book on C. — CB Bailey, Mar 14 '13 at 21:28
I also think so, however, if someone was so kind as to point my glaring mistake here, and why it especially fails on EOF, that would be most kind. Text books never answer specific questions like this, unless you buy them all, and have a lot of reading time on your hands. — pouzzler, Mar 14 '13 at 21:30
ok, so there is no way to make it work without the array one way or another. What is so strange is that it fails only on EOF. — pouzzler, Mar 14 '13 at 21:34
@pouzzler: try another input file that's much longer - I can guarantee it'll fail before EOF. You've got undefined behaviour. — teppic, Mar 14 '13 at 21:36
I have already pointed out the "glaring mistake", you are attempting to write to memory pointed at by an uninitialized pointer. If that doesn't mean much to you then you need to become a lot more familiar with C, hence I recommend a [good introductory text book](http://stackoverflow.com/questions/562303/the-definitive-c-book-guide-and-list). — CB Bailey, Mar 15 '13 at 07:45
Thanks everyone, charles and teppic principally. I accept the sum up below, as it is an answer and these are comments. All the best. — pouzzler, Mar 15 '13 at 08:40

teppic · Accepted Answer · 2013-03-14T21:39:21.193

This version, as posted, is fine:

void indexe(char * line, unsigned ref) {
  unsigned i = 0;
  char word[128];  //(1)
  //char * word;   //(2)
  while(*line) {
    if(isalpha(*line))
      word[i++] = *line;  //(1)
      //*word++ = *line;  //(2)
    *line++;
  }
}

However, if you recomment the code to use the lines marked //(2) instead, you have this:

char * word;   //(2
*word++ = *line;  //(2)

Which is simply a case of writing to a pointer you haven't initialised with allocated memory. This isn't allowed. You need to keep it as an array, or use something like malloc to reserve storage. If you want to write the function without using an array at all, the code would be:

char *word = malloc(128);    // reserve 128 bytes 
if (word == NULL) {          // these checks are important
   fprintf(stderr, "Cannot allocate memory!");
   exit(EXIT_FAILURE);
}
...other stuff...
free(word);

Note also that:

*line++;

increments line, but dereferences it (before it's incremented) for no reason.

score 0 · Answer 2 · answered Mar 15 '13 at 00:33

The pointer syntax fails because you have defined a pointer char * word; but you have not set it to point to any data - the memory you are pointing to can be anywhere. So, when you execute the following statement:

*word++ = *line;

You are storing the value pointed to by line in the value pointed to by word. Unfortunately, you do not know where word is pointing. As @teppic pointed out, you're writing to a pointer that has not been initialized to allocated memory.

You could malloc that memory as @teppic previously pointed out. You could also do the following:

char reserve[128];
char * word = reserve;  // could also have used &reserve[0]

Hope that helps!

Pointers and strings, failing on EOF

2 Answers2