5

I've looked all over for an answer to this but can't seem to find one. (I have fairly limited experience with C++)

In my library, I free a string. (Amazing, huh?)

This is where the problem arises. I have a struct which contains a char* that may be allocated on the heap or may not be. While it is a valid pointer it cannot be freed.

IE

char* s1 = "A String";
char* s2 = (char*)memcpy(malloc(9), s1, 9);

free(s2);
free(s1);

Will cause an error on "free(s1);" (As it should) Because s1 does not actually need to be freed, (It isn't on the heap) how can I handle this in an "acceptable" way? (On similar topics the answer of "let it crash" didn't seem reasonable IMO)

Because the struct is not solely created by the library, it is not possible to guarantee that a string will be properly copied over using something like memcpy.

Seeing as this is a windows library, I don't need to worry about using ISO C stuff or standard C functions.

James
  • 1,569
  • 10
  • 18
  • Best answer (IMHO): Use an opaque pointer to guarantee that the `struct` _is_ solely created in the library. (I would offer more help if I used Windows. Sorry. +1 though.) – Chris Lutz Jan 01 '11 at 05:11
  • Thanks, if worst comes to worst I can either do that or use SEH (Although the latter would make me feel like I was doing something wrong) – James Jan 01 '11 at 05:29
  • Seeing as this is a Windows library, make the argument a BSTR. Then you require the user to allocate it properly (with `SysAllocString`) and you're guaranteed to use a matching deallocator. Other methods are just... bad. If your user has a different compiler, then you can't `free()` the string even if they did use `malloc` – Ben Voigt Jan 01 '11 at 05:46
  • possible duplicate of [Howto check if a char* points to a string literal in C](http://stackoverflow.com/questions/3398719/howto-check-if-a-char-points-to-a-string-literal-in-c) – zneak Jan 01 '11 at 05:56
  • I'm going to use (gasp) IsBadWritePtr on my strings before I free them... If it is a string literal, it will return true and I won't attempt to free the string. – James Jan 01 '11 at 06:15
  • If anyone can provide a way for me to check if a string literal is being used (perhaps using fancy compiler magic?) please tell me. (zneak is correct, I did not find that question when I searched) – James Jan 01 '11 at 06:21
  • @James: Either make the client tell you or require that the client not give you a string literal. Remember, if you document the requirements of your interface and the client violates those requirements, it's perfectly acceptable to crash. In any case, Ben's suggestion is probably the cleanest approach. Why not consider that. (As for `IsBadWritePtr`, just don't. To quote Michael Howard via [Larry Osterman's blog](http://blogs.msdn.com/b/larryosterman/archive/2004/05/18/134471.aspx), you may as well call the function `CorruptMemoryAndCrashMySystem`). – James McNellis Jan 01 '11 at 06:27

7 Answers7

6

In C++, you shouldn't be worrying about this at all. Use std::string and have it manage memory for you, automatically. Don't manage memory manually.

If you were to do this manually, you would need to manage the resource yourself, by

  • making the user of the library manage the memory himself, or
  • requiring the user to tell you how to manage the memory, or
  • telling the user how you are going to manage the memory and then expecting the user to comply.
Community
  • 1
  • 1
James McNellis
  • 348,265
  • 75
  • 913
  • 977
  • 1
    This is great advice, except for the fact that it's (1) a library (2) on Windows and (3) apparently precompiled, according to the comments. And `std::string` is decided unsafe to pass across module boundaries on Windows. Following the pattern of the Win32 API, the first bullet you give is by far the preferred way for Windows libraries to handle memory. – Ben Voigt Jan 01 '11 at 05:43
  • @Ben: Windows programming: not my forte (funny, I suppose). You're right, of course: if you can't control the build process for both the library and whatever links against it, this would be quite problematic. I'd upvote your BSTR solution if you posted it as an answer. That's the most reasonable solution, IMO. – James McNellis Jan 01 '11 at 06:17
2

Seeing as this is a Windows library, make the argument a BSTR. Then you require the user to allocate it properly (with SysAllocString) and you're guaranteed to use a matching deallocator.

Other methods are just... bad. If your user has a different compiler, then you can't free() the string even if they did use malloc.

[Note: Converted from a comment at James's request, this really is just a Windows-specific case of the last of his suggestions]

Further note: BSTR is Unicode. I kinda sorta remember seeing a way to use the BSTR allocator to store ANSI strings, seems that SysAllocStringByteLen does that, but be warned that putting ANSI data in a BSTR will be highly counterintuitive to anyone familiar with BSTR.

Community
  • 1
  • 1
Ben Voigt
  • 277,958
  • 43
  • 419
  • 720
  • Alright, but I seem to remember have some bad experiences with bstr when I was required to use it (and SAFEARRAY) when dealing with .NET code from native C++. That's Off-topic though, but I'll give it a try. – James Jan 01 '11 at 06:32
  • @James: I suspect it was the COM code that caused difficulty and not BSTR. The BSTR functions are actually really simple and easy to use from C or C++. I think that VC++ even provides a wrapper class to automatically free the BSTR as it goes out of scope, but it doesn't add much functionality and not knowing exactly how it works bothered me, so I just always called `SysAllocString` and `SysFreeString` directly. – Ben Voigt Jan 01 '11 at 06:37
0

You can malloc char *s1 and place "A String" as value. After that, you can free s1.

Neilvert Noval
  • 1,655
  • 2
  • 15
  • 21
0

This is the sort of thing that's known at compile time. You look at the code and know what to free and what not to. So don't think of a way to defer it to runtime, because apart from the fact you won't find a way in this case, it's the wrong way of doing things in C++. When you can do it statically, do it statically.

Use the type system. Use RAII.

wilhelmtell
  • 57,473
  • 20
  • 96
  • 131
  • The problem is that it isn't know at __my__ compile time, only for the people actually calling my library. A pointer is just a number. – James Jan 01 '11 at 05:26
  • 1
    @James this is about **ownership**. The question to ask is who owns the pointer. This is something you say in the function's documentation. Either way, the caller shouldn't check for whether it should free the pointer or not. Again, this is static information that you need to be clear about. – wilhelmtell Jan 01 '11 at 05:52
0

Store all strings in the heap, then you'll know they all need to be freed. If the string exists as in global memory, copy it to a heap buffer.

Jonathan Wood
  • 65,341
  • 71
  • 269
  • 466
  • Whose heap? On Windows, each library tends to get its own incompatible malloc/free heap. The Win32 API functions `HeapAlloc` and `HeapFree` work across module boundaries, but they aren't compatible with CRT-provided malloc and free in any of the modules. – Ben Voigt Jan 01 '11 at 06:30
  • The heap used by malloc(), per how the OP allocated the string that was stored on the heap. – Jonathan Wood Jan 01 '11 at 06:32
0

I have done something similar in the past, where I would use a stack-allocated string in some cases, and a heap-allocated string in others, but they ultimately get passed off to some other common code.

What I have done in that case, is to have two pointers. One is either NULL, or a heap-allocated string. The other pointer points to either the stack-allocated or the same heap-allocated memory as the former. When you go to do your free(), you check the former pointer only.

Of course, that's going to look nasty in an exposed API. In my case it was internal code only.

axw
  • 6,908
  • 1
  • 24
  • 14
0

For things involving memory allocation in C++, smart pointers are awesome. Just sayin'.

http://www.cplusplus.com/reference/std/memory/auto_ptr/

http://www.boost.org/doc/libs/1_45_0/libs/smart_ptr/smart_ptr.htm

Hank
  • 547
  • 3
  • 4