Hi all,
Just registered on the forum. I have a quick (hopefully) question. I'm trying to read from a text file into one single string. I'm having some trouble though and can't quite find my error. I have the following code below right now. The problem is that my function adds junk to the beginning and end of the string. This function must be written in C. My text file i'm testing with is a one liner:

The brown fox jumped over the lazy dog.

void readFile(char filename[20], char *string)
{
	FILE *fp;	
	fp = fopen(filename, "r");  /* open file for input */

  	if (fp)
  	{
    		char c;
    		while(c != EOF) {
      			c = fgetc(fp);
      			//printf("%c", c);
      			sprintf(string, "%s%c", string, c); //apend char to our string
    		}
  	}
  	else          /* If error occurred, display message. */
  	{
    		printf("An error occurred while opening the file.\n");
  	}
  	fclose(fp);  /* close the input file */
}

however this function produces output like the following

cThe brown fox jumped over the lazy dog.


I will also note that if I uncomment the line //printf("%c", c); I get the following output:
The brown fox jumped over the lazy dog.

If anyone could help I would greatly appreciate it!

Matt

Recommended Answers

All 8 Replies

1) c must be of type int, not char, because EOF won't fit in a char.

2) Open the file in binary mode and read it one character at a time. The function readFile() is very dangerous because you have no clue how much memory was allocated for the string!

int spot = 0;
int c;
while( (c = fgets(fp) ) != EOF)
{
    string[spot++] = c;
}
string[spot] = 0; // null-terminate the string
int spot = 0;
int c;
while( (c = fgets(fp) ) != EOF)
{
    string[spot++] = c;
}
string[spot] = 0; // null-terminate the string

He really meant while((c = fgetc(fp) != EOF) And some further information. You really need to pass it a limit of how much it can read into the string, if not there's the possibility of over flowing.
The file closing needs to be inside the if() right after the string[spot] = 0; // null-terminate the string That's where it makes sense, since as it stands it will execute even if it could not open it.

He really meant while((c = fgetc(fp) != EOF)

Nope. You have mis-matched parentheses. Count them.

Nope. You have mis-matched parentheses. Count them.

Yes, I erased a parentheses when fixing the color manually.
It should be: while((c = fgetc(fp)) != EOF) >1) c must be of type int, not char, because EOF won't fit in a char.

It will fit a char, but there's not guarantee that it would be a signed char that can hold negatives. Furthermore, that's what fgetc() returns, an int, therefore, that's what it should be.

commented: you are right :) +36

Yes, I erased a parentheses when fixing the color manually.
It should be: while((c = fgetc(fp)) != EOF)

Ohhh I see my mistake now -- wasn't the parantheses, but the fgets() function instead of fgetc(). Thank you for the correction. :)

Ohhh I see my mistake now -- wasn't the parantheses, but the fgets() function instead of fgetc(). Thank you for the correction. :)

Just a tiny typo but I felt to bring it up to the attention of the OP. :)

The function readFile() is very dangerous because you have no clue how much memory was allocated for the string!

int getFileSize(char filename[20])
{
    FILE *fp = fopen(filename, "r");
    int c, sz = 0;
    while ((c = getc(fp)) != EOF) ++sz;
    
    return sz;
}

what do you think of doing this before I declare the array to pass into readFile()? Is this an acceptable way to get the number of characters in a file? And terminating it with an '/0'?

While that could work as an exercise for logic, there's already a couple of standard I/O functions that will help you to know the size of a file in a generic way, in the absence of more efficient API.

long aia_gfs(FILE *file_handle)
{
	/* file pointers */
	long current_position, end_position;

	if (file_handle != NULL) {
		/* obtain current file pointer position */
		current_position = ftell(file_handle);

		/* 
		 * seek to the end of file 
		 * SEEK_SET is start of file
		 * SEEK_END is end of file
		 */
		fseek(file_handle, SEEK_SET, SEEK_END);

		/* obtain end file pointer position */
		end_position = ftell(file_handle);

		/* restore file pointer to beginning of file */
		fseek(file_handle, current_position, SEEK_SET);

		/* divulge size of file */
		return end_position;
	} else {
		fprintf(stderr, "Couldn't read file");
		return -1;
	}
}
Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.