CODEWITHSUNDEEP
csegmentation-faultstrcpy
user1493813
user1493813

Reputation: 991

C strange error with strcpy and even stranger solution

I'm creating a program to open .txt files in a given directory, I have an array with all the absolute paths of the files inside the directory in question and I'm creating a function to extract and return the name of the files, the function is written as follows:

char *name(char *string) {
    int i = strlen(string);
    char *aux;
    while(string[i-1] != '/'){
        i--;
    }
    strcpy(aux, &string[i]); 
    return aux;
}

The above function is giving a Segmentation Fault error, but if I add the following line " int j = 0;" before the declaration of aux the mistake is gone, the new and working code is 

char *name(char *string) {
    int i = strlen(string);
    int j = 0;
    char *aux;
    while(string[i-1] != '/'){
        i--;
    }
    strcpy(aux, &string[i]); 
    return aux;
}

input: C:\test\a.txt
output: a.txt

Why the addition of "int j = 0;" solves the problem? I'm stuck with that and can't continue because I don't know if this inconsistency might lead to bigger problems later, I'm thinking about writing my own function to copy the strings, but before that I really want to understand that error.

Upvotes: 1

Views: 969

Answers (3)

houbysoft
houbysoft

Reputation: 33412

You never allocate aux. aux needs to point to a valid memory location before you attempt to copy anything to it.

Instead of char *aux, you need something like char *aux = malloc(i+1);. Note that i+1 is overkill because in your case aux will always be at least 3 characters shorter than string (it won't contain C:\), but you probably don't care for such small strings. Remember to free() the pointer once you're done with it.

Also, the reason you found it works by switching orders of declarations and/or adding a declaration is probably that you got lucky and somehow the location to which aux points to is valid (if you do just char *aux;, aux points to a random location). This is pure luck however, and is still invalid code even though it seems to work.

In the future, you might want to use a tool like Valgrind to help you diagnose memory problems. You should also read a tutorial on basic memory management and pointers in C.

Upvotes: 3

Paradigm
Paradigm

Reputation: 141

Since it sounds like you are only interested in utilizing the filename portion of the string as parameter, another option is to use the portion of the string you already have.

Try: aux = &string[i]; instead of the strcpy.

This gives you a pointer into the part of the string you are interested in (namely, the last part after the final '/').

Secondly, ensure you have a '/' in all of your input strings, otherwise bad things will happen (i.e. your loop will go past the beginning of the string, likely encountering a segmentation fault at some point). It would be best to put a condition on the loop so that it doesn't continue beyond i = 1.

Upvotes: 3

Nick
Nick

Reputation: 669

You don't allocate any memory to aux. You are trying to write to memory through an uninitialized pointer.

Upvotes: 1

Related Questions