You say that this should be C89 code, but there are some aspects of this code that are not C89 compliant. I have included a modified version of your code below that fixes these things, compiles without warnings, and seems to run correctly. By way of a disclaimer, I never write in C89, so someone else may take issue with something here.
The segfault that you report is due to the double-declaration of fp, as pointed out by @BLUEPIXY in the comments to your question. You declare this file pointer first at file scope (i.e., fp is a "global" variable), and then at block scope within the function initialize(). The second declaration hides the first one within the block, so you open a file in initialize() and assign the resulting pointer to the block scope fp (which has automatic storage duration). When main() then calls readFile(), it is the file scope fp that is passed, and this fp is initialized to NULL. So fseek(fp, 0, SEEK_END) is called on a NULL pointer, and this causes the segfault. Changing the second declaration to an assignment fixes this problem:
fp = fopen(argv[1], "rb");
You can't mix variable declarations and code in C89, so you need to move the declarations to the beginning of the functions. Also, C89 does not support the %zu format specifier for size_t variables. Instead, use %lu and cast the size_t value bytesRead to (unsigned long).
I added a definition: ONEMB 1048576, and removed the TENMB definition. In the calculation of the file size, you were dividing the number of bytes by 10MB instead of 1MB, and I take it that you were in fact trying to calculate the number of MB.
I added a check for errors to the call to ftell(). Since ftell() returns a long, I added a size_t fileSize variable and cast the value of size to size_t before assigning it to fileSize. I changed theprintf() statement to:
printf( "File size: %.2f MB\n", (fileSize * 1.0)/ONEMB );
so that the file size is reported with more precision; this way, files smaller than 1 MB will not be reported as having a size of 0 MB.
I removed the cast from the call to malloc(), as it is absolutely not needed in C.
The assignment to bytesRead calls fread(), which takes a size_t parameter for the third argument, not a long. You originally had the long value size here, and this is one reason for the new fileSize variable. The following line had a comparison between bytesRead, which is size_t, and size. This should generate compiler warnings (you do have them on, don't you?) as using signed and unsigned types in the same expression can lead to difficulties, and so here I used the fileSize variable again.
You were missing the return statement at the end of main(), and your function definitions were missing their return type specifiers, so I also added these. I also noticed that your program was segfaulting when invoked with no arguments. This is because on errors in your initialize() function, you were returning to the calling function, which then called readFile() with a NULL pointer. You should instead exit() from the program. I have made these changes to all file error traps in the modified code.
There are some other issues with your code. I would prefer not to use the End() function. It does not test for errors during closing files, and will segfault if you pass in a NULL pointer. Also, the "Unexpected number of bytes read" error in readFile() will end up exiting successfully, since End() always exits with EXIT_SUCCESS.
I don't love the casting from long to size_t that I did, and someone else may have a better approach. What I was trying to manage here was that ftell() returns a long, which you use to calculate the file size, and fread() both takes and returns size_t values.
Most of what I did was in the readFile() function, and I removed your comments from this function to make it easier to see what was changed.
I compiled with:
gcc -std=c89 -Wall -Wextra -pedantic
#include <stdio.h>
#include <stdlib.h> /*added this to provide a declaration for malloc*/
#define ONEMB 1048576
#define ONEB 1
FILE * fp = NULL;
void End(FILE * fp)/*made END a function */
{
fclose(fp); /* close and free the file */
exit(EXIT_SUCCESS); /* or return 0; */
}
int initialize(int argc, char ** argv)
{
/* Open the file given on the command line */
if( argc != 2 )
{
printf( "Usage: %s filename.mp3\n", argv[0] );
exit(EXIT_FAILURE);
}
fp = fopen(argv[1], "rb");
if( fp == NULL )
{
printf( "Can't open file %s\n", argv[1] );
exit(EXIT_FAILURE);
}
return 0; /*this might need to change */
}
int readFile(FILE * fp)
{
long size = 0;
unsigned char *data;
size_t fileSize, bytesRead;
fseek(fp, 0, SEEK_END );
if ((size = ftell(fp)) == -1) {
fprintf(stderr, "ftell() error\n");
exit(EXIT_FAILURE);
};
rewind(fp);
if( size < ONEB || size > 10 * ONEMB )
{
printf("File size is not within the allowed range\n");
End(fp);
}
fileSize = (size_t) size;
printf( "File size: %.2f MB\n", (fileSize * 1.0)/ONEMB );
data = malloc(fileSize);
bytesRead = fread( data, sizeof(unsigned char), fileSize, fp );
free(data);
if( bytesRead != fileSize )
{
printf( "Error reading file. Unexpected number of bytes read: %lu\n", (unsigned long) bytesRead );
End(fp);
exit(EXIT_FAILURE);
}
return 0;
}
int main( int argc, char ** argv )
{
initialize(argc, argv);
readFile(fp);
/* We now have a pointer to the first byte of data in a copy of the file, have fun
unsigned char * data <--- this is the pointer */
End(fp);
return 0;
}
