fscanf or fgets?

rsfurlan90 · 11-04-2009, 12:11 AM

hi everyone, i'm a beginner in c programming so i'm here to ask you some help
sorry for grammatical errors, i'm from brazil!

i've just made the code below:

Code:

#include <stdio.h>
#include <conio.h>
#include <iostream>

#define cab01 "Relatorio de alunos - Pagina "
#define cab02 "Turma: "
#define cab03 "RA\tNome\t\tMedia\tSituacao"

using namespace std;

main()
{
      int ra, ct_lin, ct_pag;
      float n1, n2, n3, prova, media;
      char *turma, *nome;
      //char str[50];
      const char *situacao;
      
      FILE *cad;
      
      cout << "Sistema de relatorios";
      cout << "\n\n";
      cout << "Insira a turma: ";
      
      gets(turma);
      
      if((cad=fopen(turma,"r"))==NULL)
      {
            cout << "\nO arquivo nao pode ser aberto!";
            getch();
            exit(1);
      }
      
      ct_lin = 1;
      ct_pag = 1;
                 
      cout << "\n\n";
      cout << cab01 << ct_pag << "\n";
      cout << cab02 << turma;
      cout << "\n\n";
      cout << cab03;
      cout << "\n\n";    

      while(!feof(cad))
      {
            /*
            fgets(str, 50, cad);
                        
            nome = strtok(NULL,",");            
            ra = atoi(strtok(str,","));
            n1 = atof(strtok(NULL,","));
            n2 = atof(strtok(NULL,","));
            n3 = atof(strtok(NULL,","));
            prova = atof(strtok(NULL,","));
            */
            
            fscanf(cad, "%d,%[^,]%f,%f,%f,%f", &ra, &nome, &n1, &n2, &n3, &prova);
            
            media = n1 + n2 + n3 + prova;
            
            if (media >= 7)
                  situacao = "Aprovado";
            else
                  if(media >= 4)
                        situacao = "Exame";
                  else
                        situacao = "Reprovado";
                        
            if (ct_lin > 10)
            {
                  ct_lin = 1;
                  ct_pag++;
                  
                  cout << "\n\n";
                  cout << cab01 << ct_pag << "\n";
                  cout << cab02 << turma;
                  cout << "\n\n";
                  cout << cab03;
                  cout << "\n\n";
            }
            
            cout << ra << "\t";
            cout << nome << "\t\t";       
            cout << media << "\t";
            cout << situacao << "\n";
            
            ct_lin++;
      }
      
      fclose(cad);
      getch();
}

the program reads a file that contains numbers and strings, formatted like this:

9999999,Name One,1.0,0.5,1.0,7.0
9999999,Name Two,0.5,0.0,1.0,5.5
... ... ... ...

as you can see, the fields are separated by commas.

i'm having trouble to read the name (second field), i have tried using both fgets (to gets the entire line of the file and token it [i commented in code]) and fscanf (which i preffer) but there are different problems! in first case (fgets and strtok) sometimes i get segmentation fault while i'm debuggin, and when this error doesnt happen, the content of the variable *nome (that stores the name) changes. in example, it starts with "Name One" content, and then, "Name O" and so "A", simply. why this happens?

what should i do to have my program running? if i remove that field from file and from code, everything works..
the best i can get is read a unique character with fscanf (using %c).

thanks in advance!

best regards,
renato furlan

cas · 11-04-2009, 12:38 AM

Two problems directly related to what you're asking (I haven't examined the rest of the code):

When you're using fscanf() to read a string, you simply cannot provide a pointer that doesn't point anywhere (not to mention the fact that you should not use & with strings and the scanf() family of functions). So you'd want something like:

Code:

char name[50];
scanf("%[^,]", name); /* No & here */

This is dangerous, of course, because you can overflow the buffer, but at least you have a fighting chance. I would also expect that you'd want a comma after your %[^,] or else the %f won't read anything (it will see the comma and just stop reading at that point).

As for strtok(), you have to call strtok(string, ","); and then strtok(NULL, ","); you're calling it with NULL first, which won't work.

And I just noticed your gets() call. First off, gets() is a terrible function. That doesn't stop people, though, so my next point will be that you're passing a pointer to nowhere to gets(). C will not automatically allocate memory for you! You have to do it yourself, and for a beginner, an array is easiest:

Code:

char string[50];
gets(string); /* correct, but gets() is still a bad function */
fgets(string, sizeof string, stdin); /* better, though not perfect */

You're mixing C and C++. While that's technically legal in C++ since C is almost a subset of C++, it's bad style. I recommend choosing one or the other.

whiteflags · 11-04-2009, 12:47 AM

Either method like breaking a string apart or just using fscanf should work. Your code has two problems that both must be fixed: first, your using feof() wrongly as this page explains.

Secondly, you're using many functions (fgets, strtok, atoi) that do not handle NULL as an argument. But some of those functions return null: In fgets this is by design so that you know when the file or keyboard input has been completely read. In strtok it means there are no more tokens to return. You create your second problem by nesting function calls and ignoring the possibility that you passed in NULL.

rsfurlan90 · 11-04-2009, 01:14 AM

thank you cas and whiteflags for replying

i have to correct some things in code i just let pass through while posting:

Code:

            /*
            fgets(str, 50, cad);
                        
            nome = strtok(str,","); // i tried this before, the invertion was due some tests      
            ra = atoi(strtok(NULL,","));
            n1 = atof(strtok(NULL,","));
            n2 = atof(strtok(NULL,","));
            n3 = atof(strtok(NULL,","));
            prova = atof(strtok(NULL,","));
            */

and about the correct using for feof() isnt this?

Code:

while (1)
{
            fscanf(.....)
            ...
            if (feof(cad)) break;
}

i will try some more, thanks!

whiteflags · 11-04-2009, 01:29 AM

Code:

while (1)
{
            fscanf(.....)
            ...
            if (feof(cad)) break;
}

No.

You don't really want to do all the "..." if fscanf() put cad into EOF state, such that feof(cad) returns true. The object of any file reading loop is to try reading until it didn't work. When it doesn't work, you have to stop immediately, or you'll just run into a problem. If you read the page I linked, you should know one way to make that happen.

while ( reading(cad) ) do stuff;

There is another way that is a little more convoluted, but I'd rather people figure it out for themselves, if they're so inclined.

And you're still nesting your function calls in an irresponsible way that might segfault.

11-04-2009, 12:38 AM	#2
cas Registered User Join Date: Sep 2007 Posts: 372	Two problems directly related to what you're asking (I haven't examined the rest of the code): When you're using fscanf() to read a string, you simply cannot provide a pointer that doesn't point anywhere (not to mention the fact that you should not use & with strings and the scanf() family of functions). So you'd want something like: Code: char name[50]; scanf("%[^,]", name); /* No & here / This is dangerous, of course, because you can overflow the buffer, but at least you have a fighting chance. I would also expect that you'd want a comma after your %[^,] or else the %f won't read anything (it will see the comma and just stop reading at that point). As for strtok(), you have to call strtok(string, ","); and then strtok(NULL, ","); you're calling it with NULL first, which won't work. And I just noticed your gets() call. First off, gets() is a terrible function. That doesn't stop people, though, so my next point will be that you're passing a pointer to nowhere to gets(). C will not* automatically allocate memory for you! You have to do it yourself, and for a beginner, an array is easiest: Code: char string[50]; gets(string); /* correct, but gets() is still a bad function / fgets(string, sizeof string, stdin); / better, though not perfect */ You're mixing C and C++. While that's technically legal in C++ since C is almost a subset of C++, it's bad style. I recommend choosing one or the other.
cas is offline

11-04-2009, 12:47 AM	#3
whiteflags Registered User Join Date: Apr 2006 Location: United States Posts: 3,201	Either method like breaking a string apart or just using fscanf should work. Your code has two problems that both must be fixed: first, your using feof() wrongly as this page explains. Secondly, you're using many functions (fgets, strtok, atoi) that do not handle NULL as an argument. But some of those functions return null: In fgets this is by design so that you know when the file or keyboard input has been completely read. In strtok it means there are no more tokens to return. You create your second problem by nesting function calls and ignoring the possibility that you passed in NULL. __________________ Os iusti meditabitur sapientiam Et lingua eius loquetur indicium "There is nothing either good or bad, but thinking makes it so." (Shakespeare, Hamlet, Act II scene ii) http://www.myspace.com/whiteflags99 Last edited by whiteflags; 11-04-2009 at 12:53 AM.
whiteflags is offline

11-04-2009, 01:14 AM	#4
rsfurlan90 Registered User Join Date: Nov 2009 Posts: 2	thank you cas and whiteflags for replying i have to correct some things in code i just let pass through while posting: Code: /* fgets(str, 50, cad); nome = strtok(str,","); // i tried this before, the invertion was due some tests ra = atoi(strtok(NULL,",")); n1 = atof(strtok(NULL,",")); n2 = atof(strtok(NULL,",")); n3 = atof(strtok(NULL,",")); prova = atof(strtok(NULL,",")); */ and about the correct using for feof() isnt this? Code: while (1) { fscanf(.....) ... if (feof(cad)) break; } i will try some more, thanks!
rsfurlan90 is offline

11-04-2009, 01:29 AM	#5
whiteflags Registered User Join Date: Apr 2006 Location: United States Posts: 3,201	Code: while (1) { fscanf(.....) ... if (feof(cad)) break; } No. You don't really want to do all the "..." if fscanf() put cad into EOF state, such that feof(cad) returns true. The object of any file reading loop is to try reading until it didn't work. When it doesn't work, you have to stop immediately, or you'll just run into a problem. If you read the page I linked, you should know one way to make that happen. while ( reading(cad) ) do stuff; There is another way that is a little more convoluted, but I'd rather people figure it out for themselves, if they're so inclined. And you're still nesting your function calls in an irresponsible way that might segfault. __________________ Os iusti meditabitur sapientiam Et lingua eius loquetur indicium "There is nothing either good or bad, but thinking makes it so." (Shakespeare, Hamlet, Act II scene ii) http://www.myspace.com/whiteflags99
whiteflags is offline

Thread Tools
Show Printable Version Email this Page
Display Modes
Linear Mode Switch to Hybrid Mode Switch to Threaded Mode

Similar Threads
Thread	Thread Starter	Forum	Replies	Last Post
read mix file: fscanf fgets	cfdprogrammer	C Programming	1	03-20-2009 11:38 AM
Strange behavior if I use fgets() after fscanf()	avi2886	C Programming	5	02-17-2009 04:20 PM
fscanf in different functions for the same file	bchan90	C Programming	5	12-03-2008 09:31 PM
Problem moving data between 'txt' files using: fopen, fscanf & fgets.	Baaaah!	C Programming	12	10-17-2005 06:49 AM
problem with fgets	Smoot	C Programming	4	12-07-2003 03:35 AM