Coding Horrors: You can gather all your technical debt into one easy framework!

The Something Awful Forums > Discussion > Serious Hardware/Software Crap > The Cavern of COBOL > Coding Horrors: You can gather all your technical debt into one easy framework!

Smackbilly: Jan 3, 2001; What kind of a name is Pizza Organ! anyway?

theg sprank posted:

I found these right next to each other in some code of mine today

code:

#define SIBLING(node) (((node)==((node)->parent->left)) ?\
                       ((node)->parent->right) : ((node)->parent->left))
#define GET_SIBLING(node) ((node) == (node)->parent->right ?\
			   (node)->parent->left : (node)->parent->right)

no idea what I was thinking

That's terrible not only for the redundancy but also for the fact that it sefaults your program if either node or parent is NULL.

# ¿ May 13, 2008 00:16

Adbot: ADBOT LOVES YOU

# ¿ Apr 29, 2024 14:36

Smackbilly: Jan 3, 2001; What kind of a name is Pizza Organ! anyway?

tef posted:

Here is a bit of a security coding horror:

http://lists.debian.org/debian-security-announce/2008/msg00152.html

Here is the fix: http://svn.debian.org/wsvn/pkg-openssl/openssl/trunk/crypto/rand/md_rand.c?op=diff&rev=300&sc=1

(Yes, it is just adding in a line that was commented out.)

And why did this happen? http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=363516

They wanted to get rid of warning messages caused by valgrind.

At this point I think this says it best:

Maybe I'm just not understanding the code, but after giving it a brief glance it seems like the "correct" code is using the fact that uninitialized variables are not guaranteed to have any particular value as a source of entropy. Isn't that hugely non-portable? Is there anything the C spec that says that the compiler cannot zero out uninitialized variables? I know it doesn't have to, but is it forbidden to? If not, isn't the "correct" code insecure on any compiler which chooses to act this way?

# ¿ May 13, 2008 15:19

Smackbilly: Jan 3, 2001; What kind of a name is Pizza Organ! anyway?

TSDK posted:

And that's a second WTF right there. Compilers are not stupid. Only people who think compilers are stupid, are stupid.

Well, the admonition not to fix warnings without understanding the code is valid - in fact you shouldn't do anything to code that you don't understand. I think however that the better statement is not that the compiler is "stupid" but that it is conservative.

For example, consider

code:

if (x = y) {
 // stuff
}

Many compilers will emit a warning for this, because they can't possibly know if you made a typo, or really wanted to assign y to x and then test the value of x for truth. There's a way to get around this of course (extra parens) but that's an extra bit of otherwise meaningless code that exists only to placate the compiler. So in this sense the compiler is "stupid" because it can't tell what the code is logically supposed to be doing like a human programmer probably could. But really it's just being conservative and assuming that code that looks like a mistake is a mistake. Similarly, it could not tell that the programmer really did mean to use uninitialized data in the previous example, so it assumed it was a mistake.

# ¿ May 13, 2008 16:00

Smackbilly: Jan 3, 2001; What kind of a name is Pizza Organ! anyway?

TSDK posted:

I also disagree that you need to have a full and complete understanding of anything before coding or bug fixing. Often the only way to get to fully understand systems is when you actually go through it in practice trying to change or implement something. It's also perfectly possible to do an adequate job fixing bugs without understanding all of the surrounding code.

I wasn't trying to say you do but you at least should understand the part of the code that you're fixing/changing, which it appears that this person didn't.

# ¿ May 13, 2008 17:20

Smackbilly: Jan 3, 2001; What kind of a name is Pizza Organ! anyway?

rotor posted:

No way. I want intersection/union glyphs. I've been typing .contains and .concat for 12 goddamn years like some kind of loving caveman. It's 2008, let's start acting like it people.

Agreed. We don't need a full whacko APL keyboard, but it would be pretty nice to at least have characters for basic set operations (intersect, union, subset, superset), and a single glyph for -> on normal keyboards. I'd even be happy with settling for a bigraph for subset-or-equal and superset-or-equal.

# ¿ May 29, 2008 22:59

Smackbilly: Jan 3, 2001; What kind of a name is Pizza Organ! anyway?

Chain Chomp posted:

It would look like this I guess

code:


..........  Click000      Click001       Click002 ...
Visitor000  index.php     NULL           NULL
Visitor001  index.php     contact.php    products.php
Visitor002  products.php  index.php      NULL
.
.
.

substitute real links for the above

:barf:

Not only is that a retarded DB design, it also is completely useless. Nothing that a database can do that a spreadsheet (or hell a flat text file) cannot do is applicable to that layout.

# ¿ Jun 7, 2008 00:11

Smackbilly: Jan 3, 2001; What kind of a name is Pizza Organ! anyway?

Zombywuf posted:

On the subject of exception based control flow:

code:

class A {
  ...
};

class a : public A {
  ...
};

class b : public A {
  ...
};

void dispatch(const A &obj) {
  try {
    throw obj;
  } catch (a &a_obj) {
    do_stuff_with_a(a_obj);
  } catch (b &b_obj) {
    do_stuff_with_b(b_obj);
  }
}

haha oh god... at least that gets creativity points. Who needs RTTI when you have exceptions?

# ¿ Jun 18, 2008 14:36

Smackbilly: Jan 3, 2001; What kind of a name is Pizza Organ! anyway?

JoeNotCharles posted:

Your way is better - you don't want people to be able to put 'os.system("rm -rf /")' in your config file and have it blindly executed.

Eh? As long as it's not running suid or something, there's nothing a user can put in his config file that he couldn't just type into the commandline. It's not a program's job to stop a user from doing something stupid when they go out of their way to do it.

# ¿ Jul 10, 2008 15:55

Smackbilly: Jan 3, 2001; What kind of a name is Pizza Organ! anyway?

JoeNotCharles posted:

What if somebody wants to install it with a web server frontend? Or some malicious person sends a complex config file saying, "Here, this'll do exactly what you're looking for!" when it actually has some bad commands embedded in it? Blindly executing anything handed to you is a terrible habit to get into.

If you're allowing untrusted web users (or untrusted ANYONE) to make arbitrary changes to the configuration file for one of your apps, you're pretty hosed regardless.

I could send someone a .emacs config file that wipes their hard drive (or at least their home directory), but that's not considered to be security problem with emacs.

# ¿ Jul 10, 2008 18:15

Smackbilly: Jan 3, 2001; What kind of a name is Pizza Organ! anyway?

Save the whales posted:

code:

  static String[] tens = { "ten", "twenty", 
    "thirty", [b]"fourty"[/b], "fifty", "sixty", "seventy", 
    "eighty", "ninety" };

Bug report. Severity: Critical.

# ¿ Jul 12, 2008 06:04

Smackbilly: Jan 3, 2001; What kind of a name is Pizza Organ! anyway?

Came across this while debugging someone else's code at work this week:

code:

int FromOctString(char *octnumber)
{
        char chdigit[1]; int iret; char octnum[3];
        strcpy(octnum, octnumber);
        sprintf(chdigit, "%c", octnum[0]);
        iret = atoi(chdigit) * 64;
        sprintf(chdigit, "%c", octnum[1]);
        iret += atoi(chdigit) * 8;
        sprintf(chdigit, "%c", octnum[2]);
        iret += atoi(chdigit);
        return iret;
}

How is this wrong? oh let me count the ways...

1. Even ignoring the other bugs, it only "works" for octal numbers that are 2 or 3 digits long (and 2 only by accident). If you pass it a 1 or 4+ digit number, the results are non-deterministic. In context, this function is not going to receive any 4+ digit numbers, but a 1-digit number is completely possible.

2. The author apparently forgot that C strings are null-terminated, leading to:

2a. The strcpy call will write past the storage of octnum if octnumber is 3 or more digits.

2b. Every call to sprintf will write past the storage of chdigit

3. There's no const-correctness on the parameter

4. The entire goddamned function can be replaced by a single call to strtol.

I replaced it with strtol.

Smackbilly fucked around with this message at 13:52 on Jan 11, 2009

# ¿ Jan 11, 2009 13:50

Smackbilly: Jan 3, 2001; What kind of a name is Pizza Organ! anyway?

awesmoe posted:

Doesn't calling erase on a container invalidate all iterators into that container? (I'm asking, here - I'm not certain). If so, the change made doesn't fix the problem anyway.

For some containers it does, for others it does not. For example: map does not invalidate all iterators on erasure, but deque does.

# ¿ Jan 17, 2009 03:21

Smackbilly: Jan 3, 2001; What kind of a name is Pizza Organ! anyway?

Flobbster posted:

STL algorithms. Use 'em, motherfuckers

Can't wait for C++0x lambda functions to make STL predicates so much less a pain in the rear end.

# ¿ Jan 17, 2009 14:12

Smackbilly: Jan 3, 2001; What kind of a name is Pizza Organ! anyway?

Otto Skorzeny posted:

Here also note the use of 'auto' for compile-time type inference, which really comes into its own in situations like
code:
map<string, const some_weird_fucken_type<did_i_mention_its_a_container_too_have_fun_gettingthe_right_iterator>> m;
for (auto i: m){
    ...
}

Also C++0x will finally make >> in a template context not be a right shift operator, thus eliminating the syntax error that the above code would have had in current C++. Thank god.

# ¿ Jan 18, 2009 14:14

Smackbilly: Jan 3, 2001; What kind of a name is Pizza Organ! anyway?

Flobbster posted:

Smackbilly is correct, but at the same time erase is still guaranteed to return a valid iterator to the element after the one being erased, regardless of whether the container's semantics invalidate other iterators on erasure.

I didn't know this off the top of my head but I just discovered that this is not actually true. The erase() method in associative containers (such as std::set and std::map) returns void. It neither invalidates all iterators nor returns an iterator. Only sequence containers return an iterator from erase().

On the one hand this is annoying because it is inconsistent, but on the other hand, advancing an iterator in an associative container may take non-constant time, and advancing the iterator is not necessarily required to perform the erasure, so I suppose this was done for efficiency reasons.

# ¿ Jan 19, 2009 03:26

Smackbilly: Jan 3, 2001; What kind of a name is Pizza Organ! anyway?

ColdPie posted:

I remember seeing this in my first month on the job.
code:
class SomeClass {
  public void doSomething(String flag){
    if(flag.equals("YES")){
      //do something
    }else{
      //do something else
    }
  }

  public void elsewhere(){
    doSomething("YES");
  }
}
Still see it pop up here and there when I work on old code.

I got something almost exactly like this (except in C, not Java) in code written by someone senior to me a few months ago. It was doubly odd because aside from that the code was very good and WTF-free.

# ¿ Jan 24, 2009 15:09

Adbot: ADBOT LOVES YOU

# ¿ Apr 29, 2024 14:36

Smackbilly: Jan 3, 2001; What kind of a name is Pizza Organ! anyway?

Ugg boots posted:

I mentioned this in IRC as it was playing out, but I'm sitting in a Computer Science majors computer lab, and there are three guys sitting next to me working on a Perl assignment. I know for a fact they're not freshmen or sophomores but even if they were, ugh.

So, they have to take input into their program and want to validate and make sure the user enters in a number (as opposed to something else.)

Their solution?

Store the input in a temp variable. Multiply the temp variable by 1. Make sure the temp variable is the same as the input.

Also this quote "You could also use a regular expression but that'd be a pain in the rear end."

Ugh.

They're missing the part where the human is supposed to do the thinking and planning, and the computer is supposed to do the simple things as quickly as possible. They're trying to have the humans do simple things as quickly as possible without the thinking and planning, and it doesn't really work well that way.

# ¿ Jan 27, 2009 03:04

The Something Awful Forums > Discussion > Serious Hardware/Software Crap > The Cavern of COBOL > Coding Horrors: You can gather all your technical debt into one easy framework!