CODEWITHSUNDEEP
cstringfunctionreturn
Jake Z
Jake Z

Reputation: 1133

C String Return Function Returns Garbage

I'm having trouble return a string from a function. It prints out a garbage value in the main method. I saw a similar question on this forum but the results on that page didn't help me. I DO NOT want to pass another variable into the function. I want to be able to return the string value as is. How can I go about doing so?

char *LookupPath(char **argv, char **dir)
{
    /* String Name To Be Returned */
    char path_name[MAX_PATH_LEN] = {0};
    char *result = malloc(sizeof(path_name));
    int i;

    /* Check To See If File Name Is Already An Absolute Path Name */
    if(*argv[0] == '/') {

    }

    /* Look In Path Directories */
    for(i = 0; dir[i] != NULL; i++) {
        strncat(path_name, dir[i], sizeof(path_name));
        strncat(path_name, "/", sizeof(path_name));
        strncat(path_name, argv[0], sizeof(path_name));
        result = path_name;
        if(access(result, F_OK) == 0) {
            printf("result: %s\n", result);
            return result;
        }
        path_name[0] = '\0';
    }

    /* File Name Not Found In Any Path Variable */
    return NULL;
}

Your help is greatly appreciated!

Upvotes: 0

Views: 1304

Answers (4)

user2045557
user2045557

Reputation:

You are assigning values path_name more than it can actually hold.

    strncat(path_name, dir[i], sizeof(path_name));
    strncat(path_name, "/", sizeof(path_name));
    strncat(path_name, argv[0], sizeof(path_name));

should be:

    sprintf(path_name, "%s%s%s", dir[i],"/",argv[0]);

Upvotes: 2

Basile Starynkevitch
Basile Starynkevitch

Reputation: 1

You cannot return a local array (like your path_name) from a function. That local array is inside the call frame, which gets popped on return.

As others replied, you should do

strncpy(result, path_name, MAX_PATH_LEN);

and document the convention that the caller should free the result.

BTW, your code is quite inefficient; you are allocating a rather large chunk of MAX_PATH_LEN (often 4096) for a string which often is much smaller.

If using GNU extensions you could simply use asprintf(3) (see this) or at least remove your malloc and return strdup(path_name); and use strdup(3) (which is standard, not requiring any GNU extension).

And learn how to use valgrind.

Upvotes: 1

Yu Hao
Yu Hao

Reputation: 122443

result = path_name;

should be:

strcpy(result, path_name);

Or better, get rid of path_name and use result directly.

Note that you should remember to free result when it's not used, when returning it, free it in the function that calls it. Since you return NULL in failure, in that case, free it directly, or it's a memory leak.

And you are using strncat wrong, read the manual.

Upvotes: 3

Some programmer dude
Some programmer dude

Reputation: 409356

Because of this line:

result = path_name;

This reassigns result to point to the local variable path_name, which goes out of scope when the function returns. This also means you have a memory leak.

Instead of using the temporary path_name variable, write directly into result.

Upvotes: 0

Related Questions