0

I'm trying to solve this question regarding moving data from a file to another in C. Running the program gives a segmentation error 11. I've attached a picture of the question. Exercise 4

I believe there is a problem in opening the files, I entered inside the terminal the C code script name: code.c file1.txt file2.bin -b. The files are included in the path.

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

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

  size_t k;
  char read1[100] = {};
  FILE* s;
  FILE* d;
  if (argc < 4) {
    printf("NA");
    exit(1);
  }

  if (strcmp(argv[4], "-b") == 0) {
    printf("binary file output\n");
    d = fopen(argv[3], "wb");
    if (d == NULL) {
      printf("cant open d");
      exit(1);
    }
  } else {
    if (strcmp(argv[4], "-t") == 0) {
      printf("textual file output\n");
      d = fopen(argv[3], "w");
    } else {
      printf("error");
      exit(1);
    }
  }

  s = fopen(argv[2], "r");
  if (s == NULL) {
    printf("cant open s");
    exit(2);
  }
  k = fread(read1,  sizeof(char),100, s);
  while (k != 0) {
    fwrite(read1,  k,1, s);
    k = fread(read1,  sizeof(char),100, s);
  }
  fwrite(read1,  k,1, s);
  fclose(s);
  fclose(d);

  return 1;
}

I expect to move all the data from file 1 to file 2, and file2 output can be binary or textual depending on the user input stream. Ignored the 'hexadecimal' case.

Marco Bonelli
  • 63,369
  • 21
  • 118
  • 128
  • Well, first look up what strlen() does. man strlen, or just Google it. – Martin James Aug 26 '19 at 17:15
  • 2
    Please don't post pictures of text. Copy the text into the question, which is more friendly to screen readers, makes it easy to copy-and-paste and perform diffs on and takes up less space. Thanks. – ggorlen Aug 26 '19 at 17:17
  • Then do the same with fread(), especially note that it returns something essential that cannot be ignored. – Martin James Aug 26 '19 at 17:17
  • 2
    Before you use `argv[4]` please check `argc`. And before you use `fread` please check that `fopen` returned a valid pointer. Without those checks, you are liable to get faults you can't detect. Aside: it's unwise to name the variable `read` the same as a library function. – Weather Vane Aug 26 '19 at 17:23
  • @WeatherVane I've done so, still the same! Regarding strlen(), I've read about it, I've replaced it with a sizeof(char), with 100 characters to read into my character array. still I'm facing the same problem, I wonder why – Mohammed Misto Aug 26 '19 at 17:33
  • 1
    As hinted above, you also need to store the returned value from `fread` and use that in `fwrite`. Also, please see [Why is `while ( !feof (file) )` always wrong?](http://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong) The loop should be driven by the return value from `fread`. Moreover, you should reverse the 2nd and 3rd arguments to `fread` and `fwrite`: `size` and `count`. – Weather Vane Aug 26 '19 at 17:44
  • ...For example `size_t bytes; while((bytes = fread(read, 1, sizeof read, s)) != 0) { fwrite(read, 1, bytes, d); }` – Weather Vane Aug 26 '19 at 17:50
  • Hi thanks WeatherVane for following, I've edited the post following what you suggested, still the error is occurrin, I removed strlen and replaced it with sizeof(char),100 in the fread as well – Mohammed Misto Aug 26 '19 at 18:00
  • 1
    You have tinkered with the code posted, but it's no longer the real code, because accessing `argv[4]` requires that `argc` be `5` or more. And, you haven't reversed the arguments for [`fread`](https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/fread?view=vs-2017) and [`fwrite`](https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/fwrite?view=vs-2017) as advised. – Weather Vane Aug 26 '19 at 18:09
  • Yeah I've missed that, just fixed it, nothing is working I dont know – Mohammed Misto Aug 26 '19 at 18:23
  • 1
    You have not changed the `fwrite`, and, there shouldn't be any `fwrite` *after* the loop. Please be careful. Aside: `sizeof (char)` is `1` by definition. – Weather Vane Aug 26 '19 at 18:32
  • Agreed with @WeatherVane: I've had bad experiences naming variables or functions the same name as libc exports. – Matthieu Aug 26 '19 at 18:48

1 Answers1

2

You seem to want to write a program that takes the name of an input file, an output file and a flag (-b or -t), so I guess you're calling your program like this:

program infile outfile [-b|-t]

Those are 3 arguments. They will be argv[1], argv[2] and argv[3] respectively. You should not access argv[4]. Your program will segfault on strcmp(argv[4], "-b"). All your argv[x] should be shifted back by one. The check if (argc < 4) is ok though.

Another thing that can cause segmentation fault is reading from a FILE* that is not valid. You're not checking if d == NULL after the second fopen(). You should do that, and exit with an error in case it's NULL.

Other than this, other problems with your code are:

  1. You should not call fwrite after exiting the while loop. You know that k == 0 when out of the loop. It is not harmful, but it's useless and will print nothing.

  2. You should reorder the arguments of fwrite like this: fwrite(read1, 1, k, s).

  3. Your last return 1 statement makes no sense, you should return 0, not 1, for sucessful program execution.

  4. You don't need to initialize the array with char read1[100] = {}; since you don't use it before overwriting its content. Doing char read[100]; is just fine.

PS: you should learn to use GDB to debug your programs. Problems like this one are very easy to spot using GDB by just stepping through the instructions one by one.

Marco Bonelli
  • 63,369
  • 21
  • 118
  • 128