returning char *

This is a discussion on returning char * within the C Programming forums, part of the General Programming Boards category; Hi. Can anyone tell me what the problem with this function is? It basically takes a file path, searches for ...

  1. #1
    Registered User
    Join Date
    Sep 2006
    Posts
    230

    returning char *

    Hi. Can anyone tell me what the problem with this function is? It basically takes a file path, searches for the file name in it. It *should* return a pointer to the first character of the string (the file name). Here it is:
    Code:
    /*	returns pointer to file name from file path. */
    char * fileName(const char * filePath)
    {
    	int i, j; char fileName[FILENAME_MAX+1];
    	for (i = strlen(filePath), j=0 ; i > 0; i--, j++) 
    	{
    		if (filePath[i] == '\\') break;
    		fileName[j] = filePath[i];
    	}
    
    	return fileName;
    }
    it gives me the following warning:
    c:\abdallah\file encrypter (everything)\file encrypter vs\file encrypter\file encrypter\fileextension.c(74) : warning C4172: returning address of local variable or temporary.
    When I try to output it using printf like this:
    Code:
    printf("%s", fileName(filename));
    it doesn't output anything.

    can anyone help me out?

  2. #2
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    You are, after all returning a local variable.

    The problem with this is that the local variable is stored on the stack, above the return address back to the calling function. As soon as you return to the calling function, this memory is "freed".

    You have (at least) four options. I've listed them as in the order I think are "best" to "worst":
    1. Pass in a second parameter to use as an output buffer.
    2. Copy the string in the calling function, and modify the incoming string.
    3. Allocate memory using malloc, then return the pointer to this memory. Don't forget to free it after you've finished [this is why it's not a "top" suggestion].
    4. Use a static variable e.g. static char fileName[...] - not very good for re-entrant [e.g. multithreaded or recursive use].

    I'm not at all convinced your "clever" loop works, but perhaps it does - it's definitely not on the upper range of "easy to read" code.


    I would also suggest that you don't have variables and functions with EXACTLY the same name - it can be VERY confusing in the long run.

    --
    Mats
    Last edited by matsp; 12-03-2007 at 09:54 AM.
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  3. #3
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,543
    Quote Originally Posted by Abda92 View Post
    Code:
    /*	returns pointer to file name from file path. */
    char * fileName(const char * filePath)
    {
    	int i, j; char fileName[FILENAME_MAX+1];
    	for (i = strlen(filePath), j=0 ; i > 0; i--, j++) 
    	{
    		if (filePath[i] == '\\') break;
    		fileName[j] = filePath[i];
    	}
    
    	return fileName;
    }
    fileName is destructed when the function ends, so you're returning a pointer to an invalid memory location on the stack.
    Allocate memory on the heap using malloc, modify that memory and then return that pointer. Or take a buffer as argument to work with. Either works fine (I recommend the second).

  4. #4
    Jack of many languages Dino's Avatar
    Join Date
    Nov 2007
    Location
    Katy, Texas
    Posts
    2,309
    Are you sure you want the file name to be reversed?

    Todd

  5. #5
    Registered User
    Join Date
    Sep 2006
    Posts
    230
    Yes. about the function body, I had made a few fixes but I couldn't post them. Here's the new code using the 1st solution from matsp:
    Code:
    int extractFileName(const char filePath[], char fileName[], size_t fileNameLength)
    {
    	int i, j;
    	for (i = strlen(filePath); filePath[i] != '\\'; i--) 
    		if (i == 0) return 0;
    	i++;
    	for (j=0 ; filePath[i] != '\0' && j < fileNameLength; j++, i++) 
    		fileName[j] = filePath[i];
    	fileName[j] = '\0';
    	return 1;
    }
    But just to make things clear, you can not return a string from a function in any way?
    Thanks
    Last edited by Abda92; 12-03-2007 at 11:52 AM. Reason: corrected code

  6. #6
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,543
    You can, if you allocate it on the heap, but it's usually considered better for the callee to pass in a buffer instead.
    Also, since this is C, pass along the length of the buffer too, so you don't get buffer overruns.

  7. #7
    and the hat of sweating
    Join Date
    Aug 2007
    Location
    Toronto, ON
    Posts
    3,545
    Quote Originally Posted by Abda92 View Post
    But just to make things clear, you can not return a string from a function in any way?
    Thanks
    Not a local string (i.e. an array of chars). You can do either of these:
    Code:
    char* func()
    {
        char* str = malloc( 80 * sizeof( char ) + 1 );
        strncpy( str, "Hello World", 80 );
        return str;  /* Now the calling function must free() the string. */
    }
    or

    Code:
    void func( char* str, unsigned int size )
    {
        strncpy( str, "Hello World", (size - 1) );
    }
    The second method is usually better since the person calling the function knows for sure that they are responsible for memory allocation, plus then they also know the amount of memory allocated.

  8. #8
    Registered User
    Join Date
    Sep 2006
    Posts
    230
    OK I corrected the code in my last post just in case someone wants it.

  9. #9
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,543
    The whole point of passing along the size of the buffer is to make sure that you don't do a buffer overrun. Remember, that function has no idea how large the buffer is.
    So go back and revise your code and make a check. When accessing fileName, check if you aren't accessing an element outside the length of the buffer.
    In other words, if j == fileNameLength, then you have a problem because you're writing to the last element in the buffer and you need enough room to save a '\0' too.
    If the buffer isn't enough for your function, ABORT.
    You can also check in the beginning of the function if there's enough space.

  10. #10
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Also, are you sure that you want to return zero when you don't find a backslash in the filename? Is "text.txt" not a valid input to your function?

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  11. #11
    Registered User
    Join Date
    Sep 2006
    Posts
    230
    to Elysia:
    yes I did check for that in (j < fileNameLength). doesn't that make sure that we're not overrunning the buffer?

    to matsp:
    I did that on purpose. in the calling function I checked the return value to know what to output. e.g.
    Code:
    /* if rfilePath is actually the file name (as in typing the file name) extractFileName returns 0 */
    if (extractFileName(wfilePath, fileName, FILENAME_MAX))
    	printf("\n&#37;s opened successfully\n", fileName);
    else printf("\n%s opened successfully\n", wfilePath);
    here, I input the file using wfilePath. But I don't want to show the user the whole path when I confirm it has succeeded in opening the file. If it is a path, I only want to show the file name. If it is a file name, then I want to show it.

    Thanks for the help
    Last edited by Abda92; 12-03-2007 at 12:40 PM.

  12. #12
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,543
    Quote Originally Posted by Abda92 View Post
    to Elysia:
    yes I did check for that in (j < fileNameLength). doesn't that make sure that we're not overrunning the buffer?
    Actually, I believe there's a subtle bug in there.
    Let's say that fileNameLength = 12.
    That means the last valid index = 11 (since index start at 0).
    So your array loops until j = 11, then the condition j < fileNameLength is met and the loop aborts, BUT j is incremented to 12 first (so j < fileNameLength is true).
    So, then you do fileName[12] = '\0', that is one char beyond the array length.

  13. #13
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    As to returning values and functionality: I would much prefer to see a function that does a copy every time, and no checking of the return value at all [or use the return value to indicate success/fail if applicable].

    The reason for this is that the code calling the function becomes much simpler.

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  14. #14
    Captain Crash brewbuck's Avatar
    Join Date
    Mar 2007
    Location
    Portland, OR
    Posts
    7,239
    You are working way too hard.

    Code:
    const char *fileName(const char *filePath)
    {
        const char *slash = strrchr(filePath, '\\');
        return (slash && slash[1]) ? slash + 1 : NULL;
    }

  15. #15
    Deathray Engineer MacGyver's Avatar
    Join Date
    Mar 2007
    Posts
    3,211
    lol brewbuck.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Replies: 12
    Last Post: 08-11-2008, 11:02 PM
  2. get keyboard and mouse events
    By ratte in forum Linux Programming
    Replies: 10
    Last Post: 11-17-2007, 04:42 PM
  3. newbie needs help with code
    By compudude86 in forum C Programming
    Replies: 6
    Last Post: 07-23-2006, 08:54 PM
  4. returning char arrays!!!!
    By bobthebullet990 in forum C Programming
    Replies: 2
    Last Post: 03-30-2006, 06:05 AM
  5. How do you search & sort an array?
    By sketchit in forum C Programming
    Replies: 30
    Last Post: 11-03-2001, 04:26 PM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21