| | |
file_read block
Please support our C advertiser: Programming Forums - DaniWeb Sister Site
Hi All,
this is my first C code in my professional life. I wanted to write a common function to retrieve file content. I do not know whether below code is upto the mark or not.
any Suggestions/modifications/tips are greatly appreciated.
Regards,
Vashek
this is my first C code in my professional life. I wanted to write a common function to retrieve file content. I do not know whether below code is upto the mark or not.
any Suggestions/modifications/tips are greatly appreciated.
C Syntax (Toggle Plain Text)
#include <stdio.h> #include <stdlib.h> /* Function Name: get_file_content Description: fills content of the file to given buffer. Parameters: file to be read(in), Mode(in), destination buffer(out) Return Type: pointer to content (SUCCESS), otherwise NULL */ char* get_file_content (char *file_name, char *file_mode, char *file_content) { FILE *fp; long file_size; size_t bytes_read; //open file if (!(fp = fopen (file_name, file_mode))) { printf ("*** Unable to open file ***\n"); return NULL; } //get file size fseek (fp, 0, SEEK_END); file_size = ftell (fp); rewind (fp); printf("*** size = %ld ***\n", file_size); //check memory file_content = (char *) realloc (file_content, sizeof(char) * file_size); if (file_content == NULL) { printf("*** memory insufficient ***"); fclose (fp); return NULL; } //get content bytes_read = fread (file_content, sizeof(char), file_size, fp); if (bytes_read != file_size) { printf("*** read failed ***"); fclose (fp); return NULL; } //put EOS file_content[bytes_read-1] = '\0'; //clean fclose (fp); //success return file_content; } int main (int argC, char *argV[]) { int status = -1; char *file_content = (char *) malloc (2); if ((file_content = get_file_content ("metadata.txt", "r", file_content)) == NULL){ status = 1; goto FREE_BLOCK; } printf("\n#%s#\n", file_content); FREE_BLOCK: free (file_content); return 0; }
Regards,
Vashek
Last edited by vashek; Sep 10th, 2009 at 10:27 am.
#include <stdio.h> #include <stdlib.h> /* Function Name: get_file_content Description: fills content of the file to given buffer. Parameters: file to be read(in), Mode(in), destination buffer(out) Return Type: pointer to content (SUCCESS), otherwise NULL */ char* get_file_content (char *file_name, char *file_mode, char *file_content) { FILE *fp; long file_size; size_t bytes_read; //open file if (!(fp = fopen (file_name, file_mode))) { printf ("*** Unable to open file ***\n"); return NULL; } //get file size fseek (fp, 0, SEEK_END); file_size = ftell (fp); rewind (fp); printf("*** size = %ld ***\n", file_size); //check memory file_content = (char *) realloc (file_content, sizeof(char) * file_size); if (file_content == NULL) { printf("*** memory insufficient ***"); fclose (fp); return NULL; } //get content bytes_read = fread (file_content, sizeof(char), file_size, fp); if (bytes_read != file_size) { printf("*** read failed ***"); fclose (fp); return NULL; } //put EOS file_content[bytes_read-1] = '\0'; //clean fclose (fp); //success return file_content; } int main (int argC, char *argV[]) { int status = -1; char *file_content = (char *) malloc (2); if ((file_content = get_file_content ("metadata.txt", "r", file_content)) == NULL){ status = 1; goto FREE_BLOCK; } printf("\n#%s#\n", file_content); FREE_BLOCK: free (file_content); return 0; }
1
•
•
•
•
> file_content = (char *) realloc (file_content, sizeof(char) * file_size);
Is this C or C++?
If it is C, then you don't need the cast.
However, what happens if realloc fails? The problem is, you just trashed your pointer with NULL, thus making it impossible to free a memory leak (if that was your only pointer).
Also, if realloc MOVES the memory block, you've only updated your local copy of the pointer (the one in main still points to the original (and now freed) block).
The whole 3rd parameter is a waste of effort. You may as well have a local pointer, initialise it with malloc and return it.
> file_content[bytes_read-1] = '\0';
What if filesize was in fact zero?
Here you're off into the weeds somewhere, getting bogged down by a segfault no doubt.
Even if the file length is non-zero, you trash the last char of the file.
Also, what if someone tries to load in say a DVD image file - will you handle that well?
What if they call the function with a mode of "w" ?
Is this C or C++?
If it is C, then you don't need the cast.
However, what happens if realloc fails? The problem is, you just trashed your pointer with NULL, thus making it impossible to free a memory leak (if that was your only pointer).
Also, if realloc MOVES the memory block, you've only updated your local copy of the pointer (the one in main still points to the original (and now freed) block).
The whole 3rd parameter is a waste of effort. You may as well have a local pointer, initialise it with malloc and return it.
> file_content[bytes_read-1] = '\0';
What if filesize was in fact zero?
Here you're off into the weeds somewhere, getting bogged down by a segfault no doubt.
Even if the file length is non-zero, you trash the last char of the file.
Also, what if someone tries to load in say a DVD image file - will you handle that well?
What if they call the function with a mode of "w" ?
0
•
•
•
•
I am assuming you are reading an ASCII file. Do not overwrite the last byte in the file. Get file length. Allocated buffer of size+1. Read file of size into the buffer. Set terminator (that extra byte you appended) buffer[size]=0;
It'll handle the empty file scenario! But you really should have a length check for zero, and not read the zero bytes!
Or handle an empty file as a special kind of error!
I personally prefer returning bool and passing the address of the pointer as an argument along with an address of a length count just so I can have both whether I chose to ignore the length count or not! Then it doesn't matter if the file is ASCII or binary, you can use the same loader for all file loads into memory!
It'll handle the empty file scenario! But you really should have a length check for zero, and not read the zero bytes!
Or handle an empty file as a special kind of error!
I personally prefer returning bool and passing the address of the pointer as an argument along with an address of a length count just so I can have both whether I chose to ignore the length count or not! Then it doesn't matter if the file is ASCII or binary, you can use the same loader for all file loads into memory!
Last edited by wildgoose; Sep 10th, 2009 at 2:13 pm.
Similar Threads
- Please Help Me block porn!!!!!! (Windows NT / 2000 / XP)
- Block Web Site (Windows NT / 2000 / XP)
- Using a robots.txt to block links (Search Engine Optimization)
- Short Cut to Block Senders List (Web Browsers)
- Block IPs (IT Professionals' Lounge)
| Thread Tools | Search this Thread |
adobe api array arrays binarysearch calculate char cm convert copyanyfile copypdffile cprogramme createcopyoffile createprocess() csyntax directory dynamic feet fflush file floatingpointvalidation fork forloop frequency getlasterror givemetehcodez global graphics gtkgcurlcompiling hacking hardware highest homework i/o inches incrementoperators initialization iso kernel kilometer km linked linkedlist linux linuxsegmentationfault list lists locate logical_drives loopinsideloop. match matrix microsoft motherboard mqqueue mysql odf open opensource openwebfoundation owf pattern pdf performance pointer pointers posix power probleminc program programming pyramidusingturboccodes read recursion recv recvblocked repetition research scanf scheduling scripting segmentationfault send shape socketprograming socketprogramming stack standard string suggestions systemcall test testautomation unix urboc user voidmain() wab win32api windows.h



