The Daily WTF: Curious Perversions in Information Technology

2009-07-30 Reply Admin

Thanks for the clear example. I guess some surprising errors can come up when people aren't paying attention to the data types.

I say of course sizeof() will give different results on a pointer and an array, but that doesn't matter if you know it's a pointer if it's an array and you write correct code. Also, in your example (Anonymoose), the person should have never used '&a' in the first place -- 'a' would have been correct (as you stated). I was thinking from my point of view rather than thinking about how bad someone could mess something up if they didn't know what was up.

Just my lack of experience in working on larger projects...

2009-07-30 Reply Admin

I just went and checked, and in my compiler (gcc 4.3), char foo[15] = "data data data" will result in a sequence of move-immediate instructions loading the string one dword at a time into its slot on the stack. The sprintf version emits a constant. If that same string constant is used a lot in other files or even just in other functions, then Winston's change will prevent any potential merging of constants. Which can be a lot of saved space, depending on the specific application in question.

2009-07-30 Reply Admin

Person:
TAlso, in your example (Anonymoose), the person should have never used '&a' in the first place -- 'a' would have been correct (as you stated).

I agree, but it happens more often then it should. Sometimes junior coders will use a cast to get rid of the warning.

char a[] = "abcdefgh";
printf("%s", (char *)&a); /* facepalm */

EvanED · 2009-07-30 Reply Admin

Stig:
...highlights how char* and char[] are essentially no different 'under the hood'.

Except for the part where the address is stored explicitly for b and c but not for a. Did you not notice the mov instructions when calling puts(b) and puts(c) but the lea instruction to call puts(a) (in order to compute the address of a, which it needs to do since it isn't stored anywhere), or are you ignoring that and saying it gets wrapped up in the "essentially"?

Because at least I would say that's a pretty damn big difference in how they are implemented. In source code, that's the difference between *x and x.

For further illustration, print a[2], b[2], and c[2]. I bet you that it will get a[2] from a memory load relative to rbp, but b[2] and c[2] from a memory load relative to another register (that it just finished loading b and c into).

2009-07-30 Reply Admin

pete:
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.

How does initialising variables straight away cut down on bugs (especially when they are initialised with rubbish)?

Oh, that's right, people suppress compiler warnings, which (with some/many compilers) will give you warnings like 'variable my be used unitialised'...

I have worked on many projects which insist on declaring variables at the beginning of a method, and haveing a clear separation between declarations and functional code - some of these projects have required that variables are declared NULL or 0 (or not at all) and initialised in the code section.

I would say the WTF is (or was meant to be) one of: a) A major effort has gone into rewriting large amounts of code for no reason (and no benefit) b) An attempt at fixing a memory leak has simply changed variables on the stack - rather than variables allocated on the heap (which are the ones that may leak) when they aren't cleaned up properly c) 2 CD's - I want to hear more about how they even fit into the story d) The 'expert' who is such an expert that he absolutely has to modify working code because it doesn't fit in with the way he learnt to code (ie, much like (a) a lot of effort has gone into doing nothing simply because this dev obviously thinks he is an authority on the correct way to code things). e) probably more similar.....

subtle maybe, but almost a WTF - more so than some that have appeared here recently

2009-07-30 Reply Admin

Anonymoose:
Of course, the correct code would've been:
char *a = "abcdefgh";
printf("%s", a);

Well, declaring the array as const char * would be better practice. Interestingly, if I'm reading my C99 draft spec right, the above is nevertheless legal: it appears that string literals are arrays of char (not const char), although attempts to modify the array result in undefined behaviour.

EvanED · 2009-07-30 Reply Admin

anonymouse:
I just went and checked, and in my compiler (gcc 4.3), char foo[15] = "data data data" will result in a sequence of move-immediate instructions loading the string one dword at a time into its slot on the stack. The sprintf version emits a constant. If that same string constant is used a lot in other files or even just in other functions, then Winston's change will prevent any potential merging of constants. Which can be a lot of saved space, depending on the specific application in question.

gcc (& other compilers; I've also looked at this issue in MSVC a while back in the context of structs) will switch between a couple different idioms. For small structs and arrays, it will do an element-by-element move, but for larger ones will do a copy in a loop (which would let it merge constants).

For instance, with the example before with an initialization of a tremendously long string (or structs), gcc emits code that calls memcopy (pulling from a constant in the rodata segment) for the array version.

RHuckster · 2009-07-30 Reply Admin

The only WTF I can find here is he changed thousands of lines of code just before a release, which is a huge no-no, no matter how trivial the changes to each of the thousands of lines of code are. There's a lot of potential for disaster from human mistakes, especially assuming he did this after it was already QA'd, which I assume it was, considering Christopher was about to burn the application to disk.

Of course, if it wasn't QA'd, then THAT's TRWTF.

2009-07-30 Reply Admin

Because at least I would say that's a pretty damn big difference in how they are implemented. In source code, that's the difference between *x and x.

The specific code the compiler emits is not really that important in this case...the important point this assembler highlights is that when you refer to the label "a", it requires the address of the text - ie. it's being ultimately handled just like a pointer.

EvanED · 2009-07-30 Reply Admin

Demon:
How does initialising variables straight away cut down on bugs (especially when they are initialised with rubbish)?
Oh, that's right, people suppress compiler warnings, which (with some/many compilers) will give you warnings like 'variable my be used unitialised'...

(1) There are plenty of times when you have something reasonable to initialize with. (I agree that uninitialized is better than "initialized with crap", but I strongly feel that "initialized with something good" is better than uninitialized. I would far far rather see "int x = 0;" than "int x; .... x = 0;" with ... being straight line code.)

(2) It's easy enough to fool the compiler into either omitting such warnings or giving spurious ones.

(3) GCC, an (perhaps unfortunately given how many gripes I have about it) widely-used compiler will not emit said warning without optimization flags specified, so if you're working on developing you won't see them unless you explicitly build a release version. (This is a good idea anyway. That doesn't mean people actually do it.)

10408. ~/temp % gcc -c -Wall -Wextra arr_overload.c 
10409. ~/temp % gcc -c -Wall -Wextra -O2 arr_overload.c
arr_overload.c: In function ‘foo’:
arr_overload.c:5: warning: ‘x’ is used uninitialized in this function

2009-07-30 Reply Admin

entropy:
Well, declaring the array as const char * would be better practice. Interestingly, if I'm reading my C99 draft spec right, the above is nevertheless legal: it appears that string literals are arrays of char (not const char), although attempts to modify the array result in undefined behaviour.

Good point. It was just example code (and to be honest, the kind of devs who don't bother to distinguish between a and &a don't really care about const-correctness). I was trying to highlight the danger of assuming char[] and char * are always interchangeable. The real situation was more like this:

Original code:

void foo(char *str);
// should have been const char *str, but again
// many devs don't care about const correctness
void bar()
{
  char some_str[] = "blah";
  foo(&some_str); // should've been foo(some_str),
                  // but code still works
}

New, broken, code:

void foo(char *str);
void bar(char *some_str)
{
  foo(&some_str); // should've been foo(some_str),
                  // and now code crashes/behaves incorrectly
}

2009-07-30 Reply Admin

Ok, maybe the WTF is that he actually tried to take the initiative and fix the code, rather than to live with its less than perfect syntax.

If we all set about trying to fix our code, you never know what horrible thing might happen next. It might actually work, or it might just not crash enough and then who knows, maybe we'd have to layoff all of those consultants and support people.

Huge chunks of the industry could get ruined. Maybe, even if things got bad enough, we'd have to shut down our blogs complaining about how bad code is.

The consequences of just allowing people to consistently go off and refactor anything at all are seriously dire. Thank goodness he didn't figure out how to use a const to point at it in the data section instead of just duplicating it on the stack, that could have been even more devastating!

2009-07-30 Reply Admin

Stig:
The simple code:<snipped for length: see directly above> ...highlights how char* and char[] are essentially no different 'under the hood'. Yes, semantically they mean different things (as I said in my original post). Here, they are both pointers, albeit local, where the array declaration resolves to an implicit pointer. This is why the "[n]" syntax is essentially a char* dereference plus an offset. This is why you can happily interchange char* and char[] without problems.
Interestingly, gcc now deprecates code like:

char* blah = "blah";

...and would prefer that you use:

char blah[] = "blah";

...and does so to enforce the semantics of the language, not because of the compiler output.

Some of us use several compilers, not just gcc. However, just to keep you happy, here's an example on msys using gcc 3.4.2:

#include "stdio.h"
void foo(char** bar) {
static int i = 0;
sprintf (*bar, "World%d", i++);
}
void hello_array(void) {
char hello[] = "Hellox";
char* ph = hello;
char** pph = &ph;
foo(pph);
printf ("%s\n", *pph);
}
void hello_ptr(void) {
char* phello = "Hellox";
foo (&phello);
printf ("%s\n", phello);
}
int main (int argc, char** argv)
{
hello_array();
hello_ptr();
};

Somehow I sense a difference here.

I know you wouldn't make this mistake. I know I wouldn't make this mistake ... well, not more than one or two times, and it only cost a coupla weeks' development effort and nearly losing a half million project ...

But. Would you really trust anybody else not to do it?

If your balls were in the vice?

Brave man, Stig.

2009-07-30 Reply Admin

Chris Becke:
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.

Having a Versioning Control doesn't mean much unless you know how to use it. I have seen many many projects where every version continues on the trunk. For small projects with a single dev this wouldn't be a problem, but when there are 5 or 10 devs implementing different fixes on the main branch (in fact only branch - and not really a branch, since we never branched from anywhere) that when something goes wrong it can be difficult to implement which 'fix' actually caused the problem - especially when they are rolled into the production environemnt in quik succession

EvanED · 2009-07-30 Reply Admin

entropy:
Well, declaring the array as const char * would be better practice. Interestingly, if I'm reading my C99 draft spec right, the above is nevertheless legal: it appears that string literals are arrays of char (not const char), although attempts to modify the array result in undefined behaviour.

That's not best practice if you are coding in portable C89, which doesn't have 'const'.

(This is why C99 has "blah" be char*, and why C++ permits that conversion in restricted cases.)

Stig:
Because at least I would say that's a pretty damn big difference in how they are implemented. In source code, that's the difference between *x and x.
The specific code the compiler emits is not really that important in this case...the important point this assembler highlights is that when you refer to the label "a", it requires the address of the text - ie. it's being ultimately handled just like a pointer.

All that illustrates that arrays decay into pointers when you pass them as parameters. Pointers and arrays are still very different to the implementation.

Or do you think that the difference between dereferencing or not is essentially not there, or that the difference between accessing an element with an extra indirection instead of relative to the frame pointer is essentially not there?

I would say that your example no more shows that pointers and arrays are essentially equivalent (because when you pass an array as a parameter it gets converted to a pointer) then an example where you pass an 'int' to a function that takes a 'double' shows that ints and doubles are essentially equivalent (because when you pass that int as a parameter it gets converted to a double).

2009-07-30 Reply Admin

Anon:
Anon:
True enough, but that pattern requires a pointer and dynamic memory allocation which is apparently outside the scope of Winston's chances.
However, the argument could be made that Winston "changed every single variable to be initialized on the stack" meaning that in the original code, there was dynamic memory allocation and post newbie, there was none.

In that case, the example is woefully inadequate. However, that would account for an extreme change in the size of the executable.

I forgot to quote this:
This kind of pattern is not unusual
void func(char const* param){
bool fexternal;

if(!param){

  param = malloc();

  fexternal=false;

}

DoSomething();

if(!fexternal)

  free(param);
}

The pattern may not be unusual but it's stupid.... Doesn't bool initialise to false? We will always free If in doubt, initialise your bool first....

We could complicate the code by not using a bool

void func(char const* param)
{
  char *local_param=NULL;

  if(!param)
  {
    param = malloc();
    local_param=param;
  }

  DoSomething();

  /* I think free does nothing with NULL, but to be safe
     We'll include the 'if */
  if(local_param) free(local_param);
}

the added advantage is that we can't accidently free param here - if we don't initialise the local variable to NULL we still won't free param (although we might free something we never malloced which is always an interesting thing to do)

Or not. Argue away!

2009-07-30 Reply Admin

Tom:
That only applies for static locals, the example from the story wasn't static. Non-static locals will always be placed on the stack and never in the bss, if they were placed in the bss, it would break recursion.

I see static locals at the pub all the time - some animated locals occasionally too (especially near closing time)

2009-07-30 Reply Admin

EvanED:
entropy:
Well, declaring the array as const char * would be better practice. Interestingly, if I'm reading my C99 draft spec right, the above is nevertheless legal: it appears that string literals are arrays of char (not const char), although attempts to modify the array result in undefined behaviour.
That's not best practice if you are coding in portable C89, which doesn't have 'const'.
(This is why C99 has "blah" be char*, and why C++ permits that conversion in restricted cases.)

I'll grant you that this is a draft spec, not the final one (which costs money), but according to http://flash-gordon.me.uk/ansi.c.txt, C89 has the const qualifier. It's possible that this was done for compatibility with K&R C.

It is legal to convert const char * to char * in both C89 and C99. The restriction is that "If an attempt is made to modify an object defined with a const-qualified type through use of an lvalue with non-const-qualified type, the behavior is undefined."

2009-07-30 Reply Admin

Zombywuf:
<snip>
I have never seen anyone strcpy into on stack arrays in that manner. The correct way looks more like char string[] = "data data data"; <snip>

I see it all the time. Have you never needed to change arrays on the stack other than at initialization??

2009-07-30 Reply Admin

Xepol:
For those who have not followed the chain, the cause of the bloat is:

The original strcpy(s,"static text") means that "static text" occupies only as much memory as the string plus a null.

The new char s[xx]="static text" requires that "static text" be stored in a buffer of xx characters, null padded - in otherwords, a heck of a lot of nulls padded into the executable to no advantage.

All the bloat was useless null padding.

Ok, maybe I missed something, but in both examples static data was copied into an array of size 15 allocated on the stack. The strcpy in #1 isn't going to dynamically resize the stack, so both examples use the exact same bss string and the same stack data-struct. Yes?

The only way the size could have doubled is if the original code pointed directly to the string in the static data (bss) section.

If he didn't spec the size of the array, the usage would be less as in:

char data_string[] = "data data data";

but:

const char *data_string = "data data data";

is still much better, I think ...

EvanED · 2009-07-30 Reply Admin

entropy:
I'll grant you that this is a draft spec, not the final one (which costs money), but according to http://flash-gordon.me.uk/ansi.c.txt, C89 has the const qualifier. It's possible that this was done for compatibility with K&R C.

Looks like I'm wrong on that one.

2009-07-30 Reply Admin

ollo:
In fact, char[] is aequivalent to char*. Each one is a pointer to an 'array' of chars, thus a pointer to the first byte of a number of bytes representing a string. The two different representations are only syntactic sugar.
AFAIR at least. :)

There's a wonderful (albeit old) book that some of you should read called "Expert C Programming - Deep C Secrets" that has a chapter dedicated to [] vs * (entitled "The Shocking Truth: C Arrays and Pointers are NOT the Same!").

Just because we have all become used to treating them the same, doesn't actually mean that they are (although in most cases they are interchangeable)

2009-07-30 Reply Admin

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*

Good afternoon, Mr. OutlawProgrammer. I'd like you to meet Mr. Diff.

2009-07-30 Reply Admin

Stig:
EvanED:

ollo:
In fact, char[] is aequivalent to char*. Each one is a pointer to an 'array' of chars, ...

No, it's not. Stop spreading this lie.

The language semantics are technically different, but under the hood, they are identical. It is not a lie.

char* == char[] char** == *char[]

FAIL! Just because under the hood an attempt is made to create the appearance that they are treated the same does not mean that they are the same. In fact, the 'under the hood' stuff is very different

A pointer holds an address, an Array holds data (though this may seem to be the same thing, it is not even close).

Just because you (think you) can treat them as identical does not make them so....

2009-07-30 Reply Admin

char trio[9] = "da da da";

2009-07-30 Reply Admin

there's nothing inherently wrong with broadly cleaning up code... it's just the people least qualified to do it tend to want to do it the most.

2009-07-30 Reply Admin

Stig:
EvanED:
Stig:
The language semantics are technically different, but under the hood, they are identical. It is not a lie.
char* == char[] char** == *char[]

That's like saying "except for the places where they are different, they are identical".
No idea what you mean. "Semantically" they are different because one 'means' a pointer to a character, t'other 'means' and array of characters....however, an array essentially 'decays' (not my choice of word) into a char*, it just happens to have a contiguous bunch of chars next to it in memory.

EvanED:
They are allocated differently, they take up different amounts of storage and provide different information to sizeof, they provide different information to C++'s typeid.
All true. This is where confusion sets in. I am not talking about how the compiler allocates stuff, merely the underlying identical nature of a char* string and a char[] string.

No Offence 1)don't use terminology that you don't understand. The 'decays' is exactly what is saying that they are not the same thing but will (in some instances) be treated as the same thing. 2) The confusion has clearly already set in. How is the underlying nature identical when you agree that they are clearly treated totally differently? sizeof's handling is a perfect example - you cannot always expect the same behavior by changing an array to a pointer (assuming you hack in the required space allocation etc....

Try it!!!!

2009-07-30 Reply Admin

Stig:
EvanED:
Stig:
No idea what you mean. "Semantically" they are different because one 'means' a pointer to a character, t'other 'means' and array of characters....however, an array essentially 'decays' (not my choice of word) into a char*, it just happens to have a contiguous bunch of chars next to it in memory.
That's not what "semantics" means to me in the world of computer languages, where it means "how the program behaves".
Semantics is all about meaning, not behavior...an understanding of what it is you are expressing with your code.
And char arrays behave differently than char pointers.
Technically, neither 'behave' like anything. They're simply memory structures. If you look at char a[10] you will see that a is a pointer....to a char....followed by 9 other chars. This is the "underlying identical nature' I was referring to.

Have a look at that char a[10] you're talking about. Is it really a pointer? Really? "followed by 9 other".... Didn't you say earlier that contiguous is irrelevant (or was that someone else)?

Would you like to borrow a shovel?

2009-07-30 Reply Admin

Observer:
Not Quite:
No ... data_string is CONSTANT pointer to 15 chars that reside in stack <=> you can not assign pointers to it. Luckily c/c++ is not an ass (i mean here x_x - otherwise it is as ass as it can be) and does here a copy instead.
http://www.cplusplus.com/doc/tutorial/pointers/

I don't know what you're saying here, but you can assign a pointer to an array all day long... The article you linked to even says:
int numbers [20];
int * p;
The following assignment operation would be valid:
p = numbers;
If you were taking a stand for char[] being different than char*, you're on the right side of the argument, but...

I think that was precisely the OP's point.... the code above is fine (a pointer can point to an array) but if you instead had

numbers = p;

Then (obviously, I think) you would have a problem. You cannot allocate a point an array to the pointer but you can point the pointer to the array....

I better read the rest of the posts 'cos I get the impression others may have already 'splained this

DaveK · 2009-07-30 Reply Admin

EvanED:
Stig:
...highlights how char* and char[] are essentially no different 'under the hood'.

Except for the part where the address is stored explicitly for b and c but not for a. Did you not notice the mov instructions when calling puts(b) and puts(c) but the lea instruction to call puts(a) (in order to compute the address of a, which it needs to do since it isn't stored anywhere), or are you ignoring that and saying it gets wrapped up in the "essentially"?

Here, I commented the assembler (x86) version to make it clearer:

gcc-4.3.2 -O0:

	.file	"opt.c"
	.def	___main;    .scl  2;    .type  32;    .endef
	.section .rdata,"dr"
	; Note!  Read-only data section, string is not writeable.
LC0:
	.ascii "jklmnopqr\0"
	.text
.globl _main
	.def	_main;	.scl	2;	.type	32;	.endef
_main:
	leal	4(%esp), %ecx
	andl	$-16, %esp
	pushl	-4(%ecx)
	pushl	%ebp
	movl	%esp, %ebp
	pushl	%ecx
	subl	$52, %esp
	call	___main
	; Initialise array a[] on stack - writeable.
	; char  a[10] = "abcdefghi";
	movl	$1684234849, -22(%ebp)	; 0x64636261
	movl	$1751606885, -18(%ebp)	; 0x68676665
	movw	$105, -14(%ebp)		; 0x69
	; Store address of string in ptr b.
	; char* b     = "jklmnopqr";
	movl	$LC0, -12(%ebp)
	; get address of array a[]
	leal	-22(%ebp), %eax
	; store it in ptr c
	; char* c = a;
	movl	%eax, -8(%ebp)
	; get address of array a[]
	leal	-22(%ebp), %eax
	; push on stack
	movl	%eax, (%esp)
	; and call puts
	; printf("%s\n", a);
	call	_puts
	; get contents of ptr b
	movl	-12(%ebp), %eax
	; push on stack
	movl	%eax, (%esp)
	; and call puts
	; printf("%s\n", b);
	call	_puts
	; get contents of ptr c
	movl	-8(%ebp), %eax
	; and call puts
	; printf("%s\n", c);
	movl	%eax, (%esp)
	call	_puts
	; Convinced yet?
	addl	$52, %esp
	popl	%ecx
	popl	%ebp
	leal	-4(%ecx), %esp
	ret
	.def	_puts;	.scl	2;	.type	32;	.endef

Gah. Alex, there is no need for the BBCode [ code ] style to insert BR tags inside the PRE tags! Surely that should be easy to rip out?

2009-07-30 Reply Admin

Stig:
Go read up on lvalue and rvalue. And also see what's type of a pointer to char* and a pointer to char[10].
No. I know I'm right, and have many years of experience being right about this kind of triviality.
Confusion over what language elements mean, what compilers do with them and underlying memory structures is common among beginners.

Hahahaha....Clearly!!!

At best you are not explaining yourself clearly. At worst you assume all sorts of behavior because you have observed some of it.

A Pointer is Not an Array. => Pointers and Arrays are not the same thing

2009-07-30 Reply Admin

I bet the article author just got the before and after code snippets reversed.

2009-07-30 Reply Admin

Paul W. Homer:
Ok, maybe the WTF is that he actually tried to take the initiative and fix the code, rather than to live with its less than perfect syntax.
If we all set about trying to fix our code, you never know what horrible thing might happen next. It might actually work, or it might just not crash enough and then who knows, maybe we'd have to layoff all of those consultants and support people.

Huge chunks of the industry could get ruined. Maybe, even if things got bad enough, we'd have to shut down our blogs complaining about how bad code is.

The consequences of just allowing people to consistently go off and refactor anything at all are seriously dire. Thank goodness he didn't figure out how to use a const to point at it in the data section instead of just duplicating it on the stack, that could have been even more devastating!

Surely there is some question (if you have read this far, at least) over whether his syntax is itself any closer to perfect than the original. If it ain't broke, why fix it?

He took initiative to fix something that in his opinion wasn't done the right way.
Does this mean it was done the wrong way? No! Was it? Depends who you ask.

The existing code was (by most people's standards) perfectly acceptable, and by some people even the correct way to do it. Just because I think things should happen a certain way doesn't (always) make that the right way to do things....

I think this WTF might break records for # of comments...

2009-07-30 Reply Admin

Wizard Stan:
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?

HAHAHAHAHHAHAHAHAHAHAH!!!!!!

THIS!!!

2009-07-30 Reply Admin

Heron:
He then excluded the original files from the build. This did result in faster complete rebuilds, at the expense of slower partial rebuilds (i.e. what we do all day) and BREAKING FILE SCOPE RULES.
Yeah... we had lots of fun tracking down compile errors due to programmers assuming that they could trust file scope rules to hold. We never did convince the manager to revert those changes.

Why the hell did a manager have commit privileges?

2009-07-30 Reply Admin

Everything is TRWTF.

The team for not having a process -- particularly not regression testing -- and adding a new programmer to the mix.

The new guy for making global stylistic changes.

The head developer for turning the new guy loose, promising an untested product to a client, expecting things to be fixed in his absence, and then trying to ship it without reviewing the changes.

The author for including a nonsensical example, and for failing to explain the significance of the second CD.

The commenters for getting into a huge food fight over stack variables based entirely on their conjecture of what imaginary problem might have existed in the code.

2009-07-30 Reply Admin

Lupus.Umbrae:
Oxyd:
guy:
char data_string[15] = "data data data";
Doesn't this mean he overwrites the pointer data_string with the pointer to "data data data". Since both are of type char*?
data_string is of type char[15], not char*. In this context, this syntax is a mere syntactical sugar for char data_string[15] = { 'd', 'a', 't', 'a', ' ', 'd', 'a', 't', 'a', ' ','d', 'a', 't', 'a', 0 };. So no, he just initialized an array.
Actually, data_string IS a char*, pointing to data_string[0]. So, he alloated a 15 byte array and gave it the adress of... a 15 byte long static string? Weird...

In C or C++, a local char[15] is created by programatically giving offset on the stack, no different than a int. Space is allocated by making sure that the offset of any other stack variable doesn't point to any of those 15 bytes. The only pointers involved are the register(s) pointing to the current position on the stack. A local char * or void * allocates however many bytes are needed for an address and the address value actually resides on the stack, in RAM.

2009-07-30 Reply Admin

Stig:
EvanED:

ollo:
In fact, char[] is aequivalent to char*. Each one is a pointer to an 'array' of chars, ...

No, it's not. Stop spreading this lie.

The language semantics are technically different, but under the hood, they are identical. It is not a lie.

char* == char[] char** == *char[]

"Under the hood" is quite nebulous. And besides, char[] isn't the issue, char[n] is.

char[n] is (locally) a stack allocated array. No pointer resides on stack. char[] is a pointer....well because it doesn't make any sense otherwise.

The issue can also become clearer when you start dealing with multi-dimensional arrays. It's been a while sense I've done C or C++, but neither char** nor char[] are equal to char[m][n]; char** points to an array of char* somewhere. char[m][n]; is a block of mn characters that reside on the stack (if local). IIRC, You can't just do char * p = char[m], you have to do char p = &char[m][0];

2009-07-30 Reply Admin

JayC:
char[n] is (locally) a stack allocated array. No pointer resides on stack. char[] is a pointer....well because it doesn't make any sense otherwise.

Nevermind.... after reading some of the other comments, and remembering some of the bugs I had encountered a long time ago... I'm going to even have to retract this statement.

2009-07-30 Reply Admin

It's amazing how someone can be so adamant about something that they are so blatently wrong about.

char* and char[] are definitely not the same. Arrays depreciate to a pointer to their first element (so, using a char[] without the [] will be a pointer to the first element, which has type char*), so in many cases you can use char* with char[]'s, but that's about the only real similarity.

2009-07-30 Reply Admin

yeah, the WTF is he did not do his job. He was assign a task of hunting down a tricky memory leak, but he did something else to all the files

2009-07-30 Reply Admin

yeah, the WTF is he did not do his job. He was assign a task of hunting down a tricky memory leak, but he did something else to all the files

Seconded, the WTF is not that there's something really bad about declaring things the way he did(If you start out by doing that and you know what you're doing), the WTF is that in addition to not doing his job at all he probably introduced hundreds bugs by changing things this way.

joelkatz · 2009-07-30 Reply Admin

I think I figured out what the WTF is and why this is a WTF. The company did not have a formal code process. The code was informally in a lock state prior to a major release. The code had been debugged and approved and a small list of issues remained, including a memory leak.

So the idea was to find the memory leak and fix it. Ideally, with a small "obviously correct" change that would minimize the amount of additional testing required.

Instead, he did the complete opposite. He made as many changes as possible, many not anywhere close to obviously correct. This undid all the effort in debugging and validating the code for release and set their release cycle back to the beginning. (And almost certainly neither found or fixed the memory leak.)

And the OP correctly points out that this points out why you do need a code cycle process, ideally with branches and defined phases. And the WTF -- even with this object demonstration of why you need code branches, release freezes, and the like, the solution was to ignore the problem.

2009-07-31 Reply Admin

Billy:
TRWTF here is that instead of finding the memory leak by using something like, heaven forbid, valgrind ...

Or using the Boehm/Demers/Weiss (sp?) garbage collector in heap analysis mode. Or just using it period and never missing the painful development overhead of tracing memory leaks ever again, for that matter.

2009-07-31 Reply Admin

ollo:
In fact, char[] is aequivalent to char*. Each one is a pointer to an 'array' of chars, thus a pointer to the first byte of a number of bytes representing a string. The two different representations are only syntactic sugar.
AFAIR at least. :)

Yes, 'char[]' is similar to 'char *'. But 'char[10]' is MUCH different than 'char *'. If you don't understand this, please pick up a copy of K&R (you SHOULD know what this is!) and read it throughly.

2009-07-31 Reply Admin

That would apply to local static variables - but not to local automatic variables as shown. They are not in the BSS section.

lrucker · 2009-07-31 Reply Admin

Touching every source file like this should result in an immediate termination

You'd think.

Last place I worked, they hired someone to "clean up" our code. Admittedly it was in dire need of it - it dated back to 1980, it all lived in one folder, and there were bits of it that predated the widespread use of header files. OTOH, despite being insanely complicated, it worked, and files contained functions that were related to each other; a good programmer could be up to speed in a week or so.

New guy did not take those weeks; new guy jumped straight into rearranging code. Half an hour before he went on a two week vacation, he checked in the changes he'd been working on for weeks. Not only did this involve moving functions into files that made sense to him - who'd never actually used said functions - he managed to give one of his header files the same name as a system header. So it did not build, which we found out on Monday.

Of course we backed out his changes, but our source code was so ancient that we also inadvertently lost one of my changes that I'd checked in mere minutes after he did - something I didn't learn until weeks later, when a supposedly fixed bug reappeared. Of course there was no trace of my original fix.

Not only did new guy not get fired, he's still there. I'm not. But I've got a better job now - and I don't have to work with him.

2009-07-31 Reply Admin

This is a better wtf than the wtf!

2009-07-31 Reply Admin

Mad Formatter:
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".

Right Mr. Formatter, now tell me how this would work with "cvs annotate" or "svn blame"?

For those who are still stuck in the GUI, these are commands that show you in which revision a given line was changed and are indispensable in hunting down why some line of code is what it is.

Idiots like this Mad Formatter here make these tools bloody useless. What you would see is the revision when the line was last formatted, instead of when it was really changed.

2009-07-31 Reply Admin

Jub-Jub:
The pattern may not be unusual but it's stupid.... Doesn't bool initialise to false? We will always free If in doubt, initialise your bool first....
We could complicate the code by not using a bool

Why would a bool auto initialize to false? Its not a complex type. Anyhow, the fact that the fexternal code is buggy is not at issue - it was made to counter the assertion that there are no common code patterns in which an uninitialized variables could result in a memory leak.

Mister Fix-it

Leave a comment on “Mister Fix-it”