13 Nov, 2009, Lobotomy wrote in the 1st comment:
Votes: 0
I've been Googling and scratching my head for a while now over this problem, and I've yet to come up with an answer/explanation. I have a variable that I'm assigning a cleanup attribute which has the program call free() on it once the variable goes out of scope. In C (gcc), it works without a hitch. From what I've read in GCC's documentation, it should work the same in g++. However, it is not; instead, I'm getting the valgrind error: "Invalid free() / delete / delete[]".

Consider the following example of a bug() function:
void bug( const char *fmt, … )
{
assert( fmt );

char *buf = NULL;
va_list va;
va_start( va, fmt );
vasprintf( &buf, fmt, va );
va_end( va );

// Using cout for testing purposes. Will be replaced later with an in-game channel.
cout << buf << endl;

free( buf );
}

The need to call free comes from using vasprintf, as it will automatically allocate (using malloc) a string to hold the resulting formatted string. The above function is pretty simplistic, and has only the one exit point, so having to specify a free() isn't so much of an issue there. However, I prefer to still use the cleanup attribute so that remembering to use free() at all future exit points becomes unnecessary. Anyhow, the above function works fine in g++ and doesn't produce any errors in valgrind.

However, the aforementioned error occurs when the code is written like this:
void bug( const char *fmt, … )
{
assert( fmt );

char *buf __attribute__( ( cleanup( free ) ) ) = NULL;
va_list va;
va_start( va, fmt );
vasprintf( &buf, fmt, va );
va_end( va );

// Using cout for testing purposes. Will be replaced later with an in-game channel.
cout << buf << endl;
}

Is there something I'm missing about using the cleanup variable attribute in g++? Could valgrind just be giving me a false positive? I'm completely baffled at this point. :sad:

Also, before I forget, there are two other important points to address:
1) I know about streams. I don't want to use streams. The syntax is awkward to me, and not nearly as convenient as the printf-style format syntax.
2) I know about the Boost format library. I don't want to use that either, for the same reason as #1, among other reasons.
14 Nov, 2009, Davion wrote in the 2nd comment:
Votes: 0
Not to familiar with the attributes for gcc extensions, but

char *buf __attribute__( ( cleanup( free ) ) ) = NULL;


That syntax looks all freaky to me :P. Maybe inline assigning the value to NULL is tweaking it out. What line specifically is Valgrind reporting that on?
14 Nov, 2009, David Haley wrote in the 3rd comment:
Votes: 0
How important is it to you to be able to use these things?

Maybe vasprintf is doing something funky. What is the function invocation of 'bug' here? Maybe it's not actually allocating memory, which is causing confusion.

When it comes to funky extensions and attributes, well, there be dragons that way.
IMHO it looks like you would rather be programming in a different language. :wink:
14 Nov, 2009, Runter wrote in the 4th comment:
Votes: 0
My only advice here is this:

Do more testing and post a follow up. You need to eliminate factors and confirm that under normal circumstances in g++ this is working. In other words, how about:
void bug(void) {
char *buf __attribute__( ( cleanup( free ) ) ) = (char*)malloc(1);
}


etc. Once you confirm exactly what is breaking it then it may be more clear on possible causes. That being said. Extensions, woooof.
14 Nov, 2009, Lobotomy wrote in the 5th comment:
Votes: 0
David Haley said:
How important is it to you to be able to use these things?

The printf syntax is what I'm most comfortable working with so far when it comes to generating displays and the like, among several other things, so it's rather important.

David Haley said:
Maybe vasprintf is doing something funky. What is the function invocation of 'bug' here? Maybe it's not actually allocating memory, which is causing confusion.

I've already ruled that out. The function spits out the allocated string before ending, and it displays just fine so it is being allocated; it's when free() is called by cleanup that the error occurs.

Quote
IMHO it looks like you would rather be programming in a different language. :wink:

I'd still like to give Go a try, but for the time being I'm waiting for Zeno to get around to installing it on his server (if he ever does, anyways). :thinking:

Runter said:
My only advice here is this:

Do more testing and post a follow up. You need to eliminate factors and confirm that under normal circumstances in g++ this is working. In other words, how about:
void bug(void) {
char *buf __attribute__( ( cleanup( free ) ) ) = (char*)malloc(1);
}


etc. Once you confirm exactly what is breaking it then it may be more clear on possible causes. That being said. Extensions, woooof.

Well, I was already fairly certain that cleanup was the culprit, and after running your suggested test it's more or less confirmed. I opted to just stick the code at the start of main and short-circuit the rest of the program with a return so I wouldn't have to wait for anything else to run to get the results, but I digress. The results are the same as before:

int main( int argc, const char* argv[] )
{
char *test __attribute__( ( cleanup( free ) ) ) = (char*)malloc(1);
return 0;

Valgrind said:
==6675== 1 errors in context 1 of 1:
==6675== Invalid free() / delete / delete[]
==6675== at 0x401C4A9: free (vg_replace_malloc.c:323)
==6675== by 0x8049B26: main
==6675== Address 0xbefff674 is on thread 1's stack


==6675== 1 bytes in 1 blocks are definitely lost in loss record 1 of 1
==6675== at 0x401C909: malloc (vg_replace_malloc.c:207)
==6675== by 0x8049B0F: main
14 Nov, 2009, David Haley wrote in the 6th comment:
Votes: 0
Lobotomy said:
The printf syntax is what I'm most comfortable working with so far when it comes to generating displays and the like, among several other things, so it's rather important.

I admit to not being sure why these issues are so related. If the only places you worry about dynamically allocated buffers are a few functions like 'bug', then surely you can just manage it yourself? As you say, the code is very simple. Elsewhere, you can use fixed-sized arrays for buffers, etc.

To be honest, I'm not sure why you have so many dynamically allocated strings that the need for this is so pressing. I understand that it's kind of annoying to deal with it, but, well… especially since you confirmed that cleanup is simply broken, I guess you don't have much of a choice anyhow.
14 Nov, 2009, Lobotomy wrote in the 7th comment:
Votes: 0
It doesn't make any sense for cleanup to not be working, though, is the thing. I need to figure out why, somehow. Since it works fine in C, it has to be something to do with C++; which may or may not be fixable. For instance, there's a function attribute for printf-style functions that extends compiler safety checks to them, but you have to tell it where in the argument list the format string is, and then where the first argument of the variable argument list is.

Example: void test( const char *fmt, … ) __attribute__( ( printf( 1, 2 ) ) );

Initially, when I added in that attribute to a class method, the compiler wouldn't accept it. After scouring the net for a while, I come to find out that the problem was C++ having the *this pointer as an implicit first argument; the fix was to move the values up by 1 to account for it, such that instead of (1,2) it had to be (2,3) and so on. After changing that, it now works just fine.

It's for that reason I'm confident that there's some way of resolving the situation with cleanup.

Edit: Typos.
14 Nov, 2009, Lobotomy wrote in the 8th comment:
Votes: 0
Sure enough, there was a fix, and it was something I should have noticed earlier. Basically, I forgot one of the most important points of how the cleanup attribute works: it passes a pointer-to-pointer of the variable to the designated function instead of a regular pointer. I hadn't had this issue with free() in the past since I was always using cleanup on customized free_something() functions for various things and, as far as I recall, never actually told cleanup to call free() directly. Anyhow, after creating a wrapper function that accepted a char** and then passed the regular pointer to free, the valgrind error went away.
14 Nov, 2009, Tyche wrote in the 9th comment:
Votes: 0
A couple of comments:
__attribute__ isn't portable, it's a GNU extension.
vasprintf() isn't Posix compatible and is a Linux extension which happens to behave differently on BSD.
You don't check the return code on vasprintf().
The intent of 'cleanup' is to destroy objects if you have multiple returns from a function (or are using the GNU C exceptions extension).
You don't have multiple returns, so why bother with all the extra typing when free() at the end of the function is just fine.
You are writing C++ code so if you throw an exception and wish to handle it you should probably put the free() in a try/catch.
14 Nov, 2009, Lobotomy wrote in the 10th comment:
Votes: 0
Tyche said:
__attribute__ isn't portable, it's a GNU extension.

I'm aware of that. Unless I come across any compelling reasons to the contrary, I'm not concerned with the compilation of my program in non-GNU compilers.

Tyche said:
vasprintf() isn't Posix compatible and is a Linux extension which happens to behave differently on BSD.

I'm aware of the thread-safety issues of vasprintf, and that isn't an issue for me at the time being since I won't be using threads. As for the BSD thing, I'm not too concerned with that right now, though I may look into adding a fix for it anyways. As for vasprintf itself, I can't think of a better way of doing what I need done. In my other project I used vsnprintf with variable sized arrays in a loop that increased in size as needed - I suppose I could do the same thing using arrays created with new[] but I'm concerned about introducing extra allocations/deallocations into the code (for speed/efficiency reasons). If there is a better way of implementing this, I'd be glad to hear it.

Tyche said:
You don't check the return code on vasprintf().

I was more concerned with figuring out why cleanup wasn't working at the time.

Tyche said:
The intent of 'cleanup' is to destroy objects if you have multiple returns from a function (or are using the GNU C exceptions extension).
You don't have multiple returns, so why bother with all the extra typing when free() at the end of the function is just fine.

I don't actually do the extra typing; I have a set of macros that I use for setting attributes. I figured if I left the macro in the example, I'd have to explain what it does anyways, so I put in the regular code instead. Also, I noted that forgetting about putting the required free() call in is a concern to me. With regards to the above code for getting the formatted string, that'll all itself end up in a macro to further simplify its use in other functions and methods having the printf-style syntax, which may have multiple exit points themselves.

Quote
You are writing C++ code so if you throw an exception and wish to handle it you should probably put the free() in a try/catch.

I'm not too interested in the whole try/catch business at this point.
14 Nov, 2009, David Haley wrote in the 11th comment:
Votes: 0
Lobotomy said:
I'm not too interested in the whole try/catch business at this point.

You should be, perhaps not as a highest priority but you should be aware of the issue because you don't always control where exceptions are thrown.
16 Nov, 2009, Tyche wrote in the 12th comment:
Votes: 0
Lobotomy said:
I'm aware of that. Unless I come across any compelling reasons to the contrary, I'm not concerned with the compilation of my program in non-GNU compilers.


Well I'll admit that I'm a portability nut. I sometimes run my code through 11 different compilers on 7 different operating systems. Of course it doesn't matter much for your personal code if you never plan to release. The main problem with the GNU extensions is the GCC group has been continually deprecating them which each new release of the compiler. Much of the Mudbytes repository is full of code that won't compile because of that "bit rot". And there is overhead with _attribute__ cleanup, moreso than just being explicit in ensuring your functions handle conditions correctly upon return.

Lobotomy said:
I'm aware of the thread-safety issues of vasprintf, and that isn't an issue for me at the time being since I won't be using threads. As for the BSD thing, I'm not too concerned with that right now, though I may look into adding a fix for it anyways. As for vasprintf itself, I can't think of a better way of doing what I need done. In my other project I used vsnprintf with variable sized arrays in a loop that increased in size as needed - I suppose I could do the same thing using arrays created with new[] but I'm concerned about introducing extra allocations/deallocations into the code (for speed/efficiency reasons). If there is a better way of implementing this, I'd be glad to hear it.


I'd use vsnprintf myself, pretty much exactly like the example on the Linux man pages. YMMV.

Lobotomy said:
I'm not too interested in the whole try/catch business at this point.


If you are using the STL, you should make it your business because many of the templates throw exceptions. Even basic_string can throw exceptions which you need to be prepared to catch.
16 Nov, 2009, Lobotomy wrote in the 13th comment:
Votes: 0
Tyche said:
I'd use vsnprintf myself, pretty much exactly like the example on the Linux man pages. YMMV.

Do you mean the example such as shown on the linux.die.net man page for vsnprintf? If so, I'm confused. I was under the impression that the point of using vsnprintf was that you would give it a local char array so that you could get a speed increase from not having to perform allocations/deallocations at the cost of having a fixed array size (except in C99 which allows variable length arrays). In the example, though, they're still allocating, freeing, and even reallocating. I mean, if allocation is going to be done anyways, why not just use vasprintf and (I would assume) only suffer a single allocation/deallocation? Is there something I'm missing with how vsnprintf and vasprintf work?

Tyche said:
If you are using the STL, you should make it your business because many of the templates throw exceptions. Even basic_string can throw exceptions which you need to be prepared to catch.

Even so, it's still probably just going to be something that I won't bother with until it actually becomes a problem.
17 Nov, 2009, Tyche wrote in the 14th comment:
Votes: 0
Lobotomy said:
Tyche said:
I'd use vsnprintf myself, pretty much exactly like the example on the Linux man pages. YMMV.

Do you mean the example such as shown on the linux.die.net man page for vsnprintf? If so, I'm confused. I was under the impression that the point of using vsnprintf was that you would give it a local char array so that you could get a speed increase from not having to perform allocations/deallocations at the cost of having a fixed array size (except in C99 which allows variable length arrays). In the example, though, they're still allocating, freeing, and even reallocating. I mean, if allocation is going to be done anyways, why not just use vasprintf and (I would assume) only suffer a single allocation/deallocation? Is there something I'm missing with how vsnprintf and vasprintf work?


Yes.
17 Nov, 2009, Lobotomy wrote in the 15th comment:
Votes: 0
Tyche said:
Lobotomy said:
Tyche said:
I'd use vsnprintf myself, pretty much exactly like the example on the Linux man pages. YMMV.

Do you mean the example such as shown on the linux.die.net man page for vsnprintf? If so, I'm confused. I was under the impression that the point of using vsnprintf was that you would give it a local char array so that you could get a speed increase from not having to perform allocations/deallocations at the cost of having a fixed array size (except in C99 which allows variable length arrays). In the example, though, they're still allocating, freeing, and even reallocating. I mean, if allocation is going to be done anyways, why not just use vasprintf and (I would assume) only suffer a single allocation/deallocation? Is there something I'm missing with how vsnprintf and vasprintf work?


Yes.

I see.
Random Picks
0.0/15