Thread: strtok() strcpy() and reversing words

When I compile this, the error that I keep getting back is
[I]'temp' may be used uninitialized in this function *temp=next;
and when I run it, it gives me segmentation fault (core dump).

I am testing two different files:
File 1) birds and bees
- the output is supposed to be:
Line: birds and bees
Reversed: bees and birds
File 2)
Line: Now is the time for all good men to come to the aid of their country
Reversed: country their of aid the to come to men good all for time the is Now

Also, I'm not too sure if my logic works out since I haven't been able to run it, so can someone help me out with that.

Code:

#include <stdio.h>
#include <string.h>
#define MAX 79


int main(int argc, char *argv[])
{
  char *temp, *first;
  int i=10;
  char next[i];
  char line[MAX];
  char line_copy[MAX];
  FILE *fp;
  //opens files
  fp=fopen(argv[1],"r");

  //gets and print line
  if (fgets(line, MAX, fp)!=NULL)
     printf ("Line: ");
     puts(line);
     printf("Reversed: ");
   
  strcpy(line_copy,line);
  //pulling the first substring out
  first= strtok(line_copy, " ");

 //pulling out substrings after the first
  for (i=0;i<=strlen(line);i++){
    next[i]=strtok(NULL, " ");

    //swapping the words
    for (i=strlen(line);i>=0;i--)
      temp=0;
      *temp=next[i];
      next[i]=next[i-1];
      next[i-1]= *temp;
      printf("%s", next);   
  }
  printf("%s",first);
          
 
  fclose(fp);
  return 0;
}

If you get compiler errors you can't run your program because it didn't actually compile, though you may be running a stale version. If it's just warnings, it will produce an executable, but you shouldn't run it. Those warnings usually hint at you doing something that will cause problems (like seg faults). Note, you should compile at the maximum warning level and fix all warnings.

[I]> 'temp' may be used uninitialized in this function *temp=next;
That is not the error I get. In fact, the code you posted does not have such an error. You initialize temp on line 33. If you get that warning but see the exact code above in your editor, it's possible that you didn't save your latest changes before compiling.

However there is still a problem on lines 33-34. You initialize temp to 0 (which is the same as NULL). Then, on line 34, you try to dereference it by doing *temp. Dereferencing NULL results in undefined behavior, which is probably manifesting as a seg fault. You need to make temp point to memory you are allowed to write to (e.g. something from malloc, or another char variable or char array) if you want to actually store (write) data where temp is pointing.

Now, when I compile the above code, I get the following

Code:

$ make revwords
gcc -g -Wall -std=c99 -o revwords revwords.c -lpthread -lm -lpq
revwords.c: In function 'main':
revwords.c:29:12: warning: assignment makes integer from pointer without a cast [enabled by default]

strtok returns a char *, but next is an array of char, so next[i] is just a single char. You probably meant char *next[i];

A few other things:
1. You should check the result of fopen. If it fails, print a useful message (look into the perror function) and exit. No point in continuing if you have no data.
2. You need to check the return of strtok. It returns NULL when it's done tokenizing. Use that to control your loop
3. You shouldn't need char *first; and char *next[]; Just use the next array (renaming it to words would be better).
4. You don't do anything useful with line_copy. Just get rid of it.
5. If your files can contain multiple lines, you should make line 18 a while loop not an if.

Might be more, but that should get you going for now.