Thread: Help Please - program to display file renders Windows Program Error

  1. #16
    Drunken Progammer CaptainMorgan's Avatar
    Join Date
    Feb 2006
    Location
    On The Rocks
    Posts
    45
    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.
    Last edited by CaptainMorgan; 11-13-2006 at 12:10 PM.

  2. #17
    {Jaxom,Imriel,Liam}'s Dad Kennedy's Avatar
    Join Date
    Aug 2006
    Location
    Alabama
    Posts
    1,065
    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?

  3. #18
    Drunken Progammer CaptainMorgan's Avatar
    Join Date
    Feb 2006
    Location
    On The Rocks
    Posts
    45
    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

  4. #19
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,660
    > 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
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Beginner Needs help in Dev-C++
    By Korrupt Lawz in forum C++ Programming
    Replies: 20
    Last Post: 09-28-2010, 01:17 AM
  2. Getting an error with OpenGL: collect2: ld returned 1 exit status
    By Lorgon Jortle in forum C++ Programming
    Replies: 6
    Last Post: 05-08-2009, 08:18 PM
  3. Testing some code, lots of errors...
    By Sparrowhawk in forum C Programming
    Replies: 48
    Last Post: 12-15-2008, 04:09 AM
  4. Script errors - bool unrecognized and struct issues
    By ulillillia in forum Windows Programming
    Replies: 10
    Last Post: 12-18-2006, 04:44 AM
  5. using c++ in c code
    By hannibar in forum C Programming
    Replies: 17
    Last Post: 10-28-2005, 09:09 PM