Comment On C SQL. C SQL Run. Run SQL Run.

Wow, that's like eight WTFs in one!

    1st!

3d

I wonder how many trials he went through to come up with the magic number 999999900...

1) The whole premise of writing to a file to do an SQL query.

2) Using one function to do three different tasks, and passing in an arbitrary, hard-coded ActionFlag to determine what to do.

3) Can anyone say "memory leak"?

4) The "C Cheat-Sheet" at the end.

5) The response variable.  First of all, it's assigned to and never used.

6) ...and second of all, is he really adding together two return values from the "system" function???  WTF?!

7) The two busy loops that "wait for database to return data."  I mean, seriously, where do they teach this stuff?

and finally...

8) It's not in Visual Basic.

Seriously, I could go on, but I need new goggles.

So, the other guy was bad compared to this guy? Um...

I suppose he was having time-out problems, so he copy-and-pasted another loop in?

(I love the C cheat-sheet.)

Well if the DB crashes and burns, at least they'll have the last resultset to start the rebuilding...

Alex Papadimoulis:

      strcpy (systemCall,"isql -U sa -P -S 192.168.3.118 -d ORGDB -n");

 

Good start; no sa password, hard coded IP address for server (though at least that would reduce DNS problems [<:o)]) - back to the old addage: those who can do, those who can't manage!

I worked with a guy once who tried that method of delaying the program from executing. He could never figure out why his code worked on some machines but not others.

Source of the problem: the computers it didn't work on had newer, faster processors that were executing his loops too quickly, not allowing the appropriate time to elapse.

Solution: Increase the number of loops it counts through. This means the faster computers work fine, the slower computers just got a little slower.

Looks like this guy did the same. Double the loops, half the IQ.

eddieboston:

1) The whole premise of writing to a file to do an SQL query.

2) Using one function to do three different tasks, and passing in an arbitrary, hard-coded ActionFlag to determine what to do.

3) Can anyone say "memory leak"?
...

Don't forget the buffer over run, should the lengths of the input and output filenames exceed 980 (or so).  Why do so many C coders think that 1024 is "big enough" for a temporary string?

Ugh...simply, ugh.

My eyes!  The goggles!  They malloc(), but free() nothing!

Alex Papadimoulis:

  // wait for database to return data.

  z = 0; for (x=0;x< 999999900;x++) {z++;} 

  z = 0; for (x=0;x< 999999900;x++) {z++;}



  return;

I guess they had to turn optimizations off, since the compiler could easily see that no code is dependent on variable z and would optimize out the looping "delay".  I used to use this kind of structure.  Granted, I was in elementary school at the time...

eddieboston:

1) The whole premise of writing to a file to do an SQL query.

2) Using one function to do three different tasks, and passing in an arbitrary, hard-coded ActionFlag to determine what to do.

3) Can anyone say "memory leak"?

4) The "C Cheat-Sheet" at the end.

5) The response variable.  First of all, it's assigned to and never used.

6) ...and second of all, is he really adding together two return values from the "system" function???  WTF?!

7) The two busy loops that "wait for database to return data."  I mean, seriously, where do they teach this stuff?

and finally...

8) It's not in Visual Basic.

Seriously, I could go on, but I need new goggles.

9) Executing DEL using system() to delete a file


10) The buffer overflow (which Grimoire mentioned)


11) The failure to escape spaces in the filename... if this is UNIX, he'd probably be able to get away with it, though.


12) The fact that it makes the Python/Perl programs that used popen() to run the command-line mysql client and execute commands look like a huge improvement.


13) The use of strcat over sprintf (I know, I'm scraping the bottom of the barrel here...)



I'm sure there are more...

Alex Papadimoulis:

// C Cheat-Sheet [from Alex]
//  strcpy(a,"somestr")   -->   a = "somestr"
//  strcat(b,"something") -->   b = b + "something"
//   (I haven't used C/C++ in nearly a decade, and I almost
//    forgot these myself)

 

C Cheat-Sheat [from Alex] seems to indicate that it was not part of the original code!

eddieboston:

7) The two busy loops that "wait for database to return data."  I mean, seriously, where do they teach this stuff?

Actually, he can probably get away with that in practice. You know why? system() waits for the command being executed to finish before returning. WTF?

Anonymous asshole 1:

1st!

Anonymous asshole 2:

3d

It would be unreasonable of me to assume that Alex has the time and energy to delete bullshit useless posts like these. I still feel compelled to throw that out there, hoping he'll do it. Here's a more realistic suggestion:

Give me the power. That's right, grant me appropriate privileges to get rid of such crap. I swear I won't delete anyone else's posts. Especially not Gene Wirchenko.

This is just perverse. And I like how he adds another cycle through incrementing x in the execute field of the for AND in the loop code increments z (whereas both could be done in the execute field of the for or in the looped code)

Wa wa waaaaa.  I think Marc should have been a little more tactful
before he opened his mouth -- although tact is certainly in short
supply among many developers, especially when confronted by code like
this.  I shudder to think what you would have to do to rank as a
"horrible coder" with the manager.

Manni:

Give me the power. That's right, grant me
appropriate privileges to get rid of such crap. I swear I won't delete
anyone else's posts. Especially not Gene Wirchenko.

(Insert obvious statement here)



Sincerely,



Ludvig Ericson (wonder if this is going to show up correctly)

Anonymous:

This is just perverse. And I like how he adds another cycle through incrementing x in the execute field of the for AND in the loop code increments z (whereas both could be done in the execute field of the for or in the looped code)

One increments z, the other x - they're different vars. The value of z is never actually used for anything (and it's initialised to 0 redundantly, too). Why are there *two* for loops, anyway?

Anonymous:

My eyes! The goggles! They malloc(), but free() nothing!

Actually, to be honest that's one of the lesser WTFs. A quick application of Valgrind or equivalent, some calls to free() in the correct places, and hey presto.

makomk:

eddieboston:

1) The whole premise of writing to a file to do an SQL query.

2) Using one function to do three different tasks, and passing in an arbitrary, hard-coded ActionFlag to determine what to do.

3) Can anyone say "memory leak"?

4) The "C Cheat-Sheet" at the end.

5) The response variable.  First of all, it's assigned to and never used.

6) ...and second of all, is he really adding together two return values from the "system" function???  WTF?!

7) The two busy loops that "wait for database to return data."  I mean, seriously, where do they teach this stuff?

and finally...

8) It's not in Visual Basic.

Seriously, I could go on, but I need new goggles.

9) Executing DEL using system() to delete a file


10) The buffer overflow (which Grimoire mentioned)


11) The failure to escape spaces in the filename... if this is UNIX, he'd probably be able to get away with it, though.


12) The fact that it makes the Python/Perl programs that used popen()
to run the command-line mysql client and execute commands look like a
huge improvement.


13) The use of strcat over sprintf (I know, I'm scraping the bottom of the barrel here...)



I'm sure there are more...

14) The name of the function. execSqlCommand - yet it can delete files which has nothing to do with the acctual SQL interfacing.



(another thing: When the CAPTCHA is in your autocomplete for the fifth time in a row, you know something isn't really working)

makomk:

Anonymous:

This is just perverse. And I like how he adds another cycle through incrementing x in the execute field of the for AND in the loop code increments z (whereas both could be done in the execute field of the for or in the looped code)

One increments z, the other x - they're different vars. The value of z is never actually used for anything (and it's initialised to 0 redundantly, too). Why are there *two* for loops, anyway?

(wonder if this is going to work... no, probably not)

Yeah, but he could as well just
for (;;x++,z++) {}
or just not increment z at all >_>

wintermyute:

Wa wa waaaaa.  I think Marc should have been a little more tactful
before he opened his mouth -- although tact is certainly in short
supply among many developers, especially when confronted by code like
this.  I shudder to think what you would have to do to rank as a
"horrible coder" with the manager.

I don't know.....it could be that the "horrible coder" and the manager were just as equally bad. I am currently working at a place where the manager is also a "coder" and he thinks that certain people that have left were just horrible. Well, breaking into the code you find that the manager and the ex-employees were about the same: just godawful.

Anyway, long story short, they were both probably pretty bad.

JJ

In other news, I never got that joke... anyone care to explain? Talking
about the DOS thingie which the title is a paraphrase on.

Anonymous:

makomk:

Anonymous:

This is just perverse. And I like how he adds another cycle through incrementing x in the execute field of the for AND in the loop code increments z (whereas both could be done in the execute field of the for or in the looped code)

One increments z, the other x - they're different vars. The value of z is never actually used for anything (and it's initialised to 0 redundantly, too). Why are there *two* for loops, anyway?

(wonder if this is going to work... no, probably not)

Yeah, but he could as well just
for (;;x++,z++) {}
or just not increment z at all >_>

I know; I misread the post. Sorry. I vote for not having z at all as being the sane solution. (Of course, this is The Daily WTF - if everyone went for the sane solution, we'd have nothing to mock).

It seems the boss was aware of security issues and tried to avoid buffer overflow with

  char * systemCall = malloc(sizeof(char) * 1024);

instead of

  char systemCall[1024];

This is hilarious. And the two loops are no better, didn't he know sleep/usleep? I bet he blamed Microsoft or something for server sluggishness...

Anonymous:

In other news, I never got that joke... anyone care to explain? Talking
about the DOS thingie which the title is a paraphrase on.

It's (apparently) a joke on a phrase from the Dick and Jane series of books, which used to be used to teach kids how to read, a long time ago. Now there's an area that's full of WTF...

Anonymous:

makomk:

Anonymous:

This is just perverse. And I like how he adds another cycle through incrementing x in the execute field of the for AND in the loop code increments z (whereas both could be done in the execute field of the for or in the looped code)

One increments z, the other x - they're different vars. The value of z is never actually used for anything (and it's initialised to 0 redundantly, too). Why are there *two* for loops, anyway?

(wonder if this is going to work... no, probably not)

Yeah, but he could as well just
for (;;x++,z++) {}
or just not increment z at all >_>

Actually, without anything happening in the body of the loop, the complier will generally optimize out the loop (as others mentioned above). 

So, here's something to ponder over...  The person who wrote this was a programmer who couldn't remember the functions of the standard CLIB string manipulation functions, but knew all about compiler optimization of looping code.  ...

Okay, even saying that hurts my brain! 

Alex Papadimoulis:

(NOTE: DNS Issues -- site may be up and down while they are resolved)

Ah... so that's why I could only get here via thedailywtf.com, not www.thedailywtf.com. Brillant!

Anonymous:

It seems the boss was aware of security issues and tried to avoid buffer overflow with

  char * systemCall = malloc(sizeof(char) * 1024);

instead of

  char systemCall[1024];

This is hilarious. And the two loops are no better, didn't he know sleep/usleep? I bet he blamed Microsoft or something for server sluggishness...

As I've said before, I'm not sure why they're needed; system() waits for the command to exit before it returns, so unless the database client forks and returns (which would be a whole new level of WTF in itself) or something weird is going on...

baldheadedguy:

Actually, without anything happening in the body of the loop, the complier will generally optimize out the loop (as others mentioned above).

So, here's something to ponder over... The person who wrote this was a programmer who couldn't remember the functions of the standard CLIB string manipulation functions, but knew all about compiler optimization of looping code. ...

Okay, even saying that hurts my brain!

Who says he needs to understand why it works? He could just have tried things until it did... (Besides, this probably wouldn't work on all compilers, at a guess.)

Anonymous:

In other news, I never got that joke... anyone care to explain? Talking
about the DOS thingie which the title is a paraphrase on.

From the Dick and Jane books:

See Spot.
See Spot run.
Run Spot run!

Spot is a dog, by the way.  That's all.

baldheadedguy:

Actually, without anything happening in the body of the loop, the complier will generally optimize out the loop (as others mentioned above). 

So, here's something to ponder over...  The person who wrote this was a programmer who couldn't remember the functions of the standard CLIB string manipulation functions, but knew all about compiler optimization of looping code.  ...

Okay, even saying that hurts my brain! 

Actually, he probably wrote the loop with no body, and found out it didn't delay at all, so tried a bunch of different things until he confused the compiler enough to not optimize it out.  Of course, that will be compiler dependent, so a newer/different compiler might (and should) optimize out the loops.  Then the manager can claim that the compiler is buggy, because the same code works fine in the old compiler.

Grimoire:

Actually, he probably wrote the loop with no body, and found out it didn't delay at all, so tried a bunch of different things until he confused the compiler enough to not optimize it out.  Of course, that will be compiler dependent, so a newer/different compiler might (and should) optimize out the loops.  Then the manager can claim that the compiler is buggy, because the same code works fine in the old compiler.

makomk, you beat me to it!  :D

Anonymous:

Manni:

Give me the power. That's right, grant me
appropriate privileges to get rid of such crap. I swear I won't delete
anyone else's posts. Especially not Gene Wirchenko.

(Insert obvious statement here)



Sincerely,



Ludvig Ericson (wonder if this is going to show up correctly)

Oh yeah?!?!  Well, (insert obvious retort here).

So THERE!

makomk:

Anonymous:

makomk:

Anonymous:

This is just perverse. And I like how he adds another cycle through incrementing x in the execute field of the for AND in the loop code increments z (whereas both could be done in the execute field of the for or in the looped code)

One increments z, the other x - they're different vars. The value of z is never actually used for anything (and it's initialised to 0 redundantly, too). Why are there *two* for loops, anyway?

(wonder if this is going to work... no, probably not)

Yeah, but he could as well just
for (;;x++,z++) {}
or just not increment z at all >_>

I know; I misread the post. Sorry. I vote for not having z at all as being the sane solution. (Of course, this is The Daily WTF - if everyone went for the sane solution, we'd have nothing to mock).

OH MY GOD!!

Are we REALLY debating the most efficient code to lock the CPU at 100% while LOOPING TO 2 BILLION.

Let's remember what we're doing here... If we're looping to 2 Billion, code efficiency gots nuttin to do with it!!

That's like arguing about,  um about... arguing about something really stupid when it doesn't really matter anyway.  (I never was good at analogies.)

Manni:

Anonymous asshole 1:

1st!

Anonymous asshole 2:

3d

It would be unreasonable of me to assume that Alex has the time and energy to delete bullshit useless posts like these. I still feel compelled to throw that out there, hoping he'll do it. Here's a more realistic suggestion:

Give me the power. That's right, grant me appropriate privileges to get rid of such crap. I swear I won't delete anyone else's posts. Especially not Gene Wirchenko.

You got my vote.
Manni for Supreme Benevolent Ruler!

marvin_rabbit:

OH MY GOD!!

Are we REALLY debating the most efficient code to lock the CPU at 100% while LOOPING TO 2 BILLION.

Let's remember what we're doing here... If we're looping to 2 Billion, code efficiency gots nuttin to do with it!!

That's like arguing about,  um about... arguing about something really stupid when it doesn't really matter anyway.  (I never was good at analogies.)

Gah - I've obviously been spending too long on here. I've come down with WTFitis - the ability to find ways of improving slightly on incredibly screwed-up code whilst ignoring the things that make it a true WTF. Be warned - soon you start doing it too. There's no hope left for you... buahahahaha!!!

eddieboston:

6) ...and second of all, is he really adding together two return values from the "system" function???  WTF?!

That's the lazy person's method of error checking. If a successful return code is zero, then success on both del's will also be zero. Anything else means a failure occurred, it just won't say where.

But just because I understand the purpose of it doesn't mean it's not wrong. It's still bad, bad, bad.

SQL client libraries are for LOSERS!!!

moron

    Yea... you know I'd create an account and all that just to help delete useless posts... this site is too cool to be filled with people posting that crap.

Chad

Otto:

If a successful return code is zero, then success on both del's will also be zero. Anything else means a failure occurred, it just won't say where.

Note if one returns +1 and one returns -1 you get a false success.

What cracks me up is -- did anyone consider, maybe the prior programmer WASN'T so bad... but he refused to use the 'best practices' of the Clearly Elite Boss?

Insincerely,

Voodoo C.

Anonymous:

It seems the boss was aware of security issues and tried to avoid buffer overflow with

  char * systemCall = malloc(sizeof(char) * 1024);

instead of

  char systemCall[1024];

Thats a bit of a sub-WTF right there as with C and C++, sizeof(char) is 1, by definition. So there never any point in using it.

But I'm guessing (from the quality of the code) that the boss really did not get the differnece between memory allocated on the heap and stack memory. Call it a hunch.

makomk:

13) The use of strcat over sprintf (I know, I'm scraping the bottom of the barrel here...)
...

Or strncat. 

Alex Papadimoulis:

// wait for database to return data.
z = 0; for (x=0;x< 999999900;x++) {z++;}
z = 0; for (x=0;x< 999999900;x++) {z++;}

Isn't this a tight loop... so won't this just always delay for the same amount of time, no matter if the DB returns or not?

voodooc:

What cracks me up is -- did anyone consider, maybe the prior programmer WASN'T so bad... but he refused to use the 'best practices' of the Clearly Elite Boss?
Insincerely,
Voodoo C.

It's clear: the prior programmer was really bad because he insisted on using Kernigan and Ritchie's brace style. This is a boss that knows what really counts as quality coding.

--Rank

Manni:

It would be unreasonable of me to assume that Alex has the time and energy to delete bullshit useless posts like these. I still feel compelled to throw that out there, hoping he'll do it. Here's a more realistic suggestion:

Give me the power. That's right, grant me appropriate privileges to get rid of such crap. I swear I won't delete anyone else's posts. Especially not Gene Wirchenko.

Here, here... Those posts are even more lame that brillant, istrue, and all the other "don't know when the joke has run its course" crap.

I would like to point out that this is C, not C++.
People keeping mixing obsolete and crappy C constructs with C++ already give a bad enough name to the language without people just putting the two in the same bag altogether.

One additional (albeit minor) WTF that no one seems to have noticed: his "systemCall" variable is not actually used for a system call.