Getting segmentation fault in fread/fwrite loop, copying file

Question

I am to copy a file from the inputed source to the inputed destination. I am getting a segmentation fault error about 2 seconds after the destination is entered. The output file is created so fopen() is working.

I looked online and I see a lot of c=getc(fp). I prefer fread() and fwrite() for this question because it's a little more basic.

Code

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

//void rmnewline(char *string);
void remove_last_newline(char *str);

int main()
{
  char x[3];
  x[0]='y';
  int ch;

  while(x[0]=='y'||x[0]=='Y')
  {
    char source[256], destination[256];
    int *a;

    printf("Enter a source file: ");
    fgets(source, 256, stdin);
    if (source[254] == '\n' && source[255] == '\0') { while ( (ch = fgetc(stdin)) != EOF && ch != '\n'); }
    remove_last_newline(source);

    printf("Enter a destination file: ");
    fgets(destination, 256, stdin);
    if (destination[254] == '\n' && destination[255] == '\0') { while ( (ch = fgetc(stdin)) != EOF && ch != '\n'); }
    remove_last_newline(destination);

    FILE *sp, *dp;

    sp = fopen(source, "r");
    if (sp == NULL) { printf("ERROR: Could not open source file."); exit(1); }
    dp = fopen(destination, "w");
    if (dp == NULL) { printf("ERROR: Could not open destination file."); exit(1); }
    while(1)
    {
      fread(a, 1, 1, sp);
      if (feof(sp))
      break;
      fwrite(a, 1, 1, dp);
    }

    fclose(sp);
    fclose(dp);

    printf("Run Again?(y/n):");
    fgets(x, 2, stdin);
    while ( (ch = fgetc(stdin)) != EOF && ch != '\n');
 }

}

/*void rmnewline(char *string)
{
    int i, l = strlen(string);

    for(i=0; i<=l; i++)
    {
      if(string[i] == '\0')
         return;
      if(string[i] == '\n')
     {
       string[i] == '\0';
       return;
     }
  }

}*/

void remove_last_newline(char *str) {
    if (*str == '\0') return; /* do nothing if the string is empty */
    while(str[1] != '\0') str++; /* search for the end of the string */
    if (*str == '\n') *str = '\0'; /* if the last character is newline, delete it */
}

fgets() reads a line *including* the \n . You should remove that first. — wildplasser, Oct 03 '15 at 14:44
Conceptual error here: `if (source[254] == '\n' && source[255] == '\0')` (and again with the destination): it's not the last character in the *buffer* that may be a newline, but the last read character instead. — Jongware, Oct 03 '15 at 14:45
"The output file is created so fopen() is working" - but only for the write file. — Jongware, Oct 03 '15 at 14:48
If user types `Y` then enter, `fgets(x, 2, stdin);` only saves `"Y"` with its null character leaving the `'\n'` in `stdin` for the next input function. — chux - Reinstate Monica, Oct 03 '15 at 14:57
`fgets(source, 256, stdin); if (source[254] == '\n' && source[255] == '\0')` --> may lead to undefined behavior. Do not use `source` until check the return value from `fgets()`. — chux - Reinstate Monica, Oct 03 '15 at 15:00

score 2 · Accepted Answer · answered Oct 03 '15 at 14:46

2

fgets store the newline character read in the buffer.

It may prevent fopen to open "the specified file" and fopen may return NULL.

Please delete newline characters if they are in the buffer.
This can be done using this function:

void remove_last_newline(char *str) {
    if (*str == '\0') return; /* do nothing if the string is empty */
    while(str[1] != '\0') str++; /* search for the end of the string */
    if (*str == '\n') *str = '\0'; /* if the last character is newline, delete it */
}

please pass source and destination to this function after reading file names to them.

answered Oct 03 '15 at 14:46

MikeCAT

73,922
11
45
70

I will look into this, but could you explain why fread will not read my new line byte. I don't understand. How would new lines be added if the text file is a new line? EDIT: Nevermind, I understand – Andrew Ricci Oct 03 '15 at 15:01
Candidate simplification `void remove_last_newline(char *str) { str[strcspn(str, "\n")] = '\0'; }` – chux - Reinstate Monica Oct 03 '15 at 15:04
@Andrew Ricci `fread()` failed because `fopen()` failed. It is not a issue of new lines in the file. Robust code checks the results of all IO operations (`fgets, fopen, fread`, etc.) – chux - Reinstate Monica Oct 03 '15 at 15:08
@chux, I added the method after my own version of it failed. I still end up with seg fault. – Andrew Ricci Oct 03 '15 at 15:31
1

@Andrew Ricci Are all the return values from IO functions checked? If not, save debug time and add tests to your code. – chux - Reinstate Monica Oct 03 '15 at 15:39
@chux, I printed the source and destination string, they looks fine. And I checked the return value of fread and fwrite, again fine. – Andrew Ricci Oct 03 '15 at 15:52
Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/91256/discussion-between-chux-and-andrew-ricci). – chux - Reinstate Monica Oct 03 '15 at 15:55

score 1 · Answer 2 · answered Oct 03 '15 at 15:06

Check the return value of fopen.
The program is getting SEGFAULT at fread(a, 1, 1, sp); This happens when SP is NULL. Since the program trying to read only one byte there will not be any chance of overflow.

fopen fails because the source consists of newline character at the end. If you remove newline character from the source and destination before calling fopen your program will work fine.

score 0 · Answer 3 · answered Oct 03 '15 at 16:17

Main issue is leaving the '\n' in the filename and then the following fopen() failed, code did not check fopen() return value. Other 2 answers well answered that and at least 1 deserves acceptance.

Subsequent OP edit changed int a[1] to int *a causing a new fault. Suggest changing to

char a[1];

Getting segmentation fault in fread/fwrite loop, copying file

Code

3 Answers3