Help Please - program to display file renders Windows Program Error

Hello,

I've been practicing writing small programs, but now I'm finding that whenever I try to run it, it runs, but half-way into the program Windows displays the (myprogram has encountered a problem and has to close).

Here's my code for reference:

Code:

---

#include <stdio.h>
#include <string.h>

int main()
{
    char *filename;
    printf("Please enter the filename");
    scanf("%c", &filename);
    fopen(filename,"r");
    char filetext[1024];
    int i = 0;
    for(i = 0; i <= 1024; i++) {
          filetext[i] = fgetc(filename);
          tolower(filetext[i]);
          }
    printf("%s",filetext[i]);
    if(i = " ") {
        printf("\n");
        }
        }

---

I know there are at least two problems here, but the compiler reports them as 'warnings' as opposed to errors. One is surely that fgetc can't pass the character to a point within an array, but I can't find a function that can do that, unless I use fgets, but then I'm not sure how to move from one line to another in the file!

Also, I think my tolower may be faulty. I can't find that function in the C reference guide, but then again I can't find a function to convert characters to lowercase in the printf operator list either.

The file being referenced is simply a notepad text file with the text "This is an example file." (without the "'s).

If anyone can lend a hand with this I'd be grateful.

Thanks.

Hussein.

You havent allocated any memory for char *filename.
So char* filename points to a random memory address that you haven't allocated. Windows doesn't allow writing data to random memory addresses.

Wow, so many problems...

> scanf("%c", &filename);
1. probably meant to use %s
2. filename isn't allocated any space

> fopen(filename,"r");
You didn't save the return result for use in other f.. functions.

> char filetext[1024];
C doesn't support mixed declarations and statements (unless you have a C99 compiler)

> for(i = 0; i <= 1024; i++)
Steps off the end of the array

> printf("%s",filetext[i]);
1. An index is not a string, you probably meant printf("%s",filetext);
2. filetext doesn't have a \0 anyway (it needs one)

> if(i = " ")
I've no idea what you wanted to do here!
But I'd wager that == would be involved.

Echoing what Salem said... and this may be of some help:
http://www.cprogramming.com/tutorial/cfileio.html

Code:

---

if(i = " ")

---

Funny, this sets i to 32 and always returns true.

Btw, what is it that you're trying to accomplish exactly, besides the resolution of compilation errors.

I've got a working copy of it producing:

This
is
an
example
file.


Correct? Or is there something else? Also, it appears your for loop is unnecessary.

Yes, that is correct.

Grrr, I had a nice reply typed up here before the computer crashed.

Anyway, thanks for all your comments and suggestions. I'm also really sorry I forgot to include comments, now you should know what I'm trying to do at each stage.

Code:

---

#include <stdio.h>
#include <string.h>

int main()
{
    char filename[20];
    // Allocate a string to store filename - does PC interpret whitespace at end of filename?
    printf("Please enter the filename");
    scanf("%s", &filename);
    // Request name of file to open, store in filename
    FILE *fp;
    fp = fopen(filename,"r");
    // Create file pointer, open file stored in string filename
    char filetext[1024];
    // Create string 1024 cells long to hold text from file
    int i = 0;
    // Compiler doesn't like my fore loops, explicitly create i
    for(i = 0; i <= 1023; i++) {
          filetext[i] = fgetc(filename);
          // Place each character of the file into a different string cell, one after the other
          tolower(filetext[i]);
          // Move through string, converting text to lowercase
          }
    printf("%s",filetext);
    // Display the contents of the file on screen, but...
    if(i == ' ') {
    // If the string cell is blank, i.e a space
        printf("\n");
        // move onto a new line
        }
    }

---

OK. For the filename[20], I suppose I could get the length of the entered filename from the user, and then store that as a string. But my question is, does the PC interpret whitespace after the filename extension or is it ignored? I.e does the string HAVE to be EXACTLY the same length as the entered filename?

I'm not sure what you mean about the mixed type definitions for char filetext[1024]. I'm creating an array of chars to hold each char of the file.

For the printf statement, I think I'm beginning to see that you can use just the name of the array to refer to its entire contents, instead of having to include the []'s.

The i == ' ' is supposed to test for spaces at which point a new line is printed (so each word is on a new line. I thought C would map the ' ' to the ASCII or numerical value of the space?

Why would I not need the for loop? It's the only loop I'm aware of to iterate through the array, printing each character as it goes and dropping to a new line when it finds a space.

I'm still getting errors at the filetext[i] = fgetc(filename) line. I know the C guide shows that there is no arguement for storing the character into a variable or array, but then I'm not sure how to do it short of coupling to two things to gether, i.e assigning each cell of the array to the character got from the file at that point.

OK, I'm confused about the fopen part. I know most C functions return either '0' or '1' to report a success or failure. This having been said, why is it necessary to store that anywhere? It would only be used to explicitly check if the file were opened, and for such a simple program is it necessary? The file opens, so why could I not just leave it at that? Confused about this.

Thanks for the Tutorial link. I didn't realise this site had them, should have suspected though. However, the linking of the use of the functions and standard I/O functions (storing results into arrays, etc) is still baffling me. I mean, one must have to link say, fgetc, to somewhere until it's used.

Thanks again for everything. Sorry, I realise most of this must seem really simple to a lot of you, but I appreciate the help.

Hussein.

I still count about 6 mistakes...

Code:

---

int main()
{
    char filename[20];
    // Allocate a string to store filename - does PC interpret whitespace at end of filename?
    printf("Please enter the filename");
    scanf("%s", &filename);
    // Request name of file to open, store in filename
    FILE *fp;
    fp = fopen(filename,"r");
    // Create file pointer, open file stored in string filename
    char filetext[1024];
    // Create string 1024 cells long to hold text from file
    int i = 0;
    // Compiler doesn't like my fore loops, explicitly create i
    for(i = 0; i <= 1023; i++) {
          filetext[i] = fgetc(filename);
          // Place each character of the file into a different string cell, one after the other
          tolower(filetext[i]);
          // Move through string, converting text to lowercase
          }
    printf("%s",filetext);
    // Display the contents of the file on screen, but...
    if(i == ' ') {
    // If the string cell is blank, i.e a space
        printf("\n");
        // move onto a new line
        }
    }

---

Now, let's see what you are doing here:

Code:

---

    scanf("%s", &filename);

---

You are passing scanf the address to the pointer of the filename buffer.

Code:

---

fp = fopen(filename,"r");

---

Doh!

Code:

---

    for(i = 0; i <= 1023; i++) {

---

filetext[1023] is the last byte of the filetext buffer, so if you don't set the last byte to 0, then you will see a buffer overrun(overread?).

Code:

---

          tolower(filetext[i]);

---

tolower() returns lowercase filetext[i], but... the return value is not used.

Code:

---

    if(i == ' ') {

---

If i is 32, it enters that if. (It is always 1024 after the loop)

Your indentation is buggy (last 2 lines).

here are some of them

1.

Code:

---


scanf("%s", &filename);

---

lose the &

2.

Code:

---

fp = fopen(filename,"r");

---

YOu better go throught fopen in the man pagesagain.


3.

Code:

---

  for(i = 0; i <= 1023; i++) {

---

Check to the end of the file instead,somehting like
int c;
while((c=fgetc(fp))!=EOF)


4.

Code:

---

if(i == ' ') {
    // If the string cell is blank, i.e a space
        printf("\n");
        // move onto a new line
        }

---

what exactly are you trying to do here?
anyway, use '\n' in your program, its horrible to read otherwise.


Err...
EDIT : sorry max , didnt see your post

Quote:

---

Originally Posted by hpteenagewizkid

OK. For the filename[20], I suppose I could get the length of the entered filename from the user, and then store that as a string. But my question is, does the PC interpret whitespace after the filename extension or is it ignored? I.e does the string HAVE to be EXACTLY the same length as the entered filename?

---

Your best bet is to do it dynamically.

Quote:

---

Why would I not need the for loop? It's the only loop I'm aware of to iterate through the array, printing each character as it goes and dropping to a new line when it finds a space.

---

kris.c gives an example of why and is a better solution.

Quote:

---

Thanks for the Tutorial link. I didn't realise this site had them, should have suspected though. However, the linking of the use of the functions and standard I/O functions (storing results into arrays, etc) is still baffling me. I mean, one must have to link say, fgetc, to somewhere until it's used.

---

To memory! :)



Seeing as you've been a good sport and probably figured it out by now with so much help from those pitching in, here's a working example for the heck of it:

Code:

---

#include <stdio.h>

int main( int argc, char* argv[] )
{
  // reserve space for the string to be input
  char *filename = malloc( sizeof( char ) * 10 );
  char c;  // individual characters found

  printf( "Enter filename: " );
  scanf ( "%s", filename );

  FILE *fp;  // pointer to FILE type(declared in stdio.h)
  fp = fopen( filename, "r" ); // open the file and read

  // perform error checking if file is not found
  if ( !fp ) {
    printf( "Error reading file.\n" ); 
    return 0;
  }

  // aesthetics
  printf( "\nFile name: %s\n", filename );
  printf( "Output:\n" );
  putchar( '\n' ); 

  while ( fscanf( fp, "%c", &c ) != EOF ) {
    c = tolower( c );  // change every uppercase to lowercase
    printf( "%c", c ); // simply output the character
    if ( c == ' ' ) {  // if character encountered is a space
      putchar( '\n' ); //  then output a newline character
    }
  }

  fclose( fp );  // close the file when finished
  return 0;      // normal program termination
}

---

Mmm, still 4 problems in CaptainMorgan's answer...

Quote:

---

Originally Posted by kris.c

1.

Code:

---


scanf("%s", &filename);

---

lose the &

---

Eh? Why?

I already told you. The second parameter of scanf is char*, not char**.
Filename is a pointer to the buffer, &filename is a pointer to the pointer of the buffer.

Gosh, this is fun isn't it.
I haven't seen so much flounder since I was last down at the fish mongers.

Quote:

---

Originally Posted by Salem

Mmm, still 4 problems in CaptainMorgan's answer...

---

Care to elaborate? Because with what the OP wanted, and run from a Linux environment with the gcc compiler there are no warnings or errors and the program runs as requested producing the correct output. If you're referring to optimization then of course it can be made better. Interested in hearing where the 'problems' exist. Thanks!

-Capt

If you don't want to post the elaboration, feel free to message me.

EDIT: one optimization might be instead of c = tolower( c ); simply do printf( "%c", tolower( c ) ); But didn't want to confuse a beginner.

Code:

---

#include <stdio.h>

int main( int argc, char* argv[] )
{
  // reserve space for the string to be input
  char *filename = malloc( sizeof( char ) * 10 );
  char c;  // individual characters found

  printf( "Enter filename: " );
  scanf ( "%s", filename );

  FILE *fp;  // pointer to FILE type(declared in stdio.h)
  fp = fopen( filename, "r" ); // open the file and read

  // perform error checking if file is not found
  if ( !fp ) {
    printf( "Error reading file.\n" ); 
    return 0;
  }

  // aesthetics
  printf( "\nFile name: %s\n", filename );
  printf( "Output:\n" );
  putchar( '\n' ); 

  while ( fscanf( fp, "%c", &c ) != EOF ) {
    c = tolower( c );  // change every uppercase to lowercase
    printf( "%c", c ); // simply output the character
    if ( c == ' ' ) {  // if character encountered is a space
      putchar( '\n' ); //  then output a newline character
    }
  }

  fclose( fp );  // close the file when finished
  return 0;      // normal program termination
}

---

You allocated 10 chars for filename, then used scanf() without bounds. . . on this board this is considered "bad".

You are using the C99 mixed declarations. . . not wrong, but not common.

fscanf() returns the number of successfull items read, not NULL. Should use something like fgetc().

malloc is *usually* defined in stdlib.h, which is not included.

Also, some on this board will tell you that sizeof(char) is redundant.

Salem, did I miss anything?

Kennedy, all of which I consider reasonable but also more to the effect of optimization issues and styling rather than 'problems'. Primarily things such as 'considered', 'not wrong, but not common', 'should use', and 'usually' are clearly not in the same class as 'problems' which would cause a program to produce errors at compile time, simply not work as expected, or cause seg faults and memory leaks.

Once again an optimization might have been

Code:

---

while ( ( c = fgetc( fp ) ) != EOF ){ ... }

---

which I agree with as this program was a whip-up but I don't see the fscanf usage as a 'problem'. Of course, I'd like to hear your opinion/response.

-Capt

> char *filename = malloc( sizeof( char ) * 10 );
1. 10 is woefully inadequate for a filename. It isn't even enough for DOS 8.3 nevermind anything else.
2. You don't check for NULL
3. Why bother with such a small alloc when an array would suffice.
4. You don't include stdlib.h, so malloc is incorrectly prototyped.
5. You didn't call free() at the end.

> FILE *fp;
I guess you're compiling in C99 mode, because standard C doesn't have mixed declarations and statements, nor does it have // comments.

> fscanf( fp, "%c", &c ) != EOF
You're probably OK reading single chars (though fgetc would have been much better), but you should check for the number of conversions.
Eg.
while ( fscanf( fp, "%c", &c ) == 1 )

Had you something like this
while ( fscanf( fp, "%d", &myint ) != EOF )
Then it would be possible for the while loop to lock up completely if input wasn't an int. You wouldn't convert anything, and you wouldn't advance through the file either.

> c = tolower( c );
This is prototyped by ctype.h

> there are no warnings or errors
Try it with
gcc -W -Wall -ansi -pedantic -O2 prog.c