- Feature Articles
- CodeSOD
- Error'd
- Forums
-
Other Articles
- Random Article
- Other Series
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
There's a return above the free() statements. It's dead code.
Admin
Most likely placed there after a 6-hour debugging session trying to figure out why the tree is disappearing 5 minutes into the application execution...
Wow.
Admin
Who node why that became necessary....
Admin
Deleting pointers, ok. Strage place to (try to) do that (better place would be the destructor), but let us humour the guy.
But freeing native integers? What language is that?
Admin
I think they forgot
;)
Admin
C, it's been a while... but I think you can't even know if the variables are on the stack, because they are passed by pointer. So the code (which luckily never runs anyway) frees some variables of the calling function (which is of course idiotic but not necessarily wrong, i.e. the objects were only created for passing to the function), but also some random memory location (given in n) which would most probably cause an error from the memory manager at best.
This is only my quick interpretation and I haven't coded in C for ages, so I could well be wrong.
Admin
Admin
I'm not a C-expert but what would 'free ( n )' do? It's an int. It would (try to) free some data on address n? (If there weren't the preceding returns.)
Caotcha: burned - I was (many times)
Admin
Er... wanted to write CaPtcha...
Captcha: tacos (burned by tacos - makes sense...)
Admin
Cargo Cult programming at its finest! Way back in the mists of time, probably someone who knew what he/she was doing told one of the clueless developers about memory leaks and fixed one by putting in a free().
"Always free() whatever you allocate," she said, then left for her next contract.
Ever since which, clueless developers have handed the wisdom down. With each generation the sense of ritual and awe increases. The code works perfectly too, assuming you don't do something dumb like place the return after the frees. And it prevents memory leaks! Are there any memory leaks in the above code? Nooooo.
I have to take issue with the comment 'stack variables want to be free'. It's the heap memory pointed to by those stack variables that's being freed -- whether they're pointers or not.
Admin
good point. He probably meant free( &n ); There. Much better.
Admin
When I saw the dead code below return 0;, I couldn't do anything but to sigh.
Admin
Would this even work? n is not a pointer, stack or heap it doesn't matter. If the compiler was at all decent it would smack you for trying call free() with a non-pointer instead of "helping" you and assuming you wanted to use the int as a memory address.
CAPTCHA: Muhahaha, indeed.
Admin
Definitely looks like C code. I'm seeing:
Admin
the n was passed in by value as an integer.
in C, there is no way to differentiate between ints and pointers. the result of free(n) depends on the situation. If during application runtime - those guys were so unlucky as to have actually allocated something at the memory address n, that address space would be freed up. If that was not so, there would be an error code returned except these guys were never checking the error codes any way so that program could continue as normal. i.e. they will have a random crash sometime and will probably never be able to find the problem. As a side note, the compiler should give a very big warning of there being no casts to turn an integer to a pointer... (I know gcc gives the warning. don't know about what they are using but if it is anything halfway decent, it should give a very big warning.)
Admin
freeing pieces of the stack... way to go...
Admin
Admin
Assuming that this is C, and notwithstanding the return statement, free() is used to give back a section of the heap which was manually allocated with a standard C function like malloc(). Heap memory manually allocated in C is NOT given back automatically, and there is no garbage collector, so a common source of crashes is running out of memory due to a 'leak'. As a corollary, a common point beaten into the heads of C programmers is to be sure to release all allocated memory -- when you no longer need it. Hence the programming lore. So if the pointers passed in were originally obtained by a call to a heap allocation function, then the free() call would actually free up the memory. The middle of a traversal function doesn't look like a good place to do that, so that's a WTF in any case: who knows who else is using references to that chunk of memory. If on the other hand, the parameters are pointers to local variables or stack variables of some calling function, then the use of free(), were it actually to run, would result in either nothing happening (if the free function was implemented in such a way that it ignored addresses not within the heap) or some kind of exception. So that's another WTF. And if the programmer was actually thinking to free up the stack, as opposed to some heap, then he/she is so utterly ignorant of computer memory organisation as to constitute a WFT**2.
Admin
The biggest, and hardest to debug, problem in this code is simply that based on what we've been shown traverseGroup doesn't allocate the memory it frees. (yes, there's a return before that but as someone else pointed out, it was probably placed there during debugging.)
A cardinal rule of C coding is never free memory you (the function) don't allocate. (Yeah, free'ing the integer will also cause problems.)
C++ has destructors but they don't this way. If you have something like this:
class SomeClass { /* bunch of stuff */ };
SomeClass *foo = new SomeClass ();
You still must free foo with the delete keyword. The class's destructor will be called to give SomeClass a chance to free any memory it allocates, but the memory allocated by new must be explicitly freed or it's allocated until the program exits. The memory is NOT freed when the end of the block is reached.
If, however, you have something like this:
void func () { SomeClass foo; foo.blah (); return; }
Then the memory is freed automatically because SomeClass is on the stack, not on the heap. Notice that I didn't use new or malloc() in that function. Therefore, I don't need to use free() or delete.
Admin
A long time ago, I worked with this guy who thought it was cute to do something like this:
Man, I hated that guy.
Admin
I should have clarified. They're OK syntactically and if the memory is dynamically allocated aren't going to shit the bed. Granted, some other code may take great offense to the data being freed up. It's valid, just not necessarily correct in-context.
Admin
What happens when this sample code runs?
int main() { Node myWilly; Node myDadsWilly; Nody childrensWillies[] = Node[2];
}
Admin
Captcha: smile (it's a nuclear hand grenade)
Admin
The line free(n) would work fine.
If the numeric value of n happened to be the address of an allocated memory block. Otherwise "The results are undefined, but probably catastrophic."
In the Good Old Days, integers were used to hold address. Therefore, the compiler has to allow integers to be used as pointers.
Admin
Admin
Admin
I assume you want to do this instead, as syntactically what you've got above isn't quite kosher:
int main() { Node myWilly; Node myDadsWilly; Node childrensWillies[2];
}
Assuming you have a very lax compiler and you actually get a binary out of it, you'll probably crash in the section that assigns status. Assuming you trim that out, you'll return. Assuming you trim that out, it's up to the free implementation. You're trying to free memory on the stack, so chances are free is going to ignore it or do A Bad Thing (tm). Maybe it'll shit the bed now, maybe it'll shit the bed later, maybe it'll continue along blissfully unaware.
Admin
Now let's not go flying around all willy nilly ok.
Admin
linux = free(as_in_beer);
g,d&r
Admin
Incredibly stupid, but...
int payload;
/* ... some other stupid code ... */
search_string = (int) malloc(....);
memcpy(search_string,"hello moto",sizeof("hello moto"));
traverseGroup(&myWilly, &myDadsWilly, &childrensWillies, search_string);
...this makes what they were doing STUPID but valid. (before the return 0, fixed it)
PS: I have seen int and void* uses for some absolutely stupid reasons.
Admin
Why would this guy pass as an argument the parent and the children of the node? Shouldn
t the data structure be organized in the way that you can actually track the node
s parent and his children having the node. Bad data structuring too.Admin
It's easy. As those pointers are arguments he just saw a comment saying something in the lines "don't forget to free those pointers after returning". And that he did.
Admin
some guy : "There ain't no such thing as a free lunch"
Mac: free(lunch); "Now there is."
Admin
Well, that's OK, as long as I get to...
free myWilly
You know, like in the film ;-)
Admin
[/quote] Take it to the extreme:
[/quote]Admin
free(dom) anyone?
Captcha: pinball (at least not that stupid onomatopoedia again!)
Admin
Yeah, tell that to Microsoft in their MFC libraries frustrated sigh
Admin
Syntax highlighting and comment folding are your friends...
I worked with a new graduate once who came in to our 800K lines+ C codebase and spent his first week figuring out a find/sed script to strip out all the comments, because they 'got in the way'.
We had a little talk...
Admin
I think I may know why, I've written similar code too.
They key thing to note is that it's dead code, never executed. Imagine you've got warning levels up so high that it issues warnings for unused local variables and unused function arguments. Now imagine that you've got "treat warnings as errors" turned on. As you're developing the code you may or may not use all of the function args, sometimes you may never use all the function args but they are there anyway because of architectural reasons. So you write dead code after the final return, something, anything, that does something to each and every function argument. Compiler is happy, optimizer then strips it all away again, developer is happy.
Of course, that doesn't explain the wording of the comment or the use of free()! Whenever I write dead code like this I just assign each arg to itself:
peer = peer; parent = parent; children = children; n = n;
Admin
Admin
The line free(n) would work fine.
No, it wouldn't. C is strongly-typed. That's not to say you can't cram an integer where a pointer is asked for--this is C we're talking about, after all--but you have to at least pretend what you're doing is legit and add a (void *) cast.
(Well, unless there's no #include <stdlib.h>, meaning free() has no prototype. But any compiler from the last 20 years should throw up a warning or two about that.)
Admin
Nope, it works. C is not strongly typed in this case. You can pass an integer where a function expects a void *. The resulting error in gcc is:
"warning: passing argument 1 of ‘free’ makes pointer from integer without a cast"
Admin
Obviosuly I meant "the resulting warning". :)
Admin
<quote>"Failed to traverse successfully"</quote>
Classic! I vote for this to be added to the same list as the
<digression> Can someone take a snapshot of this code, print it out, place it on a table, take a picture of it, get the pictures developed, scan the picture in, and post it so we can...... </digression>TRUE,FALSE,FILE_NOT_FOUND.
Admin
Learn C, you hippies.
(Those of you who know C might as well go out for a smoke whole the rest of the class catch up.)
Admin
Not that this is incredibly minor compared to the rest of it, but I like that it returns 1 when it errors out. Very intuitive.
Admin
Learn C, you hippies.
(Those few who know C might as well go out for a smoke while the rest catch up.)
Admin
free(willy);
Admin
Well, this is a matter of taste and mostly lots of confusion. The main function is supposed to return 0 on success while generally other functions return 0 on failure. All functions having several error codes (like I think pthread_create) returns 0 on success and whatever code on failure.
What C would have needed is more boolean values than two.
Admin
We a project that was compiled with MS-C version <= 7.00 and Borland C (Turbo-C) <= 2.00 and both emitted warnings for unused parameters. Ms-C could be satisfied with your code (a simple perr; was even suffisant), but Borland was another beast, it knew that it wasn't used and warned anyway. I really had to do complicated things to get him quiet on this. At the end I didn't bother and let the warnings.