The Daily WTF: Curious Perversions in Information Technology

navigator · 2006-09-19 Reply Admin

themagni:
Anonymous:

But here's something I never have been able to figure out. What if your array was placed in memory near the end or you memory space and code like this tried to access beyond your physical memory limits? What would happen then? This behavious is so undefined to be a true hardcore WTF with no reliable result.
The short answer is: "You're farked."
If you're lucky, your program will stop running. Your watchdog timer (on an embedded system) will trigger and restart. Your computer will crash. You'll get a "not responding" error.
If you're not lucky, well, have you ever played "Memory rodeo"? It's where you and a friend each write a program to write random values to a random location in memory. That's what you'll end up doing. Remember that your peripheral devices, like your HDD, monitor, Ethernet card, etc. are all locations in memory. (At least down on the lowest levels.) If you write garbage to your monitor or to a random section on your HDD, what do you think is going to happen? As someone else said, "It's like your VCR manual: it doesn't say what happens if you insert a banana upside down in zero gravity. It's undefined."
If you're really unlucky, then you've created a buffer overflow exploit and your code gets released. There are errors in every piece of C code that uses "gets". There's no limit to the buffer size, and you get all kinds of malicious code potential.
That's the problem with C. It lets you fark up. Fark up big time.

The whole point of C was that it assumed you knew what you were doing, and why. With today's languages (environments?) doing a lot of the record-keeping (memory management) work for you, maybe many programmers have just never been hardened as to how computers really work under the hood.

2006-09-19 Reply Admin

Anonymous:
Anonymous:
actually, unless Alex made a typo and reversed the arguments to fread, this procedure will return completely _defined_ results when confronted with a 23byte file, it'll claim that the merchant is not special, because the return value of "fread" will be 0
Argh... nice catch. Can't believe everyone missed that. I guess the big question then is: at what point does he run out of file handles?

I accidently leaked FILE* handles once. Those things can be HUGE and take up tons of memory. This would most likely be a huge leak.

2006-09-19 Reply Admin

Looks like most of the WTF's have been pointed out. The big one I don't see covered is, why re-build the array from the file every time you want to check a merchant code?

I think people may be getting a little ahead of themselves on proclaiming what symptoms this will cause, though.

The C spec is odd about when the environment is or is not required to initialize memory. The environment is never forbidden from doing so, however. You should never write code that expects memory to be auto-initialized, even if you know that it will be in your enviornment. Pick your reason:

1) the rules are subtle enough that you might be wrong

2) someone trying to maintain your code might have a problem with it

3) your code isn't portable; even a minor revision change in your compiler could break your code

Probably the function is chaotic, but if his environment happens to zero the array before giving it to him, then the behavior is defined.

Alexis de Torquemada · 2006-09-19 Reply Admin

Anonymous:
Anonymous:
actually, unless Alex made a typo and reversed the arguments to fread, this procedure will return completely _defined_ results when confronted with a 23byte file, it'll claim that the merchant is not special, because the return value of "fread" will be 0
Argh... nice catch. Can't believe everyone missed that. I guess the big question then is: at what point does he run out of file handles?

Why bother? His code will fail to work just as it failed before, and if he doesn't care about whether his own code works or not, why should he even mind other people's code?

Ghost Ware Wizard · 2006-09-19 Reply Admin

WTF is spot on here with their code snippet above. however did you notice the glorified C programmer who was a legend has a logical error as well?

if the file can't be open return true

i.e. it's a special merchant

IMHO the self-proclamation of a has-been is spot on

2006-09-19 Reply Admin

themagni:
Anonymous:
But here's something I never have been able to figure out. What if your array was placed in memory near the end or you memory space and code like this tried to access beyond your physical memory limits? What would happen then? This behavious is so undefined to be a true hardcore WTF with no reliable result.
The short answer is: "You're farked."
If you're lucky, your program will stop running. Your watchdog timer (on an embedded system) will trigger and restart. Your computer will crash. You'll get a "not responding" error.
If you're not lucky, well, have you ever played "Memory rodeo"? It's where you and a friend each write a program to write random values to a random location in memory. That's what you'll end up doing. Remember that your peripheral devices, like your HDD, monitor, Ethernet card, etc. are all locations in memory. (At least down on the lowest levels.) If you write garbage to your monitor or to a random section on your HDD, what do you think is going to happen? As someone else said, "It's like your VCR manual: it doesn't say what happens if you insert a banana upside down in zero gravity. It's undefined."
If you're really unlucky, then you've created a buffer overflow exploit and your code gets released. There are errors in every piece of C code that uses "gets". There's no limit to the buffer size, and you get all kinds of malicious code potential.
That's the problem with C. It lets you fark up. Fark up big time.

This is just false. No such thing is possible in any modern OS. The user above, jspenguin, gave the correct answer to this question:

jspenguin:
The array is allocated on the stack. So, on 32-bit x86, *((int*)filedat[10000]) will be the same as fln, *((void**)filedat[10004]) will be the previous value of of ebp, *((void**)filedat[10008]) is the return address of the function, *((int*)filedat[10012]) is the parameter merchnum, followed by the locals of whatever called is_special_merchant, and so on. If the index is high enough, it will wrap around and could end up pointing anywhere. If the address is not mapped (or is part of the protected kernel address space) it causes a segmentation violation, which on Windows is represented by that nice little dialog box asking you to send an error report to Microsoft. Unless this code is being run in the kernel (which itself would be a major WTF), then it would cause a panic or bluescreen.

C doesn't let you "fark up" any more than any other language.

2006-09-19 Reply Admin

themagni:
DontKnow:

W[ho]TF signed off on writing the software for maintaining this special merchants file instead of forcing it into the DB with all other merchant information?
Oh, and this code appears to be an approximate replacement for return(random()&0x01);
--DK
That right there is the real gem. Not the implementation, but the fact that this code even exists. This information should, nay MUST, be in the database along with all of the other DATA. There's no reason in this universe that a special field should be put outside of the rest of the data.
Even the most perfectly coded, logical method for determining the "special" nature of the merchant would be wrong - it's not the correct place to put this sort of information!
His code's wrong, though.

I think it goes a bit far to assume that there is a database involved in this application. It might well be a whole collection of text files like this...

Alexis de Torquemada · 2006-09-19 Reply Admin

Anonymous:
Probably the function is chaotic, but if his environment happens to zero the array before giving it to him, then the behavior is defined.

Well, technically, only if the environment is documented to work that way. And the C standard clearly marks this as undefined behavior: Lacking an explicit initialization or write access, the array values are indeterminate (§6.7.8/10 of C99), and using an indeterminate value causes undefined behavior (Appendix J2). A different specification for a particular implementation would of course override this - but only for this implementation.

navigator · 2006-09-19 Reply Admin

Alex Papadimoulis:

int is_special_merchant(int merchnum) 			// 1
{						// 2
  size_t fln;					// 3
  char filedat[10000];					// 4
  FILE *file;					// 5
  int ab = 0;					// 6
  if ((file=fopen("spcmerch.cfg","r" )) == NULL ) return(1);	// 7
  fln = fread(&filedat, 10000, 1, file);			// 8
  if(fln == (size_t)0) return(1);				// 9
  fclose(file);					// 10
  if(filedat[merchnum] == 'Y') return(0);			// 11
  return(1);						// 12
}						// 13

So, in 13 lines of code, we have:

WTF # 1: function is inversely named for what it does
WTF # 2: size of array is a hard-coded literal - as opposed to a defined constant, or worse, configurable
WTF # 3: args to fread are reversed (10000, 1), making line (9) wrong
WTF # 4: the file handle won't be closed and released more often than not
WTF # 5: line 11 is likely indexing into undefined memory
WTF # 6: separating the 'special' merchant-attribute from the DB into a flat file
WTF # 7: not necessarily WTF, but bad form: uninitialized variable (file, fln) before use
WTF # 8: 'ab' is declared but unused
WTF # 9: no way to indicate errors have occurred
WTF # 10: config file is necessarily hard to read/maintain: unscalable
WTF # 11: assuming it's not in response to a web call, where no state can be maintained, the merchant flags are not cached (they are read each time)

Ignoring braces, that's one per line, including declarations - nice!

2006-09-19 Reply Admin

themagni:
Anonymous:

But here's something I never have been able to figure out. What if your array was placed in memory near the end or you memory space and code like this tried to access beyond your physical memory limits? What would happen then? This behavious is so undefined to be a true hardcore WTF with no reliable result.
The short answer is: "You're farked."

Actually, the question and your reply both suggest a need to read up on how memory lookup is actually performed.

The physical location of the array in memory is completely irrelevant. That's what virtual memory is all about. The array could end on the last byte of the last page in physical memory, and it would make no difference in figuring what happens on an out-of-bounds index. In fact, depending on the system's paging behavior, that array could move around in physical memory over time. Luckily your program cannot know anything about physical memory; hence it cannot ask for an address outside your physical bounds.

Now, an out-of-bounds index could fall outside your allocated virtual memory space. Again, the "end" of the address space doesn't change the behavior any; MAX_INT+1 = MIN_INT, so either way you're referencing a valid address other than one you intend to reference.

So, behavior doesn't depend on the bounds of physical memory or of the address space; but it does depend on OS. In DOS, you could indeed crash the OS (or worse). In any reasonable OS, you expect the program to crash; but if it doesn't, you get a very subtle, random bug. This is why God invented memory debuggers.

Each programming language makes a trade-off somewhere between performance and "preventing you from shooting yourself in the foot". When C was in common use, (a) performance was more important, and (b) other techniques to level the performance playing field hadn't been invented. For C to do run-time bounds checking would not have been reasonable (and in some applications run-time bounds checking a la Java is still not acceptable).

So, this is not a "problem" with C. It is a reason to know what you're doing and apply the right tools if you're going to write C.

Alexis de Torquemada · 2006-09-19 Reply Admin

Anonymous:
1) Addresses are calculated using "normal computer math". This means that "integer overflows" will result. In C, the result of an "integer overflow", is simply "wrapping" the number back around. It's basic field math. 0xFFFFFFFF + 0x00000001 = 0x00000000

Popular myths about the C standard... Actually, C99 explicitely lists integer overflow as an example of undefined behavior (§3.4.3/3). Most implementations just wrap around because that's the natural result of not caring to handle possible integer overflows on x86, but that's not the same as standardized behavior.

2006-09-19 Reply Admin

Alex Papadimoulis:
NNNNYNNNYNNYYNYYNNNNYNN
Why the former Software Legend thought that a twenty-three byte file would suffice when the code reads in 10,000 is a mystery. The fact that this random-result providing function is still in production is also a mystery. But the fact that this is bug is compensated for by several other equally-buggy and somehow-working functions -- now that's legendary.

Clearly the 10,000 bytes are there for expandability. I'm not sure that one can infer a random result from the code though. It seems obvious to me that if you want set the "special" status for a given merchant, you enter a Y or N indicator at the appropriate position in the file.

The real WTF is that it's allocating memory, opening a file handle, and reading the data into memory over and over again. That's just brillant.

2006-09-19 Reply Admin

Not even a WTF

This is user error.

Someone just forgot how to maintain the spcmerch.cfg file after the 23rd merchant signed up.

Jeff S · 2006-09-19 Reply Admin

drdamour:
GoatCheez:

Somehow I don't think this code is as big of a WTF as being claimed, or if it's a WTF at all.
Merchant codes are at max 4 digits, meaning the largest merchant code would be 9999. He allocated 10000 bytes for his buffer. Perfect. It works perfect for the current spec. fread attempts to read that many bytes. It'll read as many as it can, so good. He doen't check that the merchant code is greater than the length of the file. That COULD be a wtf.... No, frankly this code isn't that bad. Sure, if it encounters an error it'll return 1, but that's the worse it'll do. Just make sure the mechant codes start at 0 and increment from there.

but his char[10000] won't be 10000 null char's it will be random, with one of them possibly being the bit codes for the char Y, and hence he'll get a false positive (or in his case a true negative?)

that's what I see as the WTF as well, though of course the file format for the "special merchants file" is certainly pretty silly. And returning 1 for false and 0 for true.

themagni · 2006-09-19 Reply Admin

navigator:
The whole point of C was that it assumed you knew what you were doing, and why. With today's languages (environments?) doing a lot of the record-keeping (memory management) work for you, maybe many programmers have just never been hardened as to how computers really work under the hood.

Indeed, I have found the same thing - a lack of knowledge about what exactly the machine is going to do with the code your compiler writes for it. Why bother learning about memory management? My language takes care of all of that for me!

Ian:
[snip out parts theMagni I said about undefined actions, bananas, and zero gravity.]

This is just false. No such thing is possible in any modern OS. The user above, jspenguin, gave the correct answer to this question:

C doesn't let you "fark up" any more than any other language.

I work on embedded systems, so I do not get to use any features of a modern OS - there is no OS! I certainly don't have access to "virtual memory" like our visiting Slashdotter (The Anonymous Coward) suggested. Perhaps the ability to fark up is more a factor of the environment I work with than the language, but don't assume that a modern OS will always save your butt. I use C almost exclusively, with a light dash of assembly to keep it interesting.

Some systems will truncate the address - if you try to write [value] to 0x10000 on a system that only has up to 0xFFFF, it might write to 0x0000. It might write to 0xFFFF. It might not execute. You don't know, because it's not defined.

I am gratified to know that at least someone reads my posts. ;)

2006-09-19 Reply Admin

The real ultimate WTF is all the special merchants (except for any in the lucky first 23) got ordinary handling (whatever that was). Why did no one catch the business WTF here in all that time?

2006-09-19 Reply Admin

Anonymous:

Yeah, looks like he's following old skool UNIX exit codes. It's not really a stretch if you turn the wayback machine to the late 70's/early 80's. I'm more offended by the magic cookies and hardcoded pathnames. And the lack of bounds checking. And any number of other things that even 20 years ago would have made this a WTF.
But the real WTF is that companies continue to hire "senior" programmers like this who haven't kept up with language changes. I recently worked with a "senior" toolsmith who clearly wasn't comfortable with any modern languages. I finally asked what language he was most comfortable with. Like Scott here, he chose C. But not just any C. He was most comfortable with pre-ANSI C. Anything after pre-ANSI C, and he was pretty lost. WTF?

I guess your "language" of choice is VB?

2006-09-19 Reply Admin

I've seen a similar WTF in a similar application, yet the system worked perfectly. How so?

Some merchants require custom handling; this handling is specific to each merchant.
A manager decides that the "custom merchants" should be specified in a config file and the developer writes some code for it.
The developer also implements a switch statement that calls the appropriate custom function given the merchant number.
The config file is consequently useless, so it doesn't matter what results it gives.

MaGnA · 2006-09-19 Reply Admin

The real WTF:

  char filedat[10000];

...

  fln = fread(&filedat, 10000, 1, file);

You don't really need the & operator because the char array can be implicitly cast to a char pointer. It makes the code 1 character longer than it's supposed to be! He probably didn't know that *shorter code runs faster*! I bet you guys didn't know either!

2006-09-19 Reply Admin

themagni:
Ian:
[snip out parts theMagni I said about undefined actions, bananas, and zero gravity.]

This is just false. No such thing is possible in any modern OS. The user above, jspenguin, gave the correct answer to this question:

C doesn't let you "fark up" any more than any other language.

I work on embedded systems, so I do not get to use any features of a modern OS - there is no OS! I certainly don't have access to "virtual memory" like our visiting Slashdotter (The Anonymous Coward) suggested. Perhaps the ability to fark up is more a factor of the environment I work with than the language, but don't assume that a modern OS will always save your butt. I use C almost exclusively, with a light dash of assembly to keep it interesting.
Some systems will truncate the address - if you try to write [value] to 0x10000 on a system that only has up to 0xFFFF, it might write to 0x0000. It might write to 0xFFFF. It might not execute. You don't know, because it's not defined.
I am gratified to know that at least someone reads my posts. ;)

Your point about writing code at the "kernel level" (I know if it's embedded it's not technically any level, but it's the same end result on either an embedded system or a PC) is well taken, except that applications usually don't run at the kernel level. I know that I would write code using very different assumptions if I know it's going to be running without an OS handling page faults and setting permission levels for me, but those assumptions aren't really relevant to the topic at hand.

GoatCheez · 2006-09-19 Reply Admin

Jeff S:
drdamour:
GoatCheez:

Somehow I don't think this code is as big of a WTF as being claimed, or if it's a WTF at all.
Merchant codes are at max 4 digits, meaning the largest merchant code would be 9999. He allocated 10000 bytes for his buffer. Perfect. It works perfect for the current spec. fread attempts to read that many bytes. It'll read as many as it can, so good. He doen't check that the merchant code is greater than the length of the file. That COULD be a wtf.... No, frankly this code isn't that bad. Sure, if it encounters an error it'll return 1, but that's the worse it'll do. Just make sure the mechant codes start at 0 and increment from there.

but his char[10000] won't be 10000 null char's it will be random, with one of them possibly being the bit codes for the char Y, and hence he'll get a false positive (or in his case a true negative?)
that's what I see as the WTF as well, though of course the file format for the "special merchants file" is certainly pretty silly. And returning 1 for false and 0 for true.

If anything, the WTF is that he didn't check that the merchant code was in the valid range. Below: the correct source for what "should" have been in the file. Note: I don't think that returning 0 for "true" is a wtf in itself without seeing other code from the library. A standard include file for this project could simply have #define TRUE 0 #definre FALSE 1.

int is_special_merchant(int merchnum) 
{
  size_t fln;
  char filedat[10000];
  FILE *file;
  int ab = 0;

  if ((file=fopen("spcmerch.cfg","r" )) == NULL ) return(1);
  fln = fread(&filedat, 1, 10000, file);

  if ((fln == (size_t)0) || (fln < merchnum) return(1);
  fclose(file);
  if(filedat[merchnum] == 'Y') return(0);

  return(1);
}

themagni · 2006-09-19 Reply Admin

Anonymous:
themagni:
Ian:
[snip out parts theMagni I said about undefined actions, bananas, and zero gravity.]

This is just false. No such thing is possible in any modern OS. The user above, jspenguin, gave the correct answer to this question:

C doesn't let you "fark up" any more than any other language.

I work on embedded systems, so I do not get to use any features of a modern OS - there is no OS! I certainly don't have access to "virtual memory" like our visiting Slashdotter (The Anonymous Coward) suggested. Perhaps the ability to fark up is more a factor of the environment I work with than the language, but don't assume that a modern OS will always save your butt. I use C almost exclusively, with a light dash of assembly to keep it interesting.
Some systems will truncate the address - if you try to write [value] to 0x10000 on a system that only has up to 0xFFFF, it might write to 0x0000. It might write to 0xFFFF. It might not execute. You don't know, because it's not defined.
I am gratified to know that at least someone reads my posts. ;)

Your point about writing code at the "kernel level" (I know if it's embedded it's not technically any level, but it's the same end result on either an embedded system or a PC) is well taken, except that applications usually don't run at the kernel level. I know that I would write code using very different assumptions if I know it's going to be running without an OS handling page faults and setting permission levels for me, but those assumptions aren't really relevant to the topic at hand.

You're absolutely right on all counts, including the relevancy. I'll get back to work now.

ParkinT · 2006-09-19 Reply Admin

Why the former Software Legend thought that a twenty-three byte file would suffice when the code reads in 10,000 is a mystery.

As any Sotware Legend will tell you, it is very important to continuously exercise the filesystem and memory of any computer.

makomk · 2006-09-19 Reply Admin

Alexis de Torquemada:
Anonymous:
Probably the function is chaotic, but if his environment happens to zero the array before giving it to him, then the behavior is defined.
Well, technically, only if the environment is documented to work that way. And the C standard clearly marks this as undefined behavior: Lacking an explicit initialization or write access, the array values are indeterminate (§6.7.8/10 of C99), and using an indeterminate value causes undefined behavior (Appendix J2). A different specification for a particular implementation would of course override this - but only for this implementation.

And in practice, I think allocating memory on the stack generally consists of a singe operation: moving the stack pointer appropriately. (I don't know of any C implementations that do bother to initialise stack-allocated arrays, though I suppose they could exist - I'd be surprised, though, since it'd make function calls more expensive...)

Jeff S · 2006-09-19 Reply Admin

GoatCheez:
Jeff S:
drdamour:
GoatCheez:

Somehow I don't think this code is as big of a WTF as being claimed, or if it's a WTF at all.
Merchant codes are at max 4 digits, meaning the largest merchant code would be 9999. He allocated 10000 bytes for his buffer. Perfect. It works perfect for the current spec. fread attempts to read that many bytes. It'll read as many as it can, so good. He doen't check that the merchant code is greater than the length of the file. That COULD be a wtf.... No, frankly this code isn't that bad. Sure, if it encounters an error it'll return 1, but that's the worse it'll do. Just make sure the mechant codes start at 0 and increment from there.

but his char[10000] won't be 10000 null char's it will be random, with one of them possibly being the bit codes for the char Y, and hence he'll get a false positive (or in his case a true negative?)
that's what I see as the WTF as well, though of course the file format for the "special merchants file" is certainly pretty silly. And returning 1 for false and 0 for true.
If anything, the WTF is that he didn't check that the merchant code was in the valid range. Below: the correct source for what "should" have been in the file. Note: I don't think that returning 0 for "true" is a wtf in itself without seeing other code from the library. A standard include file for this project could simply have #define TRUE 0 #definre FALSE 1.
int is_special_merchant(int merchnum) 
{
  size_t fln;
  char filedat[10000];
  FILE *file;
  int ab = 0;

  if ((file=fopen("spcmerch.cfg","r" )) == NULL ) return(1);
  fln = fread(&filedat, 1, 10000, file);

  if ((fln == (size_t)0) || (fln < merchnum) return(1);
  fclose(file);
  if(filedat[merchnum] == 'Y') return(0);

  return(1);
}

if TRUE and/or FALSE are #define'd, then those labels should be returned, not the hard-coded values of 1 or 0 ... that would be an even bigger WTF, since the whole point of #define is that you can maintain/view/change these values in one place .... and if you did that you would break this function.

FYI -- your code doesn't close the file if it is shorter than the merchnum.

dmitriy · 2006-09-19 Reply Admin

At first, before I saw the sample merchant file, I thought "what happens if the file's size is more than 10,000 characters?" The standard solution is to read the file one line at a time until reaching end of file.

Now, I see another bug. If, indeed, every merchange has a four-digit number, this function will never return 0. None of the indexes in the array have four digits.-

GoatCheez · 2006-09-19 Reply Admin

Jeff S:
if TRUE and/or FALSE are #define'd, then those labels should be returned, not the hard-coded values of 1 or 0 ... that would be an even bigger WTF, since the whole point of #define is that you can maintain/view/change these values in one place .... and if you did that you would break this function.
FYI -- your code doesn't close the file if it is shorter than the merchnum.

Yeah, that's true.... And yeah, i forgot to close the file.... oh well... I also forgot a ) on that if statement. Yeah, ok, i'll admit that there are WTFs all thoughout that code... Although they are more "bugs" than WTFs.... Yeah, I'd call all of those bugs except for the #define thing, that's a WTF.... Lots of bugs though....

2006-09-19 Reply Admin

GoatCheez:
Jeff S:
drdamour:
GoatCheez:

Somehow I don't think this code is as big of a WTF as being claimed, or if it's a WTF at all.

Merchant codes are at max 4 digits, meaning the largest merchant code would be 9999. He allocated 10000 bytes for his buffer. Perfect. It works perfect for the current spec. fread attempts to read that many bytes. It'll read as many as it can, so good. He doen't check that the merchant code is greater than the length of the file. That COULD be a wtf.... No, frankly this code isn't that bad. Sure, if it encounters an error it'll return 1, but that's the worse it'll do. Just make sure the mechant codes start at 0 and increment from there.

but his char[10000] won't be 10000 null char's it will be random, with one of them possibly being the bit codes for the char Y, and hence he'll get a false positive (or in his case a true negative?)

that's what I see as the WTF as well, though of course the file format for the "special merchants file" is certainly pretty silly. And returning 1 for false and 0 for true.

If anything, the WTF is that he didn't check that the merchant code was in the valid range. Below: the correct source for what "should" have been in the file. Note: I don't think that returning 0 for "true" is a wtf in itself without seeing other code from the library. A standard include file for this project could simply have #define TRUE 0 #definre FALSE 1.
int is_special_merchant(int merchnum) 

{

  size_t fln;

  char filedat[10000];

  FILE *file;

  int ab = 0;



  if ((file=fopen("spcmerch.cfg","r" )) == NULL ) return(1);

  fln = fread(&filedat, 1, 10000, file);



  if ((fln == (size_t)0) || (fln < merchnum) return(1);

  fclose(file);

  if(filedat[merchnum] == 'Y') return(0);



  return(1);

}

Let me fix that for you:

int is_special_merchant(int merchnum) 
{
  size_t fln;
  char filedat[10000];
  FILE *file;
 
  if ((file=fopen("spcmerch.cfg","r" )) == NULL ) return(1);
  fln = fread(&filedat, 1, 10000, file) - 1;
  fclose(file);

  if (fln < merchnum) return(1);
  if(filedat[merchnum] == 'Y') return(0);

  return(1);
}

2006-09-19 Reply Admin

drdamour:
A better approach in this case would to have had a directory of special merchants, with 1 file for each special merchant named with the merchant code. So, eg if merchant 5678 was special, there should be a 5678.merchant file in the special_merchants directory. You try to open up the file for reading, and if it works you know it's special, otherwise it's not. Very simple to add special merchants, and you could even go as far as putting security on merchants via the filesystem.

Probably even more of a WTF waiting to happen. Especially on filesystems that don't implement directory indexing, locating a specific file can be an O(n) operation, involving multiple disk I/O operations - on a Unixoid filesystem, each NNNN.merchant filename is 13 bytes plus a trailing null for 14, plus another 4 bytes or so for an inode number. You're looking at a 180K directory, which is 43 4K blocks. Better hope all 43 are kept hot in the in-memory cache. Also, on a Unix box, reading the directory will (unless you mounted the filesystem 'noatime') cause an update of the access time of the directory, which will force a disk write to update the time stored on disk.

It turns out that often the performance is *far* better by creating a directory '5', a directory '6', a directory '7', and then touch '8.merchants". Then the code tries to open "5/6/7/8.merchants" this will result in at most 10 compares in each of 3 directories, rather than 10K against one.

2006-09-19 Reply Admin

Hi, long time reader, first time poster.

Can't you see that Scott, like any former legend has implemented a MOD 24 function on the actual merchant number, so the value passed as merchnum will always be in the range of 0 to 23, The hidden complexity is in organizing the 23 bytes and merchant numbers such that the correct value will always be given.

Scott is my new hero.

Gryphon

2006-09-19 Reply Admin

The Jeff Dictionary:

bug¹ - noun 1 the common name for any of thousands of insects with a flattened oval body and mouthparts modified to form a beak for piercing and sucking, eg aphids. 2 an insect living in dirty houses, etc and thought of as dirty. 3 N Amer a popular name for any kind of insect. 4 colloq a popular name for a bacterium or virus that causes infection or illness • a nasty stomach bug. 5 colloq a small hidden microphone. 6 colloq missing all the previous wtf's whilst claiming to have fixed them. 7 colloq an obsession or craze • She caught the skiing bug. verb (bugged, bugging) 1 colloq to hide a microphone in (a room, telephone, etc) so as to be able to listen in to any conversations carried on there. 2 slang to annoy or worry someone. 3 US colloq said of the eyes: to bulge or pop out (like the eyes of certain insects).
ETYMOLOGY: 17c: perhaps connected with Anglo-Saxon budda a beetle.

2006-09-19 Reply Admin

Ack, my bad, the GoatCheez dictionary. Sorry :S

2006-09-19 Reply Admin

themagni:

That's the problem with C. It lets you fark up. Fark up big time.

Don't try putting this on C. Stupidity like the above function is entirely the programmer's fault. Frankly, I take exception to the notion that programmers need to be protected from themselves. Idiot-proofing never works because there's always a bigger idiot, and it makes the job harder for people who actually know what they're doing. The minute you circumscribe a language's expressive power in order to keep the programmer from making a mistake (instead of for a design reason) you're just asking for trouble down the line, because someone will want to do something clever or innovative that you didn't anticipate, and they have to jump through a bunch of hoops to make it work.

Building helpful abstractions is great. Removing the ability to get under the hood when necessary is not.

2006-09-19 Reply Admin

Everyone seems to have missed a WTF in this line:

  fln = fread(&filedat, 10000, 1, file);

 filedat is a char*. Therefore, &filedat is a char**, and fread will dump its data all over the pointer and everything following it, rather than the character array the pointer points to. There's no way this code could ever have run successfully.

2006-09-19 Reply Admin

Anonymous:
The config file would te impossible to maintain. Imagine that only merchant 3049 is "special". That means the file's author has to type 3049 (remember to count from zero) N's and then a Y. And since the file is text based and used by this monstrosity I very much doubt that there's a tool to do this for you.

Such file could be generated from database query - of course it will be pretty big WTF, but not every "legend" knows how to comunicate with databases :)

2006-09-19 Reply Admin

Alex Papadimoulis:
...he only writes code in C while everyone else uses Java because, as he claims, C is actually more object-oriented;

So is he just responsible for junk tasks, while everybody else does something productive, or are there a lot of native calls (also known as "How slow can you go")?

An while I'm at it, how is it that this guy even gets to decide what language he'll use? He sounds like a manager to me...

GoatCheez · 2006-09-19 Reply Admin

Anonymous:
The Jeff Dictionary:
bug¹ - noun 1 the common name for any of thousands of insects with a flattened oval body and mouthparts modified to form a beak for piercing and sucking, eg aphids. 2 an insect living in dirty houses, etc and thought of as dirty. 3 N Amer a popular name for any kind of insect. 4 colloq a popular name for a bacterium or virus that causes infection or illness • a nasty stomach bug. 5 colloq a small hidden microphone. 6 colloq missing all the previous wtf's whilst claiming to have fixed them. 7 colloq an obsession or craze • She caught the skiing bug. verb (bugged, bugging) 1 colloq to hide a microphone in (a room, telephone, etc) so as to be able to listen in to any conversations carried on there. 2 slang to annoy or worry someone. 3 US colloq said of the eyes: to bulge or pop out (like the eyes of certain insects).
ETYMOLOGY: 17c: perhaps connected with Anglo-Saxon budda a beetle.

Bah, I missed a damned parethesis and closing the file. I was just trying to point out that the damn thing was bugged and not really that WTF 'd up.

bug1 /b?g/ Pronunciation Key - Show Spelled Pronunciation[buhg] Pronunciation Key - Show IPA Pronunciation noun, verb, bugged, bug?ging.

4.Informal. a defect or imperfection, as in a mechanical device, computer program, or plan; glitch: The test flight discovered the bugs in the new plane.

Everything highlighted thus far falls into the category of a bug, save the 1/0 true/false thing. That's only a WTF if the system was designed otherwise. It would've been WTF'd up if he had like generated a random number and then used that as the return value. The code does come close to that for some circumstances, however it is plainly evident that the programmer did not want it to function that way, thus it's a bug.

WTF: Trying to dimish another's attempt at resolution but accidentally targeting the wrong person all whilst never contributing anything useful to the thread.

2006-09-19 Reply Admin

Anonymous:
actually, unless Alex made a typo and reversed the arguments to fread, this procedure will return completely _defined_ results when confronted with a 23byte file, it'll claim that the merchant is not special, because the return value of "fread" will be 0

fread returns the number of _objects_ read, given the specified argument order fread(buf,10000,1,file), an object is defined as being 10000 bytes long, since the file only contains 23 byte it doesn't contain a single (complete) object and thus the return value will be 0.

BINGO! So the function should always return 1 with a short file. As we see from the backwards logic with the "Y" test, a result of 1 apparently means "not special". And as others have pointed out, it won't close the file handle in this case. Depending on your OS, there may be ten to hundreds of file handles available, but the OS will usually close them automagically when the program exits.

man fread:
The functions fread() and fwrite() advance the file position indicator for the stream by the number of bytes read or written. They return the number of objects read or written. If an error occurs, or the end-of-file is reached, the return value is a short object count (or zero).

The function fread() does not distinguish between end-of-file and error, and callers must use feof(3) and ferror(3) to determine which occurred. The function fwrite() returns a value less than nmemb only if a write error has occurred.

capcha: random (isn't it always random?)

2006-09-19 Reply Admin

Dwayne:

Don't try putting this on C. Stupidity like the above function is entirely the programmer's fault. Frankly, I take exception to the notion that programmers need to be protected from themselves. Idiot-proofing never works because there's always a bigger idiot, and it makes the job harder for people who actually know what they're doing. The minute you circumscribe a language's expressive power in order to keep the programmer from making a mistake (instead of for a design reason) you're just asking for trouble down the line, because someone will want to do something clever or innovative that you didn't anticipate, and they have to jump through a bunch of hoops to make it work.
Building helpful abstractions is great. Removing the ability to get under the hood when necessary is not.

Best... post... ever.

Without getting into a language war, I'll say that sometimes you NEED something closer to the metal. That's what C and C++ are for; so the programmer can program closer to the metal without having to completely resort to Assembly (though that can be required anyway at times).

2006-09-19 Reply Admin

Eh, the real WTF is the apparant lack of code reviews by competent developers prior to a system being installed for production use. (Ok, to be fair, if that actually occured in the real world then this entire website wouldn't actually exist.)

Step 1 - code reviews detect the flaw, the product isn't released til it actually works
Step 2 - some serious retraining has to occur
Step 3 - if that fails, de-hire the guy, or perhaps just reassign him to fetching coffee

The guy might be an idiot, but unless everyone else (esp. the ones in charge) are also idiots, it's not the hardest problem in the world to contain and eliminate.

2006-09-19 Reply Admin

drdamour:
A better approach in this case would to have had a directory of special merchants, with 1 file for each special merchant named with the merchant code. So, eg if merchant 5678 was special, there should be a 5678.merchant file in the special_merchants directory. You try to open up the file for reading, and if it works you know it's special, otherwise it's not. Very simple to add special merchants, and you could even go as far as putting security on merchants via the filesystem.

The best part is that then the function could then return either TRUE or FileNotFound.

themagni · 2006-09-19 Reply Admin

Dwayne:
themagni:

That's the problem with C. It lets you fark up. Fark up big time.

Don't try putting this on C. Stupidity like the above function is entirely the programmer's fault. Frankly, I take exception to the notion that programmers need to be protected from themselves. Idiot-proofing never works because there's always a bigger idiot, and it makes the job harder for people who actually know what they're doing. The minute you circumscribe a language's expressive power in order to keep the programmer from making a mistake (instead of for a design reason) you're just asking for trouble down the line, because someone will want to do something clever or innovative that you didn't anticipate, and they have to jump through a bunch of hoops to make it work.

Building helpful abstractions is great. Removing the ability to get under the hood when necessary is not.

That, Dwayne, was an excellent post. We agree, so we shouldn't argue.

I wasn't "putting this on C". I like C. That's what I use. As I have stated in previous posts, ALL of my work is "under the hood". (Embedded systems) My point was that C lets you do things that a lot of other languages will not let you do. If you don't know what you're doing, the compiler will assume that you're being clever and run anyway. There's nothing to stop you. It's a language with all the safeties off. (The only way to get even more raw power is assembly, but that's going too far for my tastes.) That's why "C lets you fark up big time". This isn't a statement intended to show a problem in C, but to illustrate its power. You can pull crazy stunts with C that you can't pull in other languages. With that great power comes the responsibility to handle your errors, initialize, manage your memory, exit cleanly, etc.

The existence of the code that's posted in today's post shouldn't exist. The "special cases" should be flagged in a database, not in a seperate file. (And if there isn't a DB, then there should be one. That's a bigger WTF than anything that the legacy coder wrote.)

The code that WAS written just shows that this guy doesn't know what the hell he's doing. He's dangerous to some people and offensive to others. Experience be damned, he's got no business writing in C. Hell, he's got no business coding at all.

lomaxx · 2006-09-19 Reply Admin

I understand that back in the day C was hot stuff, but I really hate these "Legends" of coding that refuse to move with the times just because in 1981 they could create ascii porn better than anyone else..

2006-09-19 Reply Admin

Anonymous:
The real ultimate WTF is all the special merchants (except for any in the lucky first 23) got ordinary handling (whatever that was). Why did no one catch the business WTF here in all that time?

Maybe the real WTF is that the function doesn't actually do anything at all, and its results are totally ignored (or duplicated) later on. Thus no one would even notice.

2006-09-19 Reply Admin

themagni:
I work on embedded systems, so I do not get to use any features of a modern OS - there is no OS! I certainly don't have access to "virtual memory" like our visiting Slashdotter (The Anonymous Coward) suggested.

DRAT! That's what I came in here to say. You stole my smugness, themagni.

2006-09-19 Reply Admin

Anonymous:
shadowman:
Not to mention 23 bytes for a 4-digit merchant code? So what happens when myMerchant = 2423?
The nice-guy inside of me says we should give the guy the benefit of the doubt and presume the 23-byte file was his "test" file to make sure his function works as coded, and he just got lazy and forgot to put in the data for the other several thousand merchants.
The evil-genius inside of me says he's decent fodder for my next crusade to take over the world.

Rule 47 of the Evil Programmers Handbook:

All coders will be able to identify an unintialized array. Those who can not will be promoted to managment.

Rule 48:

Managment will be given long trips to build up team spirit... in some remote universe.

CAPTCHA = java

2006-09-19 Reply Admin

Yeah... 'Cos looking at the code it is obvious that the parameter int merchnum has to be 4 digits - that is the first thing I thought. And what kind of crazy person would believe that four digit merchant ID's could ever change in the future? Also, opening files is cheap. Just one tiny little line, and maintaining the merchant file is the kind of useful task you could give to an intern or some such. And finally, who really cares if you give the function a merchnum that is past the end of the initialised array? The chance of silently returning a wrong result is tiny - lets see, assuming a random spread of bytes in the uninitalised part of the array, (that would be, all but the first 23 bytes) for a mechant that is not special, there is 1 in 256 chance that the unitialised byte will be a 'Y' character, so you get a false positive, and then there are 2% that are supposed to be 'Y's, but will practically always not be, except 1/256th of the time, so, they will be false negatives, so really, are like, less that 2%! Sure, the bytes may not be random so it wont be exactly that, and that 2% is basically made up of EVERY SINGLE SPECIAL MERCHANT WITH AN ID ABOVE 23, but, having special merchantsis discriminatory anyhow.

2006-09-19 Reply Admin

That's exactly backwards.

cout · 2006-09-19 Reply Admin

Elsewhere in the code we find this:

  #define not !!
  #define really !not
  #define truly !!really

  ...

  if(not is_special_merchant(24))
  {
    /* is a special merchant */
  }
  else if(really truly is_special_merchant(24))
  {
    /* is not a special merchant */
  }

ElectroDruid · 2006-09-19 Reply Admin

Hello,

I've been reading this site for aaages, but I don't post much. Just wanted to say that this is the most breathtaking, jaw dropping WTF I've seen here. I almost want to applaud Scott for his truly astonishing achievement in brokenness. I would accuse the post of being made up were it not for the fact that I don't think anyone CAN make up something so bad.

On top of all the other stuff mentioned so far, there's an interesting way in which the numbers don't add up. This is the most fun I've had with a calculator since I used to write rude words on it upside-down in school. If there are up to 10,000 merchants on the database, and ~2% of them are "special", you can have about 200 special merchants. Roughly. Bear with me. Even assuming the special merchants all fall in the range of 000* - 002*, that's only 23 special ones, or 0.23% (23/10000) of all merchants. And of those 23 in the file, only 7 are special, so the total would actually be 0.07% (7/1000). Or, if we're being kind and assuming (as Gryphon suggested) that he's MOD 24'ing the merchant ID before we enter the function, then something in the region of 30% (7/23) of the vendors will be being returned as special. And if the code "works" as it looks like it does, and it thinks it has a special vendor when a byte of memory happens to equate to 'Y', it'll do that ~0.39% (1/256) of the time for any merchant numbers > 22, which is almost all of them.

These are obviously rough approximations, depending on the actual size of the database, and how accurate the 2% expectation is, but any way you look at it, you're just getting silly amounts of results compared to what's expected.

Thankyou. I'd request more of this sort of thing, but I suspect that WTFs of this magnitude are somewhat rare.

Legendary Configuration

Leave a comment on “Legendary Configuration”