0

I'm writing a Vanilla File read code.

Most of it look like this.

Firstly the header file file.h

// fheader.h
#ifndef __file_h__
#define __file_h__
// start from here
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
void readFile(FILE *fr);

FILE* openFile(char *file_name);
#endif

the main file fmain.c

#include "fheader.h"


void readFile(FILE *fr) {
    // Added Printf
    printf("\n...\n");
    printf("\n...\n");
    printf("\n...\n");
    printf("\n...\n");
  char *buffer = calloc(sizeof(char),1);
  while(!feof(fr)) {
    fread(buffer,sizeof(char),1,fr);
    printf("%s\n",buffer);
  }
  free(buffer);
  return;
}


FILE* openFile(char *file_name) {
  // printf("the file name that is going to be opened is %s",file_name);
  FILE *fr;
  fr = fopen(file_name,"r");
  return fr;
}

int main(int argc,char *argv[]) {
  if(argc < 2) {
    printf("USAGE: ./file test.txt\n");
    return 1;
  }

  if (argc > 2) {
    printf("ERROR: Too many argument\n");
    return 1;
  }
  FILE *fr;
  char *file_name = calloc(strlen(argv[1]),sizeof(char));

  strncpy(file_name,argv[1],strlen(argv[1]));
  fr = openFile(file_name);
  printf("\nReading from file\n");
  readFile(fr);
  fclose(fr);
  free(file_name);
  return 0;
}

I compiled the code using following command

gcc -g3 -Wall fmain.c -o file.o

When I ran the code

./file.o "~/workspaces/myWork/C_experiment/test.txt"

I see Segmentation fault: 11

But when I run the above program in lldb I work and exit with return code 0

lldb ./file.o
(lldb) run "~/workspaces/myWork/C_experiment/test.txt"
// output of the file 
Process 28806 exited with status = 0 (0x00000000)
(lldb) quit

Now, I'm clueless as to how to debug the code and find the Seg Fault reason.

Ratatouille
  • 1,372
  • 5
  • 23
  • 50
  • Use the core dump to at least see where it was when it crashed. – Ruslan Mar 10 '17 at 15:17
  • BTW, you didn't show the "following command" you compiled the code with. And `.o` filename extension makes me suspicious about correctness of that command. – Ruslan Mar 10 '17 at 15:20
  • You don't check of `openFile` succeeds. If `openFile` cannot open the file (e.g. because it doesn't exist) it returns `NULL` and `readfile` will then exhibit undefinedbehaviour (most likely crash). There are likely more problems. – Jabberwocky Mar 10 '17 at 15:23
  • `char *buffer = calloc(sizeof(char),1);` is pointless if the buffer size is known at compile time, write simply `char buffer[1]` (which is wrong anyway, see answer below). – Jabberwocky Mar 10 '17 at 15:29
  • @Ruslan Update the compile command. – Ratatouille Mar 10 '17 at 16:31
  • @MichaelWalz I know the point of doing this is also to familiarise myself with malloc,calloc famliy command. – Ratatouille Mar 10 '17 at 16:32
  • `#ifndef __file_h__` identifiers that start with one (or two) underscores are reserved. Dont use them. Just don't. – joop Mar 10 '17 at 16:52
  • @Ruslan How to do coredump and Clue here? – Ratatouille Mar 10 '17 at 17:10
  • Depends on your OS. For Linux see [this question](http://stackoverflow.com/questions/17965/), for MacOS X see [this one](http://stackoverflow.com/questions/9412156/). – Ruslan Mar 10 '17 at 19:32

1 Answers1

0

you forgot to add the '\0' at the end of the file_name and when you do a fread to read 1 char, you don't add a '\0' at the end of the buffer. And if you read only one character you should use a char instead of an array of 1.

The errors are here :

 char *file_name = calloc(strlen(argv[1]),sizeof(char));
  strncpy(file_name,argv[1],strlen(argv[1]));
  fr = openFile(file_name);

You copy the argv[1] but you don't add a '\0' at the end. So how fopen will know where your string stop ? You should add a +1 to calloc and after the strncpy, you should add the '\0' like that:

file_name[strlen(argv[1])] = '\0';

The second error is here:

 char *buffer = calloc(sizeof(char),1);
  while(!feof(fr)) {
    fread(buffer,sizeof(char),1,fr);
    printf("%s\n",buffer);
  }

You allocate 1 to your buffer and read 1, that's ok but when you send it to printf, you didn't add a '\0' to your buffer so how can printf know where to stop ? you should calloc 2 and not one and then add a buffer[1] = '\0';

So with these fixes:

#include "fheader.h"


void readFile(FILE *fr) {
  char buffer;
  while(!feof(fr)) {
    fread(&buffer,sizeof(char),1,fr);
    printf("%c\n",buffer);
  }
  return;
}


FILE* openFile(char *file_name) {
  // printf("the file name that is going to be opened is %s",file_name);
  FILE *fr;
  fr = fopen(file_name,"r");
  return fr;
}

int main(int argc,char *argv[]) {
  if(argc < 2) {
    printf("USAGE: ./file test.txt\n");
    return 1;
  }

  if (argc > 2) {
    printf("ERROR: Too many argument\n");
    return 1;
  }
  FILE * fr = openFile(argv[1]);
  printf("\nReading from file\n");
  readFile(fr);
  fclose(fr);
  return 0;
}
  • 1
    Anyway, don't use `strncpy` it may leave the destination string without a NUL terminator. – Jabberwocky Mar 10 '17 at 15:25
  • you can do that too :) but strncpy can be quite usefull to concatenate strings. – Gabrielle de Grimouard Mar 10 '17 at 15:27
  • `strncpy(file_name,argv[1],strlen(argv[1])); file_name[name_size] = '\0';` can be replaced by `strcpy(file_name, argv[1]);`. – Jabberwocky Mar 10 '17 at 15:32
  • And the code between `int name_size = strlen(argv[1]);` and `fr = openFile(file_name);` can be replaced by `fr = openFile(argv[1]);`. You don't need to copy the filename in `argv[1]`. – Jabberwocky Mar 10 '17 at 15:34
  • @MichaelWalz I used strncpy because I guess they are better again `Buffer Overflow attack` [http://stackoverflow.com/questions/1258550/why-should-you-use-strncpy-instead-of-strcpy](http://stackoverflow.com/questions/1258550/why-should-you-use-strncpy-instead-of-strcpy) – Ratatouille Mar 10 '17 at 16:35
  • @MichaelWalz Not to mention again the part of doing such things is to familarise with general practice of how safe C code are written. – Ratatouille Mar 10 '17 at 16:36
  • `strncpy()` is unusable. In your case, it will leave the buffer unterminated. Also: your allocated buffer for filename is 1 byte too small. – joop Mar 10 '17 at 16:39
  • @GabrieldeGrimouard Can you tell me what Exactly without changing the code what the problem in my code. I want to understand what evil I did. I'm happy take your answer as solution but I need to understand first what is wrong in mycode. even the printf that I have added at the beggining of the ReadFile do not get invoked – Ratatouille Mar 10 '17 at 16:41
  • @joop Right. But Like I said part of doing all this is to familarise with general practice of how, `safe` C code are written. – Ratatouille Mar 10 '17 at 16:43
  • @Ratatouille you did 2 wrong things. first the strncpy made a copy of argv[1] but you didn't add a '\0' after that. so you should add a strlen(argv[1])+1 to your calloc to add the '\0' at the end of file_name if you really want to make a strncpy. (just send argv[1] is better because you won't modify it). Second, when you read you make a calloc of 1 for buffer and you again, didn't add a '\0' after you read the character at the end so printf don't know where to stop. (you better have to change it to a char and send the address since you read 1 by 1 and then make a printf("%c", buffer)) – Gabrielle de Grimouard Mar 10 '17 at 16:46
  • @GabrieldeGrimouard Copy pasted your version of code. Still the same error. – Ratatouille Mar 10 '17 at 16:54
  • `strcpy` is perfectly safe here, because you know that the destination buffer is large enough to hold the string you are copying because you allocated the right quantity memory just before. – Jabberwocky Mar 10 '17 at 16:57
  • @Ratatouille you still get the segmentation fault ? Did you recompile it ? there is errors left in your code. else it can be your system. – Gabrielle de Grimouard Mar 10 '17 at 16:59
  • @GabrieldeGrimouard I found the issue It seem that command line argument is not able to Expand the `home directory syntax` i.e `~/workspaces/` but when I run it from lldb it is manage to understand that. Weird as it may sound I check the pointer after openFile it was `Null` and errorno and `strerror` `No such file or directory` – Ratatouille Mar 10 '17 at 17:05
  • @GabrieldeGrimouard Exactly as I thought now even my Version of code works. Anyway I'm accepting your answer sir. Since you enlightened me in this process. – Ratatouille Mar 10 '17 at 17:08
  • @Ratatouille that's nice but you still should remove the errors in your code since it will lead to some undefined behaviour and so not work everytime :) – Gabrielle de Grimouard Mar 10 '17 at 17:09
  • @GabrieldeGrimouard Very well. Thanks – Ratatouille Mar 10 '17 at 17:10
  • @MichaelWalz you are saying strcpy is safe in mycase because I created the buffer of exactly the same size as the `argv[1]`, right?. – Ratatouille Mar 10 '17 at 17:14
  • you need to malloc(strlen(argv[1])+1) one more if you use strcpy. as it still need place to add the '\0' but yes it's safe. – Gabrielle de Grimouard Mar 10 '17 at 17:20
  • @Ratatouille yes, more or less, _your_ code is wrong because you allocate not enough memory, you need one byte more for the NUL string terminator. But if you know that the destination buffer is long enough to hold the string you are copying, then strlen _is_ safe. – Jabberwocky Mar 11 '17 at 07:39