- Feature Articles
- CodeSOD
- Error'd
-
Forums
-
Other Articles
- Random Article
- 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
What's the WTF?
Admin
Granted, my C is a little rusty, but it looks to me like this guy made a change for the better. Still doesn't explain what's on DISC 2, though...
Admin
OK, for someone who hasn't written anything in C since he went to college in the 90s...
Why does initializing a variable on the stack, as opposed to in-code, result in a much larger compiled program? I failed at Google.
Admin
Yeah, umm, isn't example 1 just explicitly doing what the compiler will generate in example 2?
Now if one of the examples used malloc and free, sure...
Admin
Am I understanding this correctly: The WTF is that he's copying "data data data" into data_string[15] instead of the entire array?
Admin
I must be missing something. Winston's changes, though extensive and probably unnecessary, should be harmless. Of course, I'm not sure what "initiated on the stack" means. Maybe that means "initialized on the stack"? But in the example given, both variables are allocated on the stack. The only other way they could be allocated is on the heap, such as:
Changing that sort of allocation to be on the stack could cause problems, though it's generally a better idea to allocate on the stack than on the heap (faster and has automatic cleanup).
So is the WTF the fact that there was no oversight of what developers were doing?
Admin
It's good to see that not everything is a WTF here, but that things every once in a while work out for the better.
Too bad people had to get fired / pre-emptively promoted over it, though...
Admin
Guys, you're obciously missing the WTF. He changed all the variables from being on the stack to... Being on the stack! Terrible!
(Actually, my pet theory is that the WTF is the fact he touched every source file in the repository...)
Captcha: luctus (luctusly, everything still worked?)
Admin
Yeah, I don't get it either. Where did the second CD come from?
Admin
This'd be a good time for some smug bastard to come in and tell everyone what's really wrong and horrible about this, and why the second CD was needed.
For once, your opinions and knowledge are wanted.
Admin
I'll join the confusion. Not having unitialized variables is usually considered a good thing not a WTF.
Admin
but who was phone?
Admin
Ok, I dont know the terminology, but that's just not possible. The stack is freed implicitly.
Admin
The WTF is that the programmer partially rewrote a giant legacy application because he didn't like the coding conventions.
He sounds like a smart guy, but he just can't function in a team environment. He should probably go be a one-man shop where he doesn't have to maintain code other than his own.
Admin
Yeah, I did this when I was younger. Came into a shop with about 10 devs, saw their source was damn near unreadable, and ran a formatter on the whole thing. Of course the old timers hit the roof. "Now our diffs don't work and we can't tell what changed!"
I showed them "diff -w".
For those who are still stuck in a GUI, that means compare two files but disregard changes in whitespace only. In other words, show me what really changed, aside from formatting.
Them: Oh.
Years of putting up with unreadable code. All because none of them ever read the diff man page.
Admin
My best guess is that he did not remove the existing initial assignments: (code) int x; ... x = 2; (/code) became; (code) int x = 2; ... x = 2; (/code)
Admin
I'll bet Winston is also an OS/2 hatter! Good ridance to him.
OT: Is the captcha really random or somehow derived from the text? I've just got 'conventio' and the article talks about conventions...
Admin
The example is not
Ok, why didn't he just use the convention
The orginal didn't have any space left in the stack allocated array (length 14 + 1 null char) to put anything else in it, so.....???
One (char*) should cost less than a char[15]. Four bytes to allocate a pointer on a 32 bit system vs 15 (and potientially 16 bytes for word boundaries) for the array itself.
Unless of course data[15] was supposed to change as the function progressed through it's code... That was probably the reason, but it sure would have been nice if that code demonstrated it.
Admin
My thought is that one of the two CDs is the original code, and given the interview data... not even surprised.
Admin
Maybe the WTF is that when he was supposed to be tracking down a memory leak he was instead worrying about how stack variables were initialized?
Admin
Once upon a time I worked at a company that had just hired a summer intern. The only problem was everyone was too busy to deal with this poor guy. Someone (read: the president/CTO) decided it would be a good idea to put this intern in change of internationalizing the entire application.
For those of you that have never dealt with i18n before, it basically involves removing all the hard-coded strings, putting them in a properties file called a resource bundle, then adding a lookup in the source code. On the surface it does seem like the perfect job for an intern. However, they forgot to assign someone to look over him.
The intern was actually pretty smart, and wrote some tools and scripts to help him. He would run these tools on the source code, one package at a time, then check in his changes. He didn't understand how the application worked, or even how to start it, so of course he didn't test to see if his changes actually worked.
3 months later, it was time to say goodbye to the intern. We congratulated him on a job well done and wished him luck in his adventures at CMU. Now, we were working on this application in parallel with him, so we would pick up most of his bugs. Usually they came in the form of certain strings being replaced with '?????' because he either forgot to add the entry to the resource bundle or messed up the key. These were easy to track down and fix.
Not too far after, we ran into a show stopping bug. The application would start when launched through the IDE but not when we tried to run it through the test environment. Most developers just said 'fuck it' and kept plugging away through the IDEs. I, on the other hand, was determined to track down the source of the bug.
It turns out the intern's i18n tools had a bug in it where they would somehow delete certain lines of code. 99% of the time this would result in a compilation error and he would just go back and fix the line he fucked up. However, in that rare 1% of cases, he would delete an important line but the code would still compile. One of these cases was the root cause of the bug I was tracking down.
In total, the intern touched more than 2000 source files, probably a few hundred thousand lines of code in total. There was no way we could guarantee the integrity of any of the code anymore. The powers-that-be demanded a project-wide rollback of the code, reverting 3 months worth of everyone's work. Good thing we had Visual Source Safe groan
Admin
The WTF is that when the local variables are uninitialized, they can be placed in the BSS, which doesn't require "real" allocation, at least in the ELF format used by most Unix-like platforms.
There's a good discussion of it here:
http://stackoverflow.com/questions/610682/bss-section-in-elf-file
Admin
Someone kept stealing my lunch at work, until finally I set up a camera to catch the person in the act. Turns out the fridge was slowly leaking coolant! . . . what?
Admin
Reminds me of a story. A couple of months ago I went on vacation for a week. While I was gone, I hire a neighbor kid to wash my car.
When I got back I suddenly needed TWO GARAGES! Can you believe it? Two garages for one car!
Turns out not only did he wash my car, he also changed the oil.
Admin
P.S. I would say that the "Real WTF" is coming back from a two week vacation, and automatically assuming that the code has been perfected to the point that the first thing you do is burn disks for the customer.
I would say that, but I assume it's just part of the bizarre paraphrasing that goes into these articles.
Admin
Stack variables are not in .bss
Admin
I too am struggling to see the WTF although I don't program in C. Doesn't look like there is a coding mistake is there?
I suppose it was a bit of time wasting although most people prefer variables initialized straight away, cuts down on bugs.
As for the second CD i'm confused maybe they didn't mean the latest installer version and they were actually downloading the full repository. Meaning every change checked in of every file not just checking out the latest one.
Admin
I think the little code sample was somewhat irrelevant really, the WTF is perfectly obvious: he changed every single code file in the system just to satisfy his own pedantic vision of "good code". Coders like this are a serious burden to any IT shop, partly because they waste so much time "fixing" other people's code. They also tend to introduce a lot of bugs because of it ("it doesn't need a full test cycle, it's functionally equivalent to the old code"). The worst thing is, this coder is inevitably borne of good coding skills. He has a decent knowledge of the relevant principles and because of this he thinks that his way of doing things is obviously the right way.
I can live without a solid process assuming the team is small enough and experienced enough but I always mandate a basic level of coding standards for this very reason. Then, when we hire the likes of Winston we just drop the coding standards in their lap and inform them that from now on, all their code looks like the standards. They hate it but it's contractually enforced so they just have to live with it or leave. Of course, I write the coding standards so maybe I'm the Winston after all...?
Admin
Something happened. Then another thing happened. Meanwhile I'm awesome and find nothing interesting because I have a brain the size of a planet. Something else happened. /yawn
Admin
TRWTF here is that instead of finding the memory leak by using something like, heaven forbid, valgrind, the guy changed every variable assignment in every source file in hopes that he would accidentally fix the leak. Also, if a bug were introduced, reverting the changes would lose everything that this guy did (I am assuming).
http://youthunion.zapto.org/
Admin
Admin
(Maybe you weren't suggesting that; your post wasn't clear.)
Admin
Winston's changes should have had no detrimental effects, unless the expressions that were used to initialize variables had side-effects or were otherwise order-dependent.
The WTF was that he did it when the team wanted to make a release, presumably without getting buy-in from anyone else, and there's no mention of a suite of regression tests. That time could have been much better spent fixing or searching for actual bugs, instead of stylistic concerns. Or maybe he could have written a suite of regression tests, or a continuous build process.
Admin
Best guess based on some previous comments:
the variables were originally allocated in .bss, which is zeroed at runtime and takes up no space in the executable image on disk. Their initialization values were constants in the program code, and were mapped to a constant table somewhere in the executable image. If there were 500 variables being initialized to "abcdef", they would all point to one entry in the constant table.
the Winstonized variables are allocated in .data, which does take up space in the executable image on disk. If there were 500 variables being initialized to "abcdef", they suddenly take up 500 times as much space in the executable image. Multiply this by a large and redundant codebase, and you need a second CD to install your app because the EXE is 10x larger than it used to be.
Just a guess... this one left a little too much as an exercise to the reader, imho.
Admin
Do you remember stories like this one? http://thedailywtf.com/Articles/A-Common-C-Dilemma.aspx
Those were great days!
Admin
The WTF is that the guy was assigned to hunt down and fix a relatively small, but important issue and touched every file in source control. And at that, his changed did next to nothing except make the code possibly easier to read. That means next time there's a bug in some random file that no one modified recently, Winston's a suspect, even though he really didn't touch much.
When you start a new job, you have to get a feel for the process and see how open and willing they are to change. After a few months though, you suck it up and deal with the fact that fixing WTFs is going to be a gradual process and not something you can fix with a few omnibus commits logged as "General code cleanup"
Admin
I'm pretty sure this insightful analysis is blown to smithereens by the fact that stack variables don't go in .bss anyway. Oh well, partial credit?
Admin
Ah! I've got it.
The installation CD included the entire source code repository, the SCM tools, and a C compiler. What's his face there grew the repo just enough to push it over the edge of a single CD's storage!
Admin
There is this nifty revert concept in versioning systems that allows you to undo someone's changes that screw things up in whatever inexplicable way you think they are screwed up. It seems like Winston's week of massive changes could be undone with a few keystrokes - problem fixed, give that guy a warning, move on.
Oh, and write to the Daily WTF so that you can get everyone scratching their heads about what the problem was.
Admin
Not for automatic stack variables, no; that only applies to static and global variables. A variable on the stack just mean the stack pointer gets incremented by four extra bytes (or whatever the size of the variable is). If it's uninitialized, that space is just left as-is.
Admin
It looks like he made everything more efficient.
Admin
Touching every source file like this should result in an immediate termination.
Admin
I am an amateur at best with C, but I don't see any wtf with the code itself.
Eh I don't know... lol must be missing something.
Admin
Admin
"Stuck in a GUI"?? Yeah, those newfangled GUIs don't have anything like 'ignore whitespace' during diffs.
Admin
Admin
In code that has tricky memory leaks, uninitialized variables are a common cause. It was noted that the problem was hard to track down, so a shotgun approach to tracking the problem down is not that inappropriate.
He probably thought he was pr-activly dealing with an easy to fix class of code error and, unless there were side effects / order considerations the code is better as a result.
The WTF here is, why is this a WTF? They had version control? Branch. Revert. Reapply the specific bug fix. Get the immediate build out. Then re-apply. This is not a code style difference along the lines of how you arrange your bracers or whitespace - uninitialized variables are actually bad.
Admin
And this, friends, is what code reviews are for.
Generally speaking, any developer in my group must at least run the changes past another developer as a sanity check, before they can deliver their changes.
Admin
Uninitialized variables are bad, mmmkay?
Admin
Stack variables don't cause memory leaks. Crashes due to buffer over/underflow, yes, but that's not a "memory leak".