Files, pointers and function (PROBLEM) [C]

[chrisha]

Hi! I want to write a programme that asks the user to enter the name of a .txt file, and then reads it.
So, here's my code:

#include <stdio.h>
#include <string.h>
int open_file(char a[30], int *b) //This function asks for the name of the file and opens it//
{
  gets(a);
  strcat(a, ".txt");
  *b = fopen(a, "r");
}

main()
{
  FILE *archivo;
  char file_name[30], text[30];
  open_file(file_name, &archivo);
    while (!feof(archivo))//This part reads the file//
  {
    fgets(text, 30,  archivo);
    printf("%s", text);
  }
  getch();


}

This works fine. However, when I put the "read" part into a function it doesn't work!!

#include <stdio.h>
#include <string.h>
int open_file(char a[30], int *b)
{
  gets(a);
  strcat(a, ".txt");
  *b = fopen(a, "r");
}

int read_file(char c[30], int *d)
{
  while (!feof(*d))
  {
    fgets(c, 30,  *d);
    printf("%s", c);
  }
  getch();
}

main()
{
  FILE *archivo;
  char file_name [30], text[30];
  open_file(file_name, &archivo);
  read_file(text, &archivo);

}

Where's the problem? Can anybody help me?

Thanks!!

P.S: I use Dev-Cpp in Windows XP

[anduril462]

There are a number of problems I'm seeing:
1. Change the parameter in open_file to FILE **b, since fopen returns a FILE * and you need to pass this value back to main (hence your use of *b instead of b).
2. Don't use gets, since it's highly susceptible to buffer overflows. Use fgets instead.
3. Change the parameter in read_file to FILE *b, and don't dereference it.
4. Don't use feof to control your read loop. See why here: Cprogramming.com FAQ > Why it's bad to use feof() to control a loop.
5. Declare main to return int, then put a return statement at the bottom (probably "return 0").
6. In main, pass archivo to read_file, not &archivo.

Clean all that up and see where you're at. If you're still having problems, post your new code and we'll try again.

EDIT: You should be compiling with all warning and error messages on. All of these except item 4 would have generated some sort of compile-time messages about the bad stuff you're doing.

[chrisha]

Now, this is my code, the compiler doesn't show any error message, but when I execute the programme, it crashes after I put the name of the file!!!

#include <stdio.h>
#include <string.h>
int open_file(FILE**b)
{ char a[30];
  fgets(a,30,stdin);
  strcat(a, ".txt");
  *b = fopen(a, "r");
  if(*b==NULL){printf("error!");}
}

int read_file(char c[30], FILE*b)
{
  while (!feof(b))
  {
    fgets(c, 30,  b);
    printf("%s", c);
  }
  getch();
}

int main()
{
  FILE *archivo;
  char text[30];
  open_file(&archivo);
  read_file(text, archivo);
  return 0;
}

[anduril462]

So first off, you didn't do item 4 from my previous post. I fixed the read_file function to work right in my copy of your program and still encountered an error (seg fault), though there was no output. Read the link I gave you, and fix that loop please. Also, if you declare open_file and read_file to return int, you better return an int at the end of them. This would be good in the case of open_file at least, so you can return a -1 if there's an error, and completely skip trying read_file since the file was never opened.

Your program tried to tell you there was a problem, but you couldn't see the "error!" output. The cause of the error stems from a subtlety of fgets that you missed, namely that it appends the newline from your input (assuming there's room). Read the documentation on fgets to get a better understanding of exactly what it's doing. In short, if I entered "foo", your program would create the filename "foo\n.txt", with the newline in the middle. Clearly, this file didn't exist, so fopen failed, returning NULL. You correctly printed your error message, but it never showed up.

As for why you didn't see any output, it has to do with the fact that your printf("error!") statements don't contain a new line. printf uses the standard output device, which is a buffered terminal. Buffered means that it wont display output until it gets a new line. So your program actually printed error, but you never saw it on your screen. Then you hit a seg fault, which killed your program early and thus the output never showed.

This is the classic case of why debug output from your program should go to stderr. stderr is unbuffered output to your terminal, which means the output shows up immediately when you print it. While a newline would technically fix the problem of your error output showing up, you might not want a newline after every debug printf in your program (imagine printing a newline after each element in a 10x10 array). You should fix this by using fprintf(stderr, "error!") instead of your printf statements. I also recommend you include the error number and corresponding string for standard C library errors, like so
[code]
*b = fopen(a, "r");
if (NULL == *b) {
// this would show you the file it tried to open, which would have allowed you to notice the \n in the file name
fprintf(stderr, "Error opening file '%s', errno: %d: %s\n", a, errno, strerror(errno));
}

As a side note, you allow the user to enter up to 30 chars (the size of a), but then go on appending 4 more chars after that (".txt"), creating a potential buffer overflow. I recommend you let them type the .txt part of the filename too, cap the limit at 30. And of course, trim the trailing '\n' if one exists.

[chrisha]

This is the code now, and it works OK. The problem was the problem was the '\n' at the end of the file name, so I changed fgets(a,30,stdin) and put gets(a).

#include <stdio.h>
#include <string.h>
int open_file(FILE **b)
{
  char a[24];
  gets(a);
  while (strlen(a) > 20)
  {
    printf("The name of the file must have less than 21 characters\n");
    gets(a);
  }

  strcat(a, ".txt");
  *b = fopen(a, "r");
  if (*b == NULL)
  {
    fprintf(stderr, "Error opening file '%s'", a);
    return  - 1;
  }
}

int read_file(char c[30], FILE *b)
{
  while (!feof(b))
  {
    fgets(c, 30, b);
    printf("%s", c);
  }
  getch();
}

int main()
{
  FILE *archivo;
  char text[30];
  printf("Name of the file (less than 21 characters): ");
  open_file(&archivo);
  read_file(text, archivo);
  return 0;
}

Thank you a lot, anduril462!

[anduril462]

You're welcome, but I must inform you that, since you chose not to follow my advice regarding feof as a read loop control, your program does not work. I copied and pasted your code verbatim, and did not get the desired results. Now, it did open the file, read it and print it out, but it did not print an exact copy of the file as one would expect. It prints the last line twice. Here is the file I used (code tags to discriminate file contents):

test.txt:

this
is
a
test

Here is the output I got:
[charlesg@fedora test]$ ./a.out
Name of the file (less than 21 characters): test
this
is
a
test
test

Notice that it printed the last line, "test", twice! This is exactly the problem with using feof to control the loop that I was telling you about. FIX IT!!!!!!! Read the link, take a stab at fixing your code, and if you really can't figure it out, come back for some more help.

Also, for the love of all things good and holy, don't use gets. Use fgets(...stdin). Suggesting to the user to keep it under 21 characters is not a very good way to ensure valid input for at least the following two reasons:
1. Users are dumb. They probably wont count the characters of their file name or listen to your suggestion.
2. Users are malicious. People can exploit your usage of gets to do very bad stuff to your system.

Using gets creates a potential buffer overflow that could result in anything from a crashed program to somebody hacking your system and destroying stuff. Break the habit now so such code never makes it out into the "real world". Your compiler should have warned you about this is you turned on all the warnings like I suggested.