Jeffrey Jordan
Jeffrey Jordan

Reputation: 145

Warnings involving reading file into char array in C

I have code that compiles and prints data from a file, but it's stuffed to the gills with warnings, and I'm unable to fix them.

void batchMode(char **c) {

     char *batchBuffer = NULL;
     size_t batchSize = 0;

     FILE *fp = fopen(c, "r");

     fseek(fp, 0, SEEK_END);
     batchSize = ftell(fp);

     rewind(fp);

     batchBuffer = malloc((batchSize + 1) * sizeof(*batchBuffer));

     fread(batchBuffer, batchsize, 1, fp);

     batchBuffer[batchSize] = 0;

     printf("%s\n", batchBuffer);

}

int main(int argc, char **argv){

    if (argc == 2)
          batchMode(&argv[1][0]);

     return 0;

}

Warnings include: passing argument 1 of batchmode from incompatible pointer type

batchMode(&argv[1][0]);

expected 'char **' but argument is of type char *

void batchMode(char **c)

passing argument 1 of fopen from incompatible pointer type

FILE *fp = fopen(c, "r");

expected 'const char * restrict' but argument is of type char **

FILE *fopen (const char *_restrict_filename

Upvotes: 0

Views: 111

Answers (2)

user3629249
user3629249

Reputation: 16540

the following proposed changes to the OPs code:

  1. cleanly compiles
  2. performs the desired functionality
  3. properly checks for errors
  4. properly free's the allocated memory when done with it

Note: function: 'perror()' passes the error message and the text reason the system thinks the last failure occurred to 'stderr'

And now, the proposed changes, with comments

#include <stdio.h>     //<-- missing
#include <stdlib.h>    //<-- missing


//void batchMode(char **c) {        //<-- needs pointer to a string, not a single character
         //<-- suggest using meaningful name, like: 'filename'
void batchMode( char *fileName ) {

     //char *batchBuffer = NULL;    //<-- limit scope and keep with 'setter'
     //size_t batchSize = 0;       //<-- ftell() returns a 'long', not a unsigned long
     //long batchSize = 0;         //<-- limit scope and keep with 'setter'

     FILE *fp = fopen( fileName, "r");        //<-- check returned value for success (!=NULL)
     if( ! fp )
     {
         perror( "fopen failed" );
         exit( EXIT_FAILURE );
     }

     //fseek(fp, 0, SEEK_END);         //<-- returns an 'int', check for success (!=-1)
      long batchSize;
     if( (batchSize = fseek( fp, 0, SEEK_END )) == -1 )
     {
         perror( "fseek for end of file failed" );
         exit( EXIT_FAILURE );
     }

     batchSize = ftell(fp);               //<-- check the returned value for an error indication (-1)
                                     //<-- returns a 'long', not a 'unsigned long'
     if( batchSize == -1 )
     {
         perror( "ftell failed" );
         exit( EXIT_FAILURE );
     }

     rewind(fp);                   //<-- does not have error checking, suggest: using 'fseek(fp, 0, SEEK_SET )'

    // batchBuffer = malloc((batchSize + 1) * sizeof(*batchBuffer));  //<-- 'sizeof(*batchBuffer)' is the size of a char pointer 4 or 8 depending on the underlying hardware architecture and certain compile options
                                         //<-- check for success, returned value (!=NULL)
     char * batchBuffer = malloc( (size_t)batchSize+1);
     if( ! batchBuffer )
     {
         perror( "malloc failed" );
         exit( EXIT_FAILURE );
     }

     //fread(batchBuffer, batchsize, 1, fp);   // incorrect capitalization of batchSize
     if( fread(batchBuffer, (size_t)batchSize, 1, fp) != 1 )    //<-- if returned value != third parameter, then error occurred
     {
         perror( "fread failed" );
         exit( EXIT_FAILURE );
     }

     batchBuffer[batchSize] = 0;

     printf("%s\n", batchBuffer);

     free( batchBuffer );      //<-- to avoid memory leak
}


int main(int argc, char **argv){

     //if (argc == 2)               //<--  handle error first
     if( argc != 2 )
     {
         fprintf( stderr, "USAGE: %s <fileName>\n", argv[0] );
         exit( EXIT_FAILURE );
     }

     //batchMode(&argv[1][0]);   //<-- send pointer to first command line parameter, not a single character
     batchMode( argv[1] );

     return 0;
}

Upvotes: 0

Stephan Schlecht
Stephan Schlecht

Reputation: 27106

  • You may want to change the method signature of batchMode to use a character pointer instead of a pointer to a pointer.
  • Accordingly, you should adapt the function call to batchMode(argv[1]); to pass the first program argument as a parameter
  • the second parameter of fread must be batchSize instead of batchsize (note the capital S)
  • since batchBuffer is dynamically allocated, you should add a free (batchBuffer); after it is no longer needed

So your slightly modified code could look like this:

#include <stdio.h>
#include <stdlib.h>


void batchMode(const char *c) {
    char *batchBuffer = NULL;
    size_t batchSize = 0;
    FILE *fp = fopen(c, "r");
    fseek(fp, 0, SEEK_END);
    batchSize = ftell(fp);
    rewind(fp);

    batchBuffer = malloc((batchSize + 1) * sizeof(*batchBuffer));

    fread(batchBuffer, batchSize, 1, fp);

    batchBuffer[batchSize] = 0;

    printf("%s\n", batchBuffer);

    free(batchBuffer);
}

int main(int argc, char **argv) {
    if (argc == 2)
        batchMode(argv[1]);

    return 0;
}

Upvotes: 1

Related Questions