return string pointer from function in C

Question

Here is my code:

TCHAR *sResult = (TCHAR *) calloc(16384+1, sizeof(TCHAR)); sResult = (TCHAR *) GetValueFromFile(sFilename,L"Dept",L"Names"); // #1 _tcscpy(sResult,(TCHAR *) GetValueFromFile(sFilename,L"Dept",L"Names"); // #2

Function:

TCHAR *GetValueFromFile(TCHAR *sFilename,TCHAR *sDept,TCHAR *sNames) { ... }

Which is correct to do? #1 or #2?

Thanks everyone. Edit #1:

I'm using VS2008 in .cpp files, but really just C code.

I just need to open a file in GetValueFromFile and send the return string back. Should I be allocating memory in GVFF and freeing it in my program?

main() { TCHAR *sResult; DWORD dwRetVal = GetValueFromFile(sFile,L"Dept",L"Name", &sResult); ... free(sResult);sResult=NULL; }

Like this?

DWORD GetValueFromFile(TCHAR *sFilename,TCHAR *sDept,TCHAR *sNames, TCHAR ** sValueData) { dwL = GetStringDataLength(…) *sValueData = (TCHAR *) calloc(dwL+1, sizeof(TCHAR)); _tcscpy_s(sValueData,dwL,sDataFromFile); }

How does the returned TCHAR* in GetValueFromFile get allocated? — JaredPar
– JaredPar, Commented Mar 14, 2012 at 19:59
@KerrekSB casting hides error messages :-) Basically it means "I know what I'm doing" and when one turns out to be wrong... — Tomas Pruzina
– Tomas Pruzina, Commented Mar 14, 2012 at 20:15
@EdS.: More importantly, you certainly don't need to cast the return value of a function to its own type :-) — Kerrek SB
– Kerrek SB, Commented Mar 14, 2012 at 20:17
If you compile C with a C++ compiler what do you expect ? The real question is why are you compiling C with a C++ compiler (and mutilating the code to comply with the weirdness of the compiler and/or the common subsyntax) ? — wildplasser
– wildplasser, Commented Mar 14, 2012 at 21:29

Ed Swangren · Accepted Answer · 2012-03-14 20:39:58Z

Well, first off, this is unnecessary in case 1 and leads to a problem:

TCHAR *sResult = (TCHAR *) calloc(16384+1, sizeof(TCHAR));

I don't know where 16384+1 is coming from, so I'll assume that's "correct" for now, but you proceed to set the pointer to another value in the very next line. That, my friend, is a memory leak. Why are you allocating memory that you don't need?

Your question really boils down to the implementation of GetValueFromFile. If that function returns a pointer (which it does) then it should certainly be a valid pointer and you are responsible for deallocating it (probably. Again, depends on the implementation).

There is no need to create a copy unless you actually need a copy. There is no "right" or "wrong" here from the information you have given us, we need to know the details of GetValueFromFile.

Per your edit:

That function does not return anything. At all. It's signature says it returns a DWORD, not a TCHAR*. It is obviously different than your first example as it initializes the pointer as an output argument (the 4th one), but that is not how you are calling it in your original example.

I am just further confused now, but if the function initializes the pointer then you need only declare (no memory allocation outside of the function) the pointer and pas in its address.

Given

DWORD GetValueFromFile(TCHAR *sFilename,TCHAR *sDept,TCHAR *sNames, TCHAR ** sValueData)

Then the proper way to pass your pointer in is:

TCHAR *result; GetValueFromFile(filename, dept, names, &result);

The function initializes your result variable to point to a valid location. Do not forget that you are now responsible for deallocating it!

The calloc() is only "unnecessary" (read: wrong, as in "memory leak") in case #1. In case #2, the calloc() is necessary, since we need to allocate memory to copy into.
@DanielRoethlisberger: Yes, that's true. Like I said though, it comes down to what GetValueFromFile is returning, how it is allocated, and if you need a copy. We have no idea what the answer to any of those questions is. I've updated my response though, thanks.
@JeffR: You don't, but you shouldn't post it in a comment anyway. Just edit your post.

Jerry Coffin · Accepted Answer · 2012-03-14 20:05:14Z

1 clearly is not correct. You start by allocating space with calloc, but then you overwrite the point to that space with the return from GetValueFromFile. Assuming no intervening code that saves that pointer somewhere else, this means you no longer have access to the memory you allocated, so you can no longer access or free it. In short, a big memory leak.

Depending on how GetValueFromFile allocates the memory to which it returns a pointer, #2 might be just as bad, but the code you posted doesn't tell us enough to know. If it returns something like a pointer to a statically allocated buffer, then chances are it's correct (for some definition of correct). If it's allocating space with something like malloc or calloc, and expecting the client code to release that when needed, then it has a problem similar to that in #1.

Note that the "for some definition of correct" basically translates to something like: "wrong the minute you have more than one thread, or any need for reentrancy". Given the current ubiquity of multicore and multiprocessor systems, that definition is pretty narrow now, and getting narrower all the time.

@JeffR: Yes, that's certainly one way you can do things. If you do that, you'd make the call like your #1, but eliminate the preceding call to calloc.

Collectives™ on Stack Overflow

return string pointer from function in C

2 Answers 2

7 Comments

1 Comment

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

7 Comments

1 Comment

Related