CODEWITHSUNDEEP
csegmentation-faultvalgrind
Mahesh Bansod
Mahesh Bansod

Reputation: 2033

Segmentation fault- strcat

This is my code:

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

void main(int arge, char *argv[])
{
    FILE *f1;
    char ch,*fn="~/lyrics/";
    strcat(fn,argv[1]);
    strcat(fn,".txt");
    if( (f1 = fopen(fn,"r"))==NULL )
    {
        printf("\nWrong filename\n%s not found",argv[1]);
        return;
    }
    while((ch=getw(f1))!=EOF)
    {
        printf("%c",ch);
    }
}

I compiled it using gcc -g -o file file.c and the compiler gave no error messages. But when I run it I get the error message: 

Segmentation fault (core dumped)
Bad permissions for mapped region at address 0x8048659 at 0x402C36B: strcat 
(in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so) by 0x80484D6: main (lyrics.c:9)

Can anyone please help me?

Upvotes: 6

Views: 43165

Answers (4)

Incarnator
Incarnator

Reputation: 1

I think a better way to do this is by making a function like:

#include <string.h>

char* joinString(char* s1, char* s2) {
 char s[sizeof(s1) + sizeof(s2) + 1];
 strcat(s, s1);
 strcat(s, s2);
 return s;
}

Upvotes: 0

alk
alk

Reputation: 70981

This approach is not portable.

If using glibc you also might call asprintf() which just allocates as much memory as you need.

#include <stdio.h>

...

char * pFn = NULL;

if (-1 == asprintf(&pFn, "~/lyrics/%s.txt", argv+1);
{
  perror("asprintf()");
}
else
{
  ... /* use pFn */
}

free(pFn);

...

Upvotes: 1

md5
md5

Reputation: 23727

char *fn = "~/lyrics/";

Because fn could point on a string in read-only memory, you should declare fn as pointer to const char.

const char *fn = "~/lyrics/";

Then you can see there are some errors. Here is a better solution:

char fn[MAX_SIZE] = "~/lyrics/";

Here MAX_SIZE shall be the sum of the size of "~/lyrics/", the maximum length of argv[1] and the length of ".txt".

Upvotes: 6

Goz
Goz

Reputation: 62333

You don't have enough space in fn. By strcat'ing on to it you overwrite the end of its stack allocation and into the stack .. hence the segmentation fault.

You could try the following instead:

char fn[255];
strcpy( fn, "~/lyrics/" );
strcat( fn, argv[1] );
strcat( fn, ".txt" );

You just have to be sure that the whole path and filename can fit into 255 characters.

Alternatively you could do this:

char* fn = NULL;
int argvLen = strlen( argv[1] );
fn = malloc( 9 + argvLen + 4 + 1 ); // Add 1 for null terminator.
strcpy( fn, "~/lyrics/" );
strcat( fn, argv[1] );
strcat( fn, ".txt" );

And you have definitely allocated enough space for the string. Just don't forget to free it when you have finished with it!

Upvotes: 11

Related Questions