Segmentation fault when opening file.

[Hardcheese]

Hi,

I am having trouble when loading files from the disk because I keep getting 'Segmentation fault' errors which I know is something to do with the memory and I have narrowed it down to this function in my code:

Code: Select all

MEMORY *mopen( char *filename, unsigned char relative_path )
{

		FILE *f;
		
		char fname[ MAX_PATH ] = {""};
		
		if( relative_path )
		{
			get_file_path( getenv( "FILESYSTEM" ), fname );
			
			strcat( fname, filename );
		}
		else strcpy( fname, filename );

		f = fopen( fname, "rb" );
		
		if( !f ) return NULL;
		
		
		MEMORY *memory = ( MEMORY * ) calloc( 1, sizeof( MEMORY ) );
		
		strcpy( memory->filename, fname );
		
		
		fseek( f, 0, SEEK_END );
		memory->size = ftell( f );
		fseek( f, 0, SEEK_SET );
		
		
		memory->buffer = ( unsigned char * ) calloc( 1, memory->size + 1 );
		fread( memory->buffer, memory->size, 1, f );
		memory->buffer[ memory->size ] = 0;
		
		
		fclose( f );
		
		return memory;

Any help you can give is appreciated.

[PeterO]

Have you run it under gdb ? That will show you which statement is causing the segfault.

Compile with "-g" switch to gcc to include debug info...
then load it into gdb with "gdb your_file_name_here"
then at the gdb prompt type "run".....

HTH
PeterO

[rurwin]

It looks good, and it runs well here in a test harness.

I made some assumptions:

Code: Select all

#define MAX_PATH (1024)

typedef struct MemStruct
{
	char filename[MAX_PATH];
	int	size;
	char * buffer;
} MEMORY;

filename is not char* in your code I assume?

You should test the return values from calloc, but that is probably not your problem; running out of memory is rare. You should also test the value mopen returns; using the memory buffer when the file does not exist results in a segmentation error.

[gordon@drogon.net]

It looks like you allocate space for memory, but not memory->filename - which you promptly copy fname into.

calloc() zeros memory it allocates, so you're trying to copy into a null pointer.

-Gordon

[Hardcheese]

Thanks for your replies.

From running the program under gdb as PeterO suggested the problem originates from the strlen() function call in a function which is referenced by a function mopen calls called adjust_file_path i'll post the code here for those two functions:

Code: Select all

void adjust_file_path( char *filepath )
{
	unsigned int i = 0,
				 l = strlen( filepath ); // This is where the program fails!
	
	while( i != l )
	{
		if( filepath[ i ] == '\\' ) filepath[ i ] = '/';
		++i;
	}
}


void get_file_path( char *filepath, char *path )
{
	char *t = NULL;
	
	unsigned int p;
	
	adjust_file_path( filepath );

	t = strrchr( filepath, '/' );
	
	if( t )
	{
		p = ( t - filepath ) + 1;
		strncpy( path, filepath, p );
		path[ p ] = 0;
	}
}

I read online that a string has to be terminated with an 0 otherwise strlen will run off the edge. Do you think this is the problem? If so how would you implement it?

[AndyD]

The pointer returned by getenv() could be NULL! You need to check for NULL pointers in your code. If the pointer is NULL you will get a Segmentation fault in the call to strlen.

Code: Select all

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

int main(void)
{
    printf("%d\n", strlen(getenv("FOO")));
    return 0;
}

This program dies with a segmentation violation as I don't have FOO in my environment.

One of the annoying things about C programming is that memory corruption will often cause errors/crashes in code that is not related to the code that actually caused the corruption. This is why programs like Valgrind are really useful.

One possible problem is that you are modifying the string returned by getenv(). From the man page for getenv

Code: Select all

As typically implemented, getenv() returns a pointer to a string within
the  environment  list.   The  caller must take care not to modify this
string, since that would change the environment of the process.

So you need to take a copy of the string returned by getenv() and modify the copy.
Are you modifying any other environment variable? If so this may be the actual cause of your corruption! If strlen() fails here then it may mean that the string returned by getenv( "FILESYSTEM" ) is corrupt.

You are correct about strings being terminated by 0. Using this knowledge, you could rewrite adjust_file_path() to look like this...

Code: Select all

void adjust_file_path( char *filepath )
{
   while( *filepath != 0)
   {
      if( *filepath == '\\' ) *filepath = '/';
      ++filepath;
   }
}

This uses pointer arithmetic rather than treating the pointer as an array. In your code the string was being traversed twice (once to get the length, once to make the changes), but this code only traverses the string once.