Comment On Legendary Configuration

Alex Papadimoulis:

And for completeness, here is the spcmerch.cfg file in its entirety. All twenty three bytes.

NNNNYNNNYNNYYNYYNNNNYNN

 

WWWWWWWWTTTTTTTTTTTFFFFFFFFFFF!!!!!!!!!!!!!!!!!!! 

The the WTF is the name doesn't match the function? I understand that false in C is 0, and true is non-zero. So he returns true is the person in not a special merchant when the function is called is_special_merchant.

"The fact that this is still in production and provides completely random results is also a mystery."

 

How is a fact a mystery?

 Alex, you really need to proofread more. :)
 

Is it fair to say that he wasn't so much a legend in his own time as a legend in his own mind? Or is he a legend in the same way that Beowulf's Grendel is a legend?

(Hey, I almost *fist*!!)

 

First of all, welcome back Alex.

 
Secondly, what a wonderful WTF!!  Have you been saving this up for us?  The best part of this is that it's prolly called once for each transaction.  Absolutely glorious.

Hill

 

 

Hm... well, i really fail to see a WTF here. Fine, it reads no more than 10000 bytes, and it doesn't need to read that much to begin with. So ok... what else?

Haha. Nice.

And who'd have thought that we'd see the day when not having a FileNotFound return code might actually be a WTF?

Anonymous:

The the WTF is the name doesn't match the function? I understand that false in C is 0, and true is non-zero. So he returns true is the person in not a special merchant when the function is called is_special_merchant.

Follows UNIX-style exit codes maybe? I don't know, I'm stretching it a bit here. 

"Every merchant has a four-digit merchant number". Therefore, unless we assume all the four-digit merchant numbers start with 000 through 002, this function is consistently indexing into uninitialized chunks of array.
 

First, it's really enterprisey, brillant, works as coded, and all that other jazz.

Now.... 

Alex Papadimoulis:

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

Actually, it doesn't read in 10k bytes ... it only reads-in the 23 contained in the file.  However, this scheme would need to be redone if they ever have more than 10k merchants.  The concept isn't too bad of an approach, but the implementation missed the mark.

Alex Papadimoulis:

The fact that this is still in production and provides completely random results is also a mystery.

Um, this is where I think the WTF is for this code segment.  "Still in production" because it's just too enterprisey. :)  But the fact that it provides completely random results should be no mystery to a decent C/C++/{anything similar} programmer/software developer/Q&A tester.  I'll leave it to the reader to figure out why it's not a mystery.

[Note from Alex: this has since been rephrased for clarity]

Alex Papadimoulis:

int is_special_merchant(...)

I like how the return logic is negated. :P

(0 if special merchant and 1 if  not a special merchant )

 

so, he really should have done one of the following:

int might_not_be_a_special_merchant(...)

int special_merchant_not_found(...)

inline int paula_is_brillant(int true_or_false_or_file_not_found) { return("Brillant!"); }

 

 

Anonymous:

Hm... well, i really fail to see a WTF here. Fine, it reads no more than 10000 bytes, and it doesn't need to read that much to begin with. So ok... what else?

 He's reading beyond the end of the loaded file with any ID larger than the config, thus causing an undefined behaviour. If he wanted to guarantee he didn't cause it, he'd use

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

instead of 

if(fln == (size_t)0) return(1);
 

Anonymous:

Anonymous:

The the WTF is the name doesn't match the function? I understand that false in C is 0, and true is non-zero. So he returns true is the person in not a special merchant when the function is called is_special_merchant.

Follows UNIX-style exit codes maybe? I don't know, I'm stretching it a bit here. 

Yeah, seems off, because:

    if (is_special_merchant(myMerchant)) {

       //whatever

    }

will evaluate to true if the myMerchant is NOT a special merchant.

 

Not to mention 23 bytes for a 4-digit merchant code?  So what happens when myMerchant = 2423?

 

 

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. 

Alex Papadimoulis:

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, 10000, 1, file);

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

return(1);
}

The real WTF is that he declared a pointer for the file and didn't initialize it to null.

DigitalLogic:

The real WTF is that he declared a pointer for the file and didn't initialize it to null.

No, this approach is perfectly acceptable since he doesn't "use" the variable prior to it getting set appropriately later (in the call to fopen). 

shadowman:

Not to mention 23 bytes for a 4-digit merchant code?  So what happens when myMerchant = 2423?

Easy, since in C these are always offsets, it starts at the beginning of the array and moves 2423 bytes forward.  Who cares where the array ends.  If they value saved at that location is a Y then the merchant is a special case.  You can't control things outside of your defined bounderies so why allow access to them.

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.

Captcha: Fo' Shizzle

Anonymous:

DigitalLogic:

The real WTF is that he declared a pointer for the file and didn't initialize it to null.

No, this approach is perfectly acceptable since he doesn't "use" the variable prior to it getting set appropriately later (in the call to fopen). 

<sarcasm>
Sorry, I forgot my sarcasm tags
</sarcasm>

Well, let's see here.

1.  No attempt at returning any kind of error codes. If it can't open the file, then nobody is a special merchant.
2.  The same file is opened over and over again for every single line/transaction.  With caching I guess this isn't a HUGE deal, but still it's horribly inefficient.
3.  Any merchantnum > 22 will return 0 or 1 depending on what "garbage" is in the allocated memory.  Odds are heavy it will be 1, but if there just happens to be a "Y" there...
5.  ab is declared for no reason.  
6.  Any merchantnum > 10000 will just simply core dump/IPF depending on operation system.

Beautiful.
 

And given that there are several thousand merchants - who is volunteering to type in the configuration file? Never mind the implementation - the design is already a WTF.

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

 

And also, of course, if the file is empty he'll be leaving it open and leaking a filehandle.

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?

Another WTF right there. One could easily figure this out (at least what will happen as a result from a specific compiler/linker).

 

Not to mention that he only closes the file if everything succeeds.

if(fln == (size_t)0) return(1);

fclose(file);

looks like a problem to me.

 

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?

 

Anonymous:

The the WTF is the name doesn't match the function? I understand that false in C is 0, and true is non-zero. So he returns true is the person in not a special merchant when the function is called is_special_merchant.

That's *one* of the WTFs here.

Here are some others:

• The file is opened, read, and closed for *each* record being processed.
• The fread only initializes the first 23 bytes of the array, the rest (which are almost certainly indexed) contains random trash (local variables are not automatically zeroed out in C).
• If the file is not found, the function returns 1, so the caller will assume that it's (or isn't - depending on whether the caller knows this is named wrong) a special merchant. Correct behavior would be an error code which the caller could use to send an email to a support guy and flag the record as "process later".
• 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.
  

Anonymous:

1.  No attempt at returning any kind of error codes. If it can't open the file, then nobody is a special merchant.

Not necessarily an invalid assumption.  He wrote the function, so he gets to define the assumptions too.

Anonymous:

2.  ...
3.  ...
5.  ab is declared for no reason.

um, 4. .... ?? profit ??

I think "ab" comes from a cut-and-paste snippet from some earlier code where he was doing something similar and needed a variable named "ab" to represent the absolute merchant number for those odd merchants with a negative number. :P 

Anonymous:

6.  Any merchantnum > 10000 will just simply core dump/IPF depending on operation system.

or simply get garbage (i.e., see #3) 

Lot of fun if the file is empty, it won't get closed.

Anonymous:

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?

Another WTF right there. One could easily figure this out (at least what will happen as a result from a specific compiler/linker).

Since he's allocating the 10000-element array on the stack, the processor will make sure that array never extends beyond the end of the physical memory space:  if the stack is blown like that, you'll almost certainly get a processor interrupt and the OS will kill the process.  But he could create a 10000-element array on the stack and then get a reference to the 1,000,000th element of it.  If the processor had hardware memory management, that'd probably trigger an interrupt that'd make the OS kill the process; if it didn't, my guess is that it'd wrap around into low memory and read something from the OS code.

Yeah, you can just keep on poining out problems with this function.  Reminds me of those "kiddy" exercises you find at the back of the comics pages in the local newspaper.  "How many things can you spot wrong with this function?"

Anonymous:

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?

Another WTF right there. One could easily figure this out (at least what will happen as a result from a specific compiler/linker).

Um, why does one need to figure it out?  It's fairly understood. 

Anonymous:

um, 4. .... ?? profit ??

I think "ab" comes from a cut-and-paste snippet from some earlier code where he was doing something similar and needed a variable named "ab" to represent the absolute merchant number for those odd merchants with a negative number. :P 

Whoops.  I pointed out something else and deleted it.  But forgot to update the sequence.  Hasty posting will get you every time.

Anonymous:

6.  Any merchantnum > 10000 will just simply core dump/IPF depending on operation system.

or simply get garbage (i.e., see #3) 

Yeup..you're right.  I forgot that if you're LUCKY you will get a coredump in C.  The 10,001st element, for instance, may in fact give you the value of one of the stack variables...

Anonymous:

Anonymous:

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?

Another WTF right there. One could easily figure this out (at least what will happen as a result from a specific compiler/linker).

Um, why does one need to figure it out?  It's fairly understood. 

While I don't generally assume, I assumed he meant something along the lines of:

Address space is 0x00000000 - 0xFFFFFFFF, and you have a void* ptr pointing to address 0xFFFFFF00 and try to access ptr+0x100.

Or possibly, what would happen if address 0xC8000000-0xCFFFFFFF were reserved for some physical device not currently present in the system and tried to access it (accidently through pointer arithmetic) or otherwise.

Regardless, to answer *your* question "why does one need to figure it out?" ... because apparently some "ones" don't know ... there are various methods to figure it out .. one, trial and error and two, scanning google/wikipedia/{your favorite means of standing on the shoulders of those before you}.
 

Anonymous:

Anonymous:

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?

Another WTF right there. One could easily figure this out (at least what will happen as a result from a specific compiler/linker).

Um, why does one need to figure it out?  It's fairly understood. 

Obviously my question is not easily understood.  Your explainations are to the idea that there si still memory locations to access, and yes that is understood.  But what if we have filled our physical memory almost completly. 

You have 1 gig worth of memory, you fill all but 10k of it, you then create an array that fills 9k.  You attempt to access a location 9.5k past the end, no problem, buffer overrun. 

But try to access something 11k past the end.  Herin lies my question.  Since there is no physical address what happens?  A pure core dump?  A wrap to the beginning for system memory (which I doubt)? Or something else?  This is totally undefined behaviour based off my knowledge. 

Remember C tends to believe that the programmer knows what he is doing and will attempt it regardless of the ramifications.  Some other languages (vb) notice this access and simply raises an out of bounds error.

Alex Papadimoulis:

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

The above code is so object-oriented, I almost OOPed my pants.

Alex Papadimoulis:

Only two percent of merchants require this and these merchants are identified in the Special Merchants Configuration file ("spcmerch.cfg").

And for completeness, here is the spcmerch.cfg file in its entirety. All twenty three bytes.

NNNNYNNNYNNYYNYYNNNNYNN

Trivial wtf next to the rest, but 7 Y in 23 charaters is 30.4%. And isn't he lucky the Y's are all within the first 23 merchant id's, 0000 thru 0022.

Also, if merchants are 4 digit the max is 9999, and indexing won't overflow the 10000 array length.

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

Anonymous:

shadowman:

Not to mention 23 bytes for a 4-digit merchant code?  So what happens when myMerchant = 2423?

Easy, since in C these are always offsets, it starts at the beginning of the array and moves 2423 bytes forward.  Who cares where the array ends.  If they value saved at that location is a Y then the merchant is a special case.  You can't control things outside of your defined bounderies so why allow access to them.

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.

Captcha: Fo' Shizzle

That would likely cause a memory-access-violation.

However, I believe that this legend also had another function that just happened, by design, to have 10K worth of Y's and N's on the stack, just before this function gets called (every time), so that when it indexes off the end of the array, the data is valid, albeit by chance (I've actually seen this done in production code, and when confronted, the developer said that it worked, so why bother changing it)

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.
 

Did no-one else catch the other WTF here?

 if ( ! is_special_mercant(merc_id) ){

    // DO SPECIAL MERCH STUFF 

}
 

 He returns 0 for success and 1 for failure!!! WTF?!
 

Good catches. 

Anonymous:

3.  Any merchantnum > 22 will return 0 or 1 depending on what "garbage" is in the allocated memory.  Odds are heavy it will be 1, but if there just happens to be a "Y" there...

He probably had an accompanying utility function to handle this case:

void init_stack_for_is_special_merchant() {

  char stackData[20000];

  int i;

  for(i = 0; i < 20000; ++ i) stackData[i] = 'Y';

} 

:) 

Anonymous:

6.  Any merchantnum > 10000 will just simply core dump/IPF depending on operation system.

This isn't really a problem if the merchant ID is a 4-digit number like the problem statement says it is.  Of course, it would be a problem if they ever needed more digits, but, judging from the style, this constraint is probably hard-coded in many more places than just this function. :)

You got all the errors I could see, except for the leaked file handle if the file is empty.
 

Anonymous:

Anonymous:

The the WTF is the name doesn't match the function? I understand that false in C is 0, and true is non-zero. So he returns true is the person in not a special merchant when the function is called is_special_merchant.

That's *one* of the WTFs here. Here are some others:

• The file is opened, read, and closed for *each* record being processed.
• The fread only initializes the first 23 bytes of the array, the rest (which are almost certainly indexed) contains random trash (local variables are not automatically zeroed out in C).
• If the file is not found, the function returns 1, so the caller will assume that it's (or isn't - depending on whether the caller knows this is named wrong) a special merchant. Correct behavior would be an error code which the caller could use to send an email to a support guy and flag the record as "process later".
• 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.

Granted, that's one screwed up way of doing it, but a 3 line shell script could be used to create the file, then edit in vi, then enter: 3048 <right arrow> rY<esc> (not that you'd want to do it that way, but you could)

 

In the 70's speed a space were such an importance...i mean i'm not defending the guy but:

If you knew that all the special merchants would be near the front, this is actually pretty fast, and for his time-frame that's pretty configurable to have a file like that. You have to assume the merchants don't change that much, i mean it's in a flat file after all.

His only wtf was the char[10000] part, which is what gives you random data. the fread doesn't jsut read random data past the end of file marker, but the char array will already have the random data in there.  What he should have done was check to see if merch_code > filesize and return not special for that case.

 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.

obviously a layer above this for cacheing would even be better, but you guys are all assuming that this function is called multiple times per an execution pool. For all you know this is a CGI that is activated every time a request comes in, meaning there is nowhere to cache results.

DigitalLogic:

The real WTF is that he declared a pointer for the file and didn't initialize it to null.

I actually format my pointers before I use them. Not quick format, but the Real Thing! I also double check that the pointer's master boot record is in place, and run S.M.A.R.T. to ensure that it doesn't have any bad sectors or sumfin. You can never not know.

Anonymous:

Alex Papadimoulis:

Only two percent of merchants require this and these merchants are identified in the Special Merchants Configuration file ("spcmerch.cfg").

And for completeness, here is the spcmerch.cfg file in its entirety. All twenty three bytes.

NNNNYNNNYNNYYNYYNNNNYNN

Trivial wtf next to the rest, but 7 Y in 23 charaters is 30.4%. And isn't he lucky the Y's are all within the first 23 merchant id's, 0000 thru 0022.

Also, if merchants are 4 digit the max is 9999, and indexing won't overflow the 10000 array length.

Not necessarily; you are thinking in decimal. This guy is a legend in his own mind - maybe he thinks in hex, so 4 digits would be: 0..63K-1</smirk>

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.

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

Anonymous:

shadowman:

Not to mention 23 bytes for a 4-digit merchant code?  So what happens when myMerchant = 2423?

Easy, since in C these are always offsets, it starts at the beginning of the array and moves 2423 bytes forward.  Who cares where the array ends.  If they value saved at that location is a Y then the merchant is a special case.  You can't control things outside of your defined bounderies so why allow access to them.

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.

Captcha: Fo' Shizzle

It will cause a perfectly reliable segfault. (Although it's the address space allocated for the process and not the physical memory that counts )
 

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.

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.

 Yeah, if you didn't catch what i was hinting at with random data: since he doesn't check that the merchant code is greater than the file length, (or 9999), he could pass back "random" data.... will most likely be 1's.
 

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.

  

 

 

shit... good catch.