user012225474
user012225474

Reputation: 45

Changing pointer char value in function

I try to parse client HTML GET request and get address of file he wants to send. But when i pass argument to function in path variable is bad output. Parsers function printf: New file path returned is=/somedir/index.html Main fuction printf: In main path=random chars

What is wrong?

int parser(char* buffer,char **newPath)
{
int numberOfChars = 0;
int index = 4;//start at 4 char

while(buffer[index] != ' '){
            numberOfChars++;
            index++;
}
// in numberOfChars is number of path characters from while loop
// this part of code is in if statment but it is irrelevant now
char filePath[numberOfChars];    
strncpy(filePath,buffer+4,numberOfChars);
char* fullPath;
fullPath = filePath;                
char name[] = "index.html";
strcat(fullPath,name);

(*newPath) = fullPath;
printf("New file path returned is=%s\n",(*newPath));
return 1;
//some more code if file is .sh or .bash or .png ...
.
.
}

main

int main(int argc, char *argv[])
{
            //some code
            .
            .
            .
            char* path;
            //msg is client HTML get request i want parse it and send data
            // which he wants
            parser(msg,&path);
            printf("In main path=%s\n",path);
}

Upvotes: 2

Views: 61

Answers (2)

P.P
P.P

Reputation: 121427

char filePath[numberOfChars];    
char* fullPath;
fullPath = filePath; 
....
(*newPath) = fullPath;

filePath has automatic storage duration and you are accessing it after its lifetime is over. This is undefined behaviour.

Instead, you could use malloc() and strcpy() to copy the string into *newPath.

Replace

(*newPath) = fullPath;

with:

*newPath = malloc(strlen(fullPath) + 1);
 if (*newPath == NULL) {
    /* handle error */
 }
strcpy(*newPath, fullPath);

and call free() in main() to de-allocate it.


As noted in the comments, you need to allocate one extra byte for the '\0' terminator. You can handle this by allocating an extra byte:

char filePath[numberOfChars + 1];    
strncpy(filePath,buffer+4,numberOfChars);
filePath[numberOfChars] = '\0';

strncpy() automatically fills rest of the with the memory with NUL bytes. In this case, just the last byte. In general, you don't want to use strncpy() as that would be unnecessary to fill the rest of the buffer with NUL bytes.

Upvotes: 1

dbush
dbush

Reputation: 225817

You're returning a pointer to a local variable. When the function exits, that variable goes out of scope and the memory it occupied can be used for other purposes, leading to undefined behavior.

Your function needs to dynamically allocate space for the string (and the NULL terminator) and return a pointer to that buffer. You then need to be sure to free it in the calling function:

int parser(char* buffer,char **newPath) {
    ....
    char *filePath = malloc(numberOfChars+1);
    if (filePath == NULL) {
        perror("malloc failed");
        exit(1);
    }
    ...
}

int main(int argc, char *argv[])
{ 
    ...
    parser(msg,&path);
    printf("In main path=%s\n",path);
    free(path);
}

Upvotes: 1

Related Questions