What's wrong with my code here???

Hello, everybody. Need help with this program. It's meant to take a pattern 1 and replace the first occurance in a string with pattern 2. The patterns are case sensitive. I keep getting a segmentation fault.

Here's an example of what the code should do:

Text before code:

Chris is great!
Is
is

Text after code with i as pattern 1 and x as pattern 2.

Chrxs is great!
Is
xs

Here's my code:

Code:

---

#include <unistd.h>
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <fcntl.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <errno.h>

int main(int argc, char *argv[])
{
 char str[5];        //Char to hold .out
 char * line;        //Char to hold line read in.
 char * pch;        //Char to hold first occurance of pattern
 char * string;      //Char to hold the entire string.
 char * string2;    //Makes a second array to hold the modified string.
 char * replace1;    //Char to hold string of pattern to replace.
 char * replace2;    //Char to hold string of pattern to replace with.
 FILE * inFILE;      //Name of file to read in.
 char * outFILE;    //Name of outfile.
 int in, out;        //Int to hold the return values of in, out, and nread.

 if (argc != 4)
 {
  fprintf(stderr,"Not enough arguments.\n"); //Prints out error message if there is not
                                            //enough arguments.
  exit(1); //Exits the program and returns failure.
 }
 strcpy (str, ".out"); //Copies .out to str.
 replace1 = argv[2]; //Gets the pattern to be replaced from the arguments.
 replace2 = argv[3]; //Gets the pattern that will replace pattern one from the arguments.
 string = (char*) malloc (101); //Allocates memory for string.
 inFILE = fopen(argv[1],"r"); //Opens the file.
 outFILE = argv[1]; //Gets the name of the file.
 strncat (outFILE,str,5); //Adds .out to that end of the file.
 out = open(outFILE, O_RDWR|O_CREAT, S_IRUSR|S_IRGRP|S_IROTH|S_IWUSR); //Uses system calls
 //to open the outfile and create permissions for the file.

 if (out == -1)
 {
  fprintf(stderr, "Error with system calls.\n"); //Prints error message if there is an error
                                                //opening the outfile.
  exit(1); //Exits the program and returns failure.
 }
 if (inFILE == NULL)
 {
  fprintf(stderr, "Error opening file.\n"); //Prints error message if there is an error
                                            //opening the infile.
  exit(1); //Exits the program and returns failure.
 }
 if (!feof(inFILE))
 {
  line = fgets(string, 101, inFILE); //Gets a string from the array.
  pch = strstr (line,replace1); //Finds the first occurance of replace1.
  int index; //Declares an int variable named index.
  string2 = (char *) malloc (101); //Allocates memory for string2.
  index = strlen(string2); //Makes index equal to the length of string2
  if (pch == NULL)
  strcpy (string2, string); //Copies string if pch is null.
  else
  memcpy (string2, line, (strlen(string) - strlen(pch))); //Copies the string up to the
                                                          //first occurance of replace1.
  string2[index] = '\0'; //Adds null terminating value to the end of string2.
  strncat (string2, replace2, strlen(replace2)); //Adds replace2 to the modified string.
  strncat (string2, pch + strlen(replace1), (strlen(pch) - strlen(replace1))); //Adds the
  //rest of the string to the modified string.

  write (out, string2, strlen(string2)); //Writes the string to file.
 }
 else
  fprintf(stderr, "Error detected in file.\n"); //Prints error message if there is an error
                                                //with the file.
  exit(1); //Exits the program and returns failure.
 if (out == -1)
 {
  fprintf(stderr, "Error with system calls.\n"); //Prints error message if there is an error
                                                //closeing the file.
  exit(1); //Exits the program and returns failure.
 }
 close(out); //Closes the output file.
 fclose (inFILE); //Closes the input file.
 free (string2); //Frees the memory used to create string2.
 free (string); //Frees the memory used to create string.
 exit(0); //Exits program and returns success.
}

---

Any help given will be greated appreciated.

Thanks!!!

Signed,

Chris Beech

First thing, use some vertical white space (aka, blank lines) to separate separate sections of the program.

> string = (char*) malloc (101);
Do not cast malloc in C, see the FAQ for why not.

> if (!feof(inFILE))
Also a bad idea, and it's also in the FAQ
Simply opening a file (even one with zero length) will not set feof() to return true, so in this respect it is a useless test (it will always pass).

> strncat (outFILE,str,5);
Whilst modifying argv strings is allowed (I think), making them larger is full of all sorts of problems since you've no idea how much space is allocated to each one, beyond the \0 at the end of each string.
If it's laid out in memory as
arg1\0arg2\0arg3\0
then modifying arg1 trashes arg2

> out = open(outFILE, O_RDWR|O_CREAT, S_IRUSR|S_IRGRP|S_IROTH|S_IWUSR);
Why the change from using fopen?
Mixing styles is not usually a good idea.

> strncat (string2, replace2, strlen(replace2));
You have to be pretty careful with strncat. Whilst it helps prevent buffer overflows, you also have to make sure that a \0 is copied as well (which strncat doesn't always do).

I need to use System calls with the outfile.

Changed the program a little:

Code:

---

#include <unistd.h>
#include <stdio.h>
#include <string.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>

int main(int argc, char *argv[])
{
 char str[5];        //Char to hold .out
 char * line;        //Char to hold line read in.
 char * pch;        //Char to hold first occurance of pattern
 char * string;      //Char to hold the entire string.
 char * string2;    //Makes a second array to hold the modified string.
 char * replace1;    //Char to hold string of pattern to replace.
 char * replace2;    //Char to hold string of pattern to replace with.
 FILE * inFILE;      //Name of file to read in.
 char * outFILE;    //Name of outfile.
 int in, out;        //Int to hold the return values of in and out.
 int index;          //Declares an int variable named index.

 if (argc != 4)
 {
  printf("Hello.");
  fprintf(stderr,"Not enough arguments.\n"); //Prints out error message if there is not
                                            //enough arguments.
  exit(1); //Exits the program and returns failure.
 }
 
 strcpy (str, ".out"); //Copies .out to str.
 replace1 = argv[2]; //Gets the pattern to be replaced from the arguments.
 replace2 = argv[3]; //Gets the pattern that will replace pattern one from the arguments.
 string = malloc (101); //Allocates memory for string.
 inFILE = fopen(argv[1],"r"); //Opens the file.
 outFILE = argv[1]; //Gets the name of the file.
 strncat (outFILE,str,5); //Adds .out to that end of the file.
 out = open(outFILE, O_RDWR|O_CREAT, S_IRUSR|S_IRGRP|S_IROTH|S_IWUSR); //Uses system calls
 //to open the outfile and create permissions for the file.
 
 if (out == -1)
 {
  fprintf(stderr, "Error with system calls.\n"); //Prints error message if there is an error
                                                //opening the outfile.
  exit(1); //Exits the program and returns failure.
 }
 if (inFILE == NULL)
 {
  fprintf(stderr, "Error opening file.\n"); //Prints error message if there is an error
                                            //opening the infile.
  exit(1); //Exits the program and returns failure.
 }
 
 if (!feof(inFILE))
 {
  line = fgets(string, 101, inFILE); //Gets a string from the array.
  pch = strstr (line,replace1); //Finds the first occurance of replace1.
  string2 = malloc (101); //Allocates memory for string2.
  if (pch == NULL)
  strcpy (string2, string); //Copies string if pch is null.
  else
  index = strlen(string) - strlen(pch); //Makes index equal to the length of string minus length of pch.
  memcpy (string2, line, (strlen(string) - strlen(pch))); //Copies the string up to the
                                                          //first occurance of replace1.
  string2[index] = '\0'; //Adds null terminating value to the end of string2.
  strcat (string2, replace2); //Adds replace2 to the modified string.
  strcat (string2, pch + strlen(replace1)); //Adds the
  //rest of the string to the modified string.

  write (out, string2, strlen(string2)); //Writes the string to file.
 }
 else
  fprintf(stderr, "Error detected in file.\n"); //Prints error message if there is an error
                                                //with the file.
  exit(1); //Exits the program and returns failure.
 if (out == -1)
 {
  fprintf(stderr, "Error with system calls.\n"); //Prints error message if there is an error
                                                //closeing the file.
  exit(1); //Exits the program and returns failure.
 }
 close(out); //Closes the output file.
 fclose (inFILE); //Closes the input file.
 free (string2); //Frees the memory used to create string2.
 free (string); //Frees the memory used to create string.
 exit(0); //Exits program and returns success.
}

---

To me, it appears that you are doing a lot of unnecessary work. Most of the things you are doing could be accomplished simply with a proper use of sscanf and sprintf.

> outFILE = argv[1]; //Gets the name of the file.
> strncat (outFILE,str,5); //Adds .out to that end of the file.
You're still strncating to argv. As Salem said, this is a bad idea. You should make outFile a char array like you did with str, making it big enough to hold whatever length string the final product will contain. Then use strcpy for this line:
> outFILE = argv[1];

Code:

---

strcpy(outFile, argv[1]);

---

I changed some of the code again. Same segmentation fault error.

Code:

---

#include <unistd.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>

int main(int argc, char *argv[])
{
 char str[5];      //Char to hold .out
 char outFILE[100]; //Char to hold name of outfile.
 char * line;      //Char to hold line read in.
 char * pch;        //Char to hold first occurance of pattern
 char * string;    //Char to hold the entire string.
 char * string2;    //Makes a second array to hold the modified string.
 char * replace1;  //Char to hold string of pattern to replace.
 char * replace2;  //Char to hold string of pattern to replace with.
 FILE * inFILE;    //Name of file to read in.
 int in, out;      //Int to hold the return values of in and out.
 int index;        //Declares an int variable named index.

 if (argc != 4)
 {
  fprintf(stderr,"Not enough arguments.\n"); //Prints out error message if there is not
                                            //enough arguments.
  exit(1); //Exits the program and returns failure.
 }

 strcpy (str, ".out"); //Copies .out to str.
 replace1 = argv[2]; //Gets the pattern to be replaced from the arguments.
 replace2 = argv[3]; //Gets the pattern that will replace pattern one from the arguments.
 string = malloc (101); //Allocates memory for string.
 inFILE = fopen(argv[1],"r"); //Opens the file.
 strcpy(outFILE, argv[1]); //Copies filename to outFILE.
 strncat (outFILE,str,5); //Adds .out to that end of the file.
 out = open(outFILE, O_RDWR|O_CREAT, S_IRUSR|S_IRGRP|S_IROTH|S_IWUSR); //Uses system
 //calls to open the outfile and create permissions for the file.

 if (out == -1)
 {
  fprintf(stderr, "Error with system calls.\n"); //Prints error message if there is an error
                                                //opening the outfile.
  exit(1); //Exits the program and returns failure.
 }

 if (inFILE == NULL)
 {
  fprintf(stderr, "Error opening file.\n"); //Prints error message if there is an error
                                            //opening the infile.
  exit(1); //Exits the program and returns failure.
 }

 else
 {
  while (!feof(inFILE))
  {
  line = fgets(string, 101, inFILE); //Gets a string from the array.
  pch = strstr (line,replace1); //Finds the first occurance of replace1.
  string2 = malloc (101); //Allocates memory for string2.

  if (pch == NULL)
  {
    strcpy (string2, string); //Copies string if pch is null.
  }

  else
  {
    index = strlen(string) - strlen(pch); //Makes index equal to the length of string minus
                                        //length of pch.
    memcpy (string2, line, (strlen(string) - strlen(pch))); //Copies the string up to the
                                                          //first occurance of replace1.
    string2[index] = '\0'; //Adds null terminating value to the end of string2.
    strcat (string2, replace2); //Adds replace2 to the modified string.
    strcat (string2, pch + strlen(replace1)); //Adds the rest of the string to the modified
                                            //string.
    write (out, string2, strlen(string2)); //Writes the string to file.
  }
  }
 }

 if (out == -1)
 {
  fprintf(stderr, "Error with system calls.\n"); //Prints error message if there is an error
                                                //closeing the file.
  exit(1); //Exits the program and returns failure.
  }

 close(out); //Closes the output file.
 fclose (inFILE); //Closes the input file.
 free (string); //Frees the memory used to create string.
 free (string2); //Frees the memory used to create string2.
 exit(0); //Exits program and returns success.
}

---

Except now I can't seem to find a place to put the

Code:

---

fprintf(stderr, "Error detected in file.\n"); //Prints error message if there is an error
                                              //with the file.
exit(1); //Exits the program and returns failure.

---

Right after this line:

inFILE = fopen(argv[1],"r"); //Opens the file.

Adak

> while (!feof(inFILE))
This should be
while ( (line = fgets(string, 101, inFILE)) != NULL )

Because at EOF, fgets will return NULL, which you then try and do strstr on.

> string2 = malloc (101);
You do this for EVERY line, but you only call free() once.
Big memory leak here.

You may as well just allocate it once, like you do with string1

Also, there is little point in calling malloc at all,
char string[101], string2[101];
would work just as well.

Ok, I got rid of that annoying segmentation fault. Now it's not writting anything.

Code:

---

#include <unistd.h>
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>

int main(int argc, char * argv[])
{

 char str [5];      //Char to hold .out
 char outFILE [101]; //Char to hold name of outfile.
 char * line;        //Char to hold line read in.
 char * pch;        //Char to hold first occurance of pattern
 char string[101];  //Char to hold the entire string.
 char string2[101];  //Makes a second array to hold the modified string.
 char * replace1;    //Char to hold string of pattern to replace.
 char * replace2;    //Char to hold string of pattern to replace with.
 FILE * inFILE;      //Name of file to read in.
 int in, out;        //Int to hold the return values of in and out.
 int index;          //Declares an int variable named index.

 if (argc != 4)
 {
  fprintf(stderr,"Not enough arguments.\n"); //Prints out error message if there is not
                                            //enough arguments.
  exit(1); //Exits the program and returns failure.
 }

 strcpy (str, ".out"); //Copies .out to str.
 replace1 = argv[2]; //Gets the pattern to be replaced from the arguments.
 replace2 = argv[3]; //Gets the pattern that will replace pattern one from the arguments.
 inFILE = fopen(argv[1],"r"); //Opens the file.
 strcpy(outFILE, argv[1]); //Copies filename to outFILE.
 strncat (outFILE,str,5); //Adds .out to that end of the file.
 out = open(outFILE, O_RDWR|O_CREAT, S_IRUSR|S_IRGRP|S_IROTH|S_IWUSR); //Uses system
 //calls to open the outfile and create permissions for the file.

 if (out == -1)
 {
  fprintf(stderr, "Error with system calls.\n"); //Prints error message if there is an error
                                                //opening the outfile.
  exit(1); //Exits the program and returns failure.
 }

 if (inFILE == NULL)
 {
  fprintf(stderr, "Error opening file.\n"); //Prints error message if there is an error
                                            //opening the infile.
  exit(1); //Exits the program and returns failure.
 }

 else
 {
  while((line = fgets(string,101,inFILE)) != NULL)
  {
  line = fgets (string,101,inFILE); //Gets a string from the array.
  pch = strstr (line,replace1); //Finds the first occurance of replace1.

  if (pch == NULL)
  {
    strcpy (outFILE, string); //Copies string if pch is null.
  }

  else
  {
    index = strlen(string) - strlen(pch); //Makes index equal to the length of string
                                          //minus length of pch.
    memcpy (string2, line, (strlen(string) - strlen(pch))); //Copies the string up to the
                                                            //first occurance of replace1.
    string2[index] = '\0'; //Adds null terminating value to the end of string2.
    strcat (string2, replace2); //Adds replace2 to the modified string.
    strcat (string2, pch + strlen(replace1)); //Adds the rest of the string to the modified
                                              //string.
    write (out, string2, strlen(string2)); //Writes the string to file.
  }
  }
 }

 if (out == -1)
 {
  fprintf(stderr, "Error with system calls.\n"); //Prints error message if there is an error
                                                //closeing the file.
  exit(1); //Exits the program and returns failure.
  }

 close(out); //Closes the output file.
 fclose (inFILE); //Closes the input file.
 exit(0); //Exits program and returns success.
}

---

Also, I'm not sure if I'm doing the error checking correctly. The way I'm doing it is print an appropriate error message on standard error and exiting, right??? Also, am I checking all the possible errors???

Also, thanks for the help so far!!!

Code:

---

  while((line = fgets(string,101,inFILE)) != NULL)
  {
// delete this line  line = fgets (string,101,inFILE); //Gets a string from the array.

---

The while loop now reads a line AND continues with the body of the loop if the read was successful.

Okay. It mostly works now. However, it's doing strange things if the outfile already exists. It's supposed to overwrite the original content. Here's an example of what its doing.

Input text

Kenny is great!
Because Kenny treats his students very well and never breaks his promises

Read the following sentence carefully: read the textbook!
Or you will fail CS306

first run with rea as pattern1 and 12?45 as pattern2

Kenny is g12?45t!
Because Kenny t12?45ts his students very well and never breaks his promises

Read the following sentence carefully: 12?45d the textbook!
Or you will fail CS306

second run with input text using Kenny as pattern1 and a as pattern2

a is great!
Because a treats his students very well and never breaks his promises

Read the following sentence carefully: read the textbook!
Or you will fail CS306
ll fail CS306

Where's that last ll fail CS306 coming from???

Code:

---

#include <unistd.h>
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <errno.h>

int main(int argc, char * argv[])
{

 char str[5];        //Char to hold .out
 char outFILE[101];  //Char to hold name of outfile.
 char string[101];  //Char to hold the entire string.
 char string2[101];  //Makes a second array to hold the modified string.
 char * line;        //Char to hold line read in.
 char * pch;        //Char to hold first occurance of pattern.
 char * replace1;    //Char to hold string of pattern to replace.
 char * replace2;    //Char to hold string of pattern to replace with.
 FILE * inFILE;      //Name of file to read in.
 int in, out;        //Int to hold the return values of in and out.
 int index;          //Declares an int variable named index.

 if (argc != 4)
 {
  printf("Not enough arguments.\n", strerror(errno)); //Prints out error message if
                                                      //there is not enough arguments.
  exit(1); //Exits the program and returns failure.
 }

 strcpy (str, ".out"); //Copies .out to str.
 replace1 = argv[2]; //Gets the pattern to be replaced from the arguments.
 replace2 = argv[3]; //Gets the pattern that will replace pattern one from the arguments.
 inFILE = fopen(argv[1],"r"); //Opens the file.
 strcpy(outFILE, argv[1]); //Copies filename to outFILE.
 strncat (outFILE,str,5); //Adds .out to that end of the file.
 out = open(outFILE, O_RDWR|O_CREAT, S_IRUSR|S_IRGRP|S_IROTH|S_IWUSR); //Uses system
 //calls to open the outfile and create permissions for the file.

 if (out == -1)
 {
  printf("Error with system calls.\n", strerror(errno)); //Prints error message if there is
                                                        //an error opening the outfile.
  exit(1); //Exits the program and returns failure.
 }

 if (inFILE == NULL)
 {
  printf("Error opening file.\n",strerror(errno)); //Prints error message if there is an
                                                  //error opening the infile.
  exit(1); //Exits the program and returns failure.
 }

 else
 {
  while((line = fgets(string,101,inFILE)) != NULL)
  {
  pch = strstr (line,replace1); //Finds the first occurance of replace1.

  if (pch == NULL)
  {
    write (out, string, strlen(string)); //Copies string to out if pch is null.
  }

  else
  {
    index = strlen(string) - strlen(pch); //Makes index equal to the length of string
                                          //minus length of pch.
    memcpy (string2, line, (strlen(string) - strlen(pch))); //Copies the string up to the
                                                            //first occurance of replace1.
    string2[index] = '\0'; //Adds null terminating value to the end of string2.
    strcat (string2, replace2); //Adds replace2 to the modified string.
    strcat (string2, pch + strlen(replace1)); //Adds the rest of the string to the modified
                                              //string.
    write (out, string2, strlen(string2)); //Writes the string to file.
  }
  }
 }

 if (out == -1)
 {
  printf("Error with system calls.\n",strerror(errno)); //Prints error message if there is
                                                        //an error closeing the file.
  exit(1); //Exits the program and returns failure.
 }

 close(out); //Closes the output file.
 fclose (inFILE); //Closes the input file.
 exit(0); //Exits program and returns success.
}

---

Also, you missed one of the "rea" in break. . . but I haven't looked at your code to answer your original question.

EDIT: Oh come on!!! do some good indenting. . . Boy howdy, that really hurts my eyes to even look at it!

Quote:

---

Originally Posted by Kennedy

Also, you missed one of the "rea" in break. . . but I haven't looked at your code to answer your original question.

---

It only replaces the first occurance of pattern1 per line.

And what's wrong with the indenting??? Everything is lined up perfectly.

EDIT: Well, not everything. One } needed to be moved one space to the left.

>It only replaces the first occurance of pattern1 per line.
Sure, that's all you tell it to replace.

>Everything is lined up perfectly.
Exactly. :D A one space indent is generally not enough. At the very least, use two spaces, though more than four starts to get too much of a roomy feeling.

Quote:

---

Exactly. A one space indent is generally not enough. At the very least, use two spaces, though more than four starts to get too much of a roomy feeling.

---

One space indendation is not more readable than without any indentation. It just looks so strange, as you've tried to squeeze the code so there would be less free space.

Ok, I get the point. I'll indent more.

Now, where's that last ll fail CS306 coming from???

>Now, where's that last ll fail CS306 coming from???
Probably because you're opening the output file in read/write mode versus write mode:
>out = open(outFILE, O_RDWR|O_CREAT, S_IRUSR|S_IRGRP|S_IROTH|S_IWUSR); //Uses system

Opening it in write mode should truncate the file first.