-
reading from files
i'm trying to read words from a .txt file and then put them in a linked list and then print them out.
here is the text file:
Code:
hello my birthday is in may hello my birthday is in may
i have created a list header file (list.h) which includes the information about the list structure and here is my program:
Code:
#include "list.h"
#include <stdlib.h>
/*Function to create a list recursivly*/
LINK create( char h, Element *t )
{
LINK y = calloc( 1, sizeof( Element ) );
y->d = h;
if( t ) t->next = y; /* Only do this if there is a previous element. */
return y;
}
/* Function to read data from file*/
FILE readdata(void)
{
FILE *ifp; /*infile pointer*/
ifp=fopen("birthday","r"); /*Open file birthday for reading only*/
return *ifp;
}
/* Function to insert data into the list*/
LINK insert(FILE *ifp)
{
LINK head = NULL;
LINK tail = NULL;
char h;
fscanf(ifp,"%s",&h);
while(h!=0){
tail = create( h, tail );
fscanf(ifp,"s",&h);
if( !head ) { head = tail; } /* If head == 0 (ie. no list): */
}
printf("\n");
return head;
}
/*Function to print list*/
void print_list(LINK head)
{
/* This 'for' loop executes while head != 0*/
LINK next;
for( ; head ; head = next ) {
printf( "%s", head->d );
next = head->next;
}
}
int main(void)
{
LINK head = NULL;
FILE *ifp;
readdata();
insert(ifp);
print_list(head);
return 0;
}
my compiler (dev-c++) doesnt give me any warning or errors, however when i try and run the program it crashes, can anyone see where i have gone wrong? i think the problem is with my reading of files and the in file pointer. any help is appreciated.
-
> my compiler (dev-c++) doesnt give me any warning or errors,
Well edit the properties and choose more warning options.
With your code, and these options, I get
Code:
$ gcc -W -Wall -ansi -pedantic -O2 hello.c
hello.c: In function `insert':
hello.c:39: warning: too many arguments for format
hello.c: In function `print_list':
hello.c:52: warning: format argument is not a pointer (arg 2)
hello.c: In function `main':
hello.c:61: warning: `ifp' might be used uninitialized in this function
For a start, all your printf and scanf conversions are broken.
> fscanf(ifp,"%s",&h);
Whilst this conforms to the letter of the law - &h is indeed a char* pointer, the char it points to is only one char, so it is useless for storing a string (which is what %s will try to do)
-
so will i have to store the strings in an array?
-
>so will i have to store the strings in an array?
> char h;
This only stores one character (for example, the h in hello), so yes, you would need an array to store a word.
-
this is my code now
Code:
#include "list.h"
#include <stdlib.h>
/*Function to create a list recursivly*/
LINK create( char h, Element *t )
{
LINK y = calloc( 1, sizeof( Element ) );
y->d = h;
if( t ) t->next = y; /* Only do this if there is a previous element. */
return y;
}
/* Function to read data from file*/
FILE readdata(void)
{
FILE *ifp; /*infile pointer*/
ifp=fopen("birthday","r"); /*Open file birthday for reading only*/
return *ifp;
}
/* Function to insert data into the list*/
LINK insert(FILE *ifp)
{
LINK head = NULL;
LINK tail = NULL;
char h[20];
fscanf(ifp,"%s",&h);
while(h!=0){
tail = create( *h, tail );
fscanf(ifp,"%s",&h);
if( !head ) { head = tail; } /* If head == 0 (ie. no list): */
}
printf("\n");
return head;
}
/*Function to print list*/
void print_list(LINK head)
{
/* This 'for' loop executes while head != 0*/
LINK next;
for( ; head ; head = next ) {
printf( "%s", head->d );
next = head->next;
}
}
int main(void)
{
LINK head = NULL;
FILE *ifp;
readdata();
insert(ifp);
print_list(head);
return 0;
}
regarding the warning about the ifp ,what am i doing wrong?
*ifp is of type FILE and that is what i'm returning....
-
Code:
FILE readdata(void)
{
FILE *ifp; /*infile pointer*/
ifp=fopen("birthday","r"); /*Open file birthday for reading only*/
return *ifp;
}
You don't want to return *ifp...you want to return ifp.
But, the whole ifp variable is useless here. Just write it like this:
Code:
FILE *readdata(void)
{
return fopen("birthday","r");
}
And it's not technically "*ifp is of type FILE". The correct phrasing is "ifp is of type FILE *". It can seem like a meaningless technicality, but realizing the difference can sometimes mean the difference in understanding pointers.
-
Code:
while(h!=0){
tail = create( *h, tail );
fscanf(ifp,"%s",&h);
if( !head ) { head = tail; } /* If head == 0 (ie. no list): */
}
This looks like an endless loop.
-
thanks itsme86, i've narrowed down the crashing of my program down to the INSERT function(as the poster above said it must be going into an endless loop , heres my code:
Code:
LINK insert(FILE *ifp)
{
LINK head = NULL;
LINK tail = NULL;
char h[20];
fscanf(ifp,"%s",&h);
while(h!=0){
tail = create( *h, tail );
fscanf(ifp,"%s",&h);
if( !head ) { head = tail; } /* If head == 0 (ie. no list): */
}
printf("\n");
return head;
}
what am i doing wrong?
-
h can never be 0. It's a pointer to somewhere in memory, but it can never be 0. *h on the other hand can be 0, which means that the value at the memory address that h points to is 0. *h is equivalent to doing h[0]...at least in this circumstance.
-
does this mean that i just have to change it in that part of the code or throughout because when i change
Code:
LINK create( char h, Element *t )
to
Code:
LINK create( char *h, Element *t )
i get an error and it still crashes when i changed insert to this
Code:
/* LINK insert(FILE *ifp)
{
LINK head = NULL;
LINK tail = NULL;
char h[20];
fscanf(ifp,"%s",&h);
while(*h!=0){
tail = create( *h, tail );
fscanf(ifp,"%s",&h);
if( !head ) { head = tail; } /* If head == 0 (ie. no list): */
}
printf("\n");
return head;
} */
thanks for the help itsme86
-
Code:
/* LINK insert(FILE *ifp)
{
LINK head = NULL;
LINK tail = NULL;
char h[20];
fscanf(ifp,"%s",&h);
while(*h!=0){
tail = create( *h, tail );
fscanf(ifp,"%s",&h);
if( !head ) { head = tail; } /* If head == 0 (ie. no list): */
}
printf("\n");
return head;
} */
I'd suggest you go read the pointer tutorial in the FAQ. There's a few things you'll be having problems with here:
Code:
LINK head = NULL;
LINK tail = NULL;
You've never included the header which contains your node in any of your examples. So is LINK a pointer, or an instance? It had better be a pointer. Otherwise it's just plain messy. I suppose you could get it working, but it still wouldn't be as clean as using pointers here.
We'll assume it's a pointer for this discussion.
Code:
char h[20];
fscanf(ifp,"%s",&h);
while(*h!=0){
How about instead of checking your array (incorrectly I might add), you instead check the return value of fscanf to see if it failed or not?
If you insist on checking the array, consider the following instead:
Code:
char h[20];
fscanf(ifp,"%s",&h);
while( h[0] != 0 ){
Well actually, even that doesn't really work well, because your array to start is never initialized. So it may very well have a bunch of garbage values in it, and as such, even if fscanf fails here, it still may pass that loop.
Code:
char h[20] = {0};
fscanf(ifp,"%s",&h);
while( h[0] != 0 ){
But still, it would be better to just check the return value of fscanf.
Code:
tail = create( *h, tail );
fscanf(ifp,"%s",&h);
if( !head ) { head = tail; } /* If head == 0 (ie. no list): */
}
The above is again wrong, because you're trying to dereference an array. You don't dereference arrays. You access their elements. However, in this case, I believe you want to pass the string (ie: the whole contents of said array) to the function, correct?
In that case, you simply use the name of the array by itself. Not accessing an index of it, not trying to dereference it, simply pass the name along:
Code:
tail = create( h, tail );
fscanf(ifp,"%s",&h);
if( !head ) { head = tail; } /* If head == 0 (ie. no list): */
}
Also, you usually don't pass the address of an array around. Again, you simply use the name of the array like it were a pointer to the first element:
Code:
tail = create( h, tail );
fscanf(ifp,"%s",h);
if( !head ) { head = tail; } /* If head == 0 (ie. no list): */
}
And so, were we to rewrite this so it works a bit smoother:
Code:
while( fscanf( ifp, "%s", h ) == 1 )
{
tail = create( h, tail );
if( !head ) { head = tail; }
}
Now I'd again would really suggest you go read the pointer tutorial. You may also find following link helpful. Also, pay close attention to your compiler's warnings. They're there for a reason.
Quzah.
-
thanks quzah the link was helpful. i think my insert function is now working properly , however the arguements that i'm passing to my create function are wrong: here is my code(including the header file):
list.h
Code:
/*List header file */
#include <stdio.h> /* Header file includes the file stdio.h
because that is where NULL is defined*/
typedef char DATA; /* Creates names of types that are more
suggestive of their use*/
typedef struct linked_list {
DATA d;
struct linked_list *next;
} Element;
typedef Element* LINK;
list.c
Code:
#include "list.h"
#include <stdlib.h>
/*Function to create a list recursivly*/
LINK create( char h, Element *t )
{
LINK y = calloc( 1, sizeof( Element ) );
y->d = h;
if( t ) t->next = y; /* Only do this if there is a previous element. */
return y;
}
/* Function to read data from file*/
FILE *readdata(void)
{
return fopen("birthday","r");
}
/* Function to insert data into the list*/
LINK insert(FILE *ifp)
{
LINK head = NULL;
LINK tail = NULL;
char b[30];
fscanf(ifp,"%s",b);
while( fscanf( ifp, "%s", b ) == 1 )
{
tail = create( b, tail );
if( !head ) { head = tail; }
}
printf("\n");
return head;
}
/*Function to print list*/
void print_list(LINK head)
{
/* This 'for' loop executes while head !=0 */
LINK next;
for( ; head ; head = next ) {
printf( "%s", head->d );
next = head->next;
}
}
int main(void)
{
LINK head = NULL;
FILE *ifp;
readdata();
insert(ifp);
print_list(head);
return 0;
}
my compiler is giving me warnings about this stament :
Code:
tail = create( b, tail );
this is because it differs from the parameters i have specified create will take:
Code:
LINK create( char h, Element *t )
would i change it to
Code:
LINK create(char h[], Element *t)
as im now passing an array to it? doing this makes me get more warnings about 'h' as my list creation is now wrong.where am i going wrong?
my compiler is also telling me that ifp has been used without beeing initialised, how do you i initialise ifp to make it point to the file birthday.txt? sorry for asking all these stupid questions.
-
Learn to read, or give up programming would be my advice.
Code:
typedef struct linked_list {
DATA d;
struct linked_list *next;
} Element;
We've been telling you for the last 4 days that you need a char array inside this structure, but you're simply not getting the message.
-
sorry salem , this is the first time i have created my own header files and so i forgot to change my linked list structure.....
is this correct?:
Code:
typedef struct linked_list {
DATA d[30];
struct linked_list *next;
} Element;
as the data in my list structure is now an array of characters how do i modify my create function ?
-
Code:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
typedef char DATA[30]; /* DATA is an array of 30 chars */
typedef struct linked_list {
DATA d;
struct linked_list *next;
} Element;
typedef Element* LINK;
/*Function to create a list recursivly*/
LINK create( DATA h, Element *t )
{
LINK y = calloc( 1, sizeof( Element ) );
strcpy( y->d, h );
if ( t ) t->next = y;
return y;
}
/* Function to read data from file*/
FILE *readdata(void)
{
return fopen("birthday","r");
}
/* Function to insert data into the list*/
LINK insert(FILE *ifp)
{
LINK head = NULL;
LINK tail = NULL;
DATA b;
/* fscanf(ifp,"%s",b); - why are you throwing away the first input ? */
while( fscanf( ifp, "%s", b ) == 1 ) {
tail = create( b, tail );
if( !head ) { head = tail; }
}
printf("\n");
return head;
}
void print_list(LINK head)
{
LINK next;
for( ; head ; head = next ) {
printf( "%s\n", head->d );
next = head->next;
}
}
int main(void)
{
LINK head = NULL;
FILE *ifp;
readdata();
head = insert(stdin); /* use ifp here, and ASSIGN the return result */
print_list(head);
return 0;
}
Look at this, in particular
a) what DATA is typedef'ed as
b) all the places where DATA is used.