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!

«‹›1503 »

zokie: Feb 13, 2006; Out of many, Sweden

Munkeymon posted:

Because the charity that maintains it is called Legion of the Bouncy Castle, obviously:

At least they didn't pick some godawful 1337 poo poo name, I guess As Volmaris pointed out, being overt about having a sense of humor is better than confirming you have none, but I'm not crazy about the name they picked, either.

In May I start at a firm called literally 1337, but they're chill. :colbert:

# ? Apr 14, 2014 06:32

Adbot: ADBOT LOVES YOU

# ? May 17, 2024 18:26

necrotic: Aug 2, 2005; I owe my brother big time for this!

More fun code from the project I work on: we have some tests around our image processing (not the actual processing, just that things get called and files created). Well, the person who wrote the test decide to stub a couple methods on all instances of the String class which ended up causing ~250mb of the string "Image" repeating for each test in the context (which was about 5). Needless to say, this caused issues with Jenkins crashing and forced us to delay a couple deploys because our build system kept crashing.

# ? Apr 14, 2014 17:49

Alien Arcana: Feb 14, 2012; You're related to soup, Admiral.

zokie posted:

In May I start at a firm called literally 1337, but they're chill.

Ask them what happened to the previous 1,336 firms.

# ? Apr 14, 2014 19:53

JawnV6: Jul 4, 2004; So hot ...

Alien Arcana posted:

Ask them what happened to the previous 1,336 firms.

They have not failed. They've just found 1,336 ways that don't work.

# ? Apr 14, 2014 19:56

Doc Hawkins: Jun 15, 2010; Dashing? But I'm not even moving!

We call it Continuous Business Model Deployment.

# ? Apr 14, 2014 22:26

gonadic io: Feb 16, 2011; >>=

code:

bool VertInfluencedByActiveBone(
  FParticleEmitterInstance* Owner,
  USkeletalMeshComponent* InSkelMeshComponent,
  int32 InVertexIndex,
  int32* OutBoneIndex = NULL);

void UParticleModuleLocationSkelVertSurface::Spawn(....)
{
  ....
  int32 BoneIndex1, BoneIndex2, BoneIndex3;
  BoneIndex1 = BoneIndex2 = BoneIndex3 = INDEX_NONE;

  if(!VertInfluencedByActiveBone(
        Owner, SourceComponent, VertIndex[0], &BoneIndex1) &&
     !VertInfluencedByActiveBone(
        Owner, SourceComponent, VertIndex[1], &BoneIndex2) && 
     !VertInfluencedByActiveBone(
        Owner, SourceComponent, VertIndex[2]) &BoneIndex3)
  {
  ....
}

(from here)

Look carefully at the ampersand on the last line.

# ? Apr 15, 2014 01:18

Suspicious Dish: Sep 24, 2011; 2020 is the year of linux on the desktop, bro; Fun Shoe

I found a bug in GNU bash, so I went to go check out the source code. Huh...

# ? Apr 15, 2014 04:34

pseudorandom name: May 6, 2007

Suspicious Dish posted:

I found a bug in GNU bash, so I went to go check out the source code. Huh...

Take a look at http://git.savannah.gnu.org/cgit/bash.git/commit/?id=ac50fbac377e32b98d2de396f016ea81e8ee9961 to understand the horror.

# ? Apr 15, 2014 08:55

hackbunny: Jul 22, 2007; I haven't been on SA for years but the person who gave me my previous av as a joke felt guilty for doing so and decided to get me a non-shitty av

pseudorandom name posted:

Take a look at http://git.savannah.gnu.org/cgit/bash.git/commit/?id=ac50fbac377e32b98d2de396f016ea81e8ee9961 to understand the horror.

I'm not sure I get it? Is it a huge monolithic commit that's not a merge?

# ? Apr 15, 2014 09:11

pseudorandom name: May 6, 2007

hackbunny posted:

I'm not sure I get it? Is it a huge monolithic commit that's not a merge?

No, its the 45 undocumented commits over a two year period followed by a single commit containing the changelog.

# ? Apr 15, 2014 09:41

canis minor: May 4, 2011

AlsoD posted:

code:

bool VertInfluencedByActiveBone(
  FParticleEmitterInstance* Owner,
  USkeletalMeshComponent* InSkelMeshComponent,
  int32 InVertexIndex,
  int32* OutBoneIndex = NULL);

void UParticleModuleLocationSkelVertSurface::Spawn(....)
{
  ....
  int32 BoneIndex1, BoneIndex2, BoneIndex3;
  BoneIndex1 = BoneIndex2 = BoneIndex3 = INDEX_NONE;

  if(!VertInfluencedByActiveBone(
        Owner, SourceComponent, VertIndex[0], &BoneIndex1) &&
     !VertInfluencedByActiveBone(
        Owner, SourceComponent, VertIndex[1], &BoneIndex2) && 
     !VertInfluencedByActiveBone(
        Owner, SourceComponent, VertIndex[2]) &TBoneIndex3)
  {
  ....
}

(from here)

Look carefully at the ampersand on the last line.

This one is nasty, but entire article deserves a read. It surely would give you warnings though, no?

# ? Apr 15, 2014 11:50

Blotto Skorzany: Nov 7, 2008; He's a PSoC, loose and runnin'
came the whisper from each lip
And he's here to do some business with
the bad ADC on his chip
bad ADC on his chiiiiip

pseudorandom name posted:

No, its the 45 undocumented commits over a two year period followed by a single commit containing the changelog.

Also the fact that the repository makes frankly bizarre assumptions as to where the source lives

code:

@@ -2106,7 +2106,7 @@ yyreduce:
     break;
 
   case 3:
-#line 388 "/Users/chet/src/bash/src/parse.y"
+#line 388 "/usr/src/local/chet/src/bash/bash-4.2-patched/parse.y"
     {
 			  /* Case of regular command, but not a very
 			     interesting one.  Return a NULL command. */

etc

# ? Apr 15, 2014 13:16

Athas: Aug 6, 2007; fuck that joker

Otto Skorzeny posted:

Also the fact that the repository makes frankly bizarre assumptions as to where the source lives

code:

@@ -2106,7 +2106,7 @@ yyreduce:
     break;
 
   case 3:
-#line 388 "/Users/chet/src/bash/src/parse.y"
+#line 388 "/usr/src/local/chet/src/bash/bash-4.2-patched/parse.y"
     {
 			  /* Case of regular command, but not a very
 			     interesting one.  Return a NULL command. */

etc

That is generated code.

# ? Apr 15, 2014 13:19

Blotto Skorzany: Nov 7, 2008; He's a PSoC, loose and runnin'
came the whisper from each lip
And he's here to do some business with
the bad ADC on his chip
bad ADC on his chiiiiip

Athas posted:

That is generated code.

Yacc dumbs absolute paths in the comments of its generated code?

# ? Apr 15, 2014 13:26

nielsm: Jun 1, 2009

Otto Skorzeny posted:

Yacc dumbs absolute paths in the comments of its generated code?

You're not supposed to distribute Yacc-generated code anyway, it's just an intermediate compilation step.

# ? Apr 15, 2014 13:52

Subjunctive: Sep 12, 2006; ✨sparkle and shine✨

Otto Skorzeny posted:

Yacc dumbs absolute paths in the comments of its generated code?

Yep, just like cpp.

Edit: I've made so many of the errors in that UE4 analysis, and so many of them have made it into production... :negative:

Subjunctive fucked around with this message at 15:05 on Apr 15, 2014

# ? Apr 15, 2014 14:39

Fuck them: Jan 21, 2011; and their bullshit

$CLIENT's database devs: "We want our own dev server so the eventual end users who are not programmers or designers can play around with and bikeshed our case management system."

$CLIENT: "Sure. Eventually. Whatever."

$CLIENT's database devs: "Oh can we use the sandbox environment currently in use by the $CONTRACTOR to develop our system? We never got that server"

$CLIENT: "Sure, whatever."

US: "Would you like is to twiddle our thumbs but keep billing you almost six figures a month?"

:haw:

I should add that about half a year ago someone fat fingered, spilled beer, or quit in an actionable manner; they managed to delete all of our development VMs and a lot of other VMs, and most of the backups, and I think even our TFS server. Our local copies saved a lot of asses.

Nevertheless, their tables are so normalized it's a bit of a breeze to work with them. They just kind of suck at the whole backups and best practices thing.

# ? Apr 15, 2014 15:13

Zopotantor: Feb 24, 2013; ...und ist er drin dann lassen wir ihn niemals wieder raus...

Something I found in our codebase today, slightly paraphrased:

code:

FILE *pFile = 0;
int someValueToBeLogged = ...;

...

void logStuff(const char *const message)
{
    char buffer[1024];
    if (pFile) {
        snprintf(buffer, 1024, "%s: %d", message, someValueToBeLogged);
        fprintf(pFile, buffer);
    }
}

Props for using snprintf instead of sprintf, but still... :bang:

# ? Apr 15, 2014 17:31

Dessert Rose: May 17, 2004; awoken in control of a lucid deep dream...

Apart from the use of globals, what's really wrong with that code?

# ? Apr 15, 2014 18:32

Blotto Skorzany: Nov 7, 2008; He's a PSoC, loose and runnin'
came the whisper from each lip
And he's here to do some business with
the bad ADC on his chip
bad ADC on his chiiiiip

Dessert Rose posted:

Apart from the use of globals, what's really wrong with that code?

Look at the function signature of fprintf(3), then look at how he's calling it, then look at what he's doing with snprintf and the fixed buffer.

e: i can't use spoiler tags properly

code:

snprintf(buffer, 1024, "%s: %d", message, someValueToBeLogged);
fprintf(pFile, buffer);

should instead be

code:

fprintf(pFile, "%s: %d", message, someValueToBeLogged);

or at least

code:

snprintf(buffer, 1024, "%s: %d", message, someValueToBeLogged);
fprintf(pFile, "%1024s", buffer);

Blotto Skorzany fucked around with this message at 18:41 on Apr 15, 2014

# ? Apr 15, 2014 18:38

Volmarias: Dec 31, 2002; EMAIL... THE INTERNET... SEARCH ENGINES...

Dessert Rose posted:

Apart from the use of globals, what's really wrong with that code?

Not using sizeof for the buffer

No guarantee that the message will fit in the buffer

Not using fprintf :frog:

# ? Apr 15, 2014 18:38

tractor fanatic: Sep 9, 2005; Pillbug

logStuff("%s");

# ? Apr 15, 2014 18:40

Dessert Rose: May 17, 2004; awoken in control of a lucid deep dream...

Somehow I missed that nagging voice in the back of my head "doesn't fprintf do the same thing as all the other printfs...?"

tractor fanatic posted:

logStuff("%s");

This is the best part, because I bet the original author was like, "I know, I'll be extra secure with snprintf!"

# ? Apr 15, 2014 18:43

The_Franz: Aug 8, 2003

Volmarias posted:

Not using sizeof for the buffer

No guarantee that the message will fit in the buffer

Not using fprintf

Plus, unlike POSIX platforms, snprintf on Windows doesn't guarantee null-termiantion so fprintf may not stop at the end of the buffer if the message is too long.

# ? Apr 15, 2014 18:44

necrotic: Aug 2, 2005; I owe my brother big time for this!

eithedog posted:

It surely would give you warnings though, no?

One of the tools the article author used did:

quote:

PVS-Studio's diagnostic message: V564 The '&' operator is applied to bool type value. You've probably forgotten to include parentheses or intended to use the '&&' operator. particlemodules_location.cpp 2120

# ? Apr 15, 2014 19:30

canis minor: May 4, 2011

necrotic posted:

One of the tools the article author used did:

Yes, but that's the app they're advertising on the cited site. I wondered if VS wouldn't pick up on it (I truly don't know - had only experience with gcc, and am pretty sure such casts are picked up)

# ? Apr 15, 2014 21:46

necrotic: Aug 2, 2005; I owe my brother big time for this!

eithedog posted:

Yes, but that's the app they're advertising on the cited site. I wondered if VS wouldn't pick up on it (I truly don't know - had only experience with gcc, and am pretty sure such casts are picked up)

Oh wow, didn't even notice that.

# ? Apr 15, 2014 22:42

ToxicFrog: Apr 26, 2008

The details of this story have been changed to protect the guilty.

At work, one of the things I work on is a system for managing server configuration. People write configuration files for various subsystems, and these files are processed and combined into a master configuration image which is periodically deployed to the servers. I am currently working on a major overhaul to the format of the configuration image and the way it's generated. This has turned up (and provided me with an excuse to destroy forever) a lot of terrible legacy stuff that was officially deprecated years ago but never actually retired.

For testing, we have an end-to-end test that builds a config image from a test configuration, brings up a representative set of servers reading from that configuration, and makes requests to them and verifies the results. This test configuration shares the same default settings as the real configuration.

However, some of these default settings actually interfere with the tests, so for the test, a small subset of them need to be overridden and then selectively changed during test execution. Now, this is pretty easy, since the configuration language supports conditional settings, so you can just do something like this:

code:

option "use_shitloads_of_ram" {
  default = true; # slightly decreases latency
  if (testing) {
    default = false;
  }
}

The problem? Some of the tests were failing with my change. The configuration image was good, and the test was running to completion, but some of the queries were returning unexpected results - as though certain settings did not have the test-specific defaults applied. But most of the settings that had such overrides were set correctly.

A co-worker and I spend some time digging and, finally, the horrible truth dawns.

The configuration language did not always support such conditional settings. In those days, the overrides were applied by a shell script. When the end to end test was starting up, but before actually starting any of the servers, it would not directly generate the configuration image from the input files. Instead, it would generate a (text-based) intermediate representation, then run this script to effectively do a find-and-replace on it, replacing the default settings with the test-specific overrides. Then it would convert the IR into the actual config image and start the servers.

For some reason, when conditional settings were implemented, not all of the overrides were moved out of the script, and a few stayed there, buried in the bowels of the test harness, until the day that I tried to change the configuration format -- causing it not to fail, but instead start silently passing through the configuration unaltered.

# ? Apr 16, 2014 01:26

cowboy beepboop: Feb 24, 2001

code:

<?php


echo <<<EOF
</body>
</html>
EOF;

?>

# ? Apr 16, 2014 03:32

Extortionist: Aug 31, 2001; Leave the gun. Take the cannoli.

Perl code:

close STDERR;
open STDERR, "> /dev/null";

open SQL, "| /informix/bin/dbaccess" 
    or die "Unable to open SQL handle: $!\n";

print SQL $sql;

The path to dbaccess was recently changed.

Not pictured: capturing STDOUT in a variable, then using regex matches to parse out the information that was queried from the database.

# ? Apr 16, 2014 04:23

Gazpacho: Jun 18, 2004; by Fluffdaddy; Slippery Tilde

Suspicious Dish posted:

I found a bug in GNU bash, so I went to go check out the source code. Huh...

I hope you did eventually find the more informative change logs on ftp.gnu.org, right where they have been for the last 20+ years. Why change? :buddy:

e: If you were a real hacker you would have spent all of 30 minutes rigging up emacs to import the patches and tarballs into quilt

Gazpacho fucked around with this message at 06:56 on Apr 16, 2014

# ? Apr 16, 2014 06:49

Suspicious Dish: Sep 24, 2011; 2020 is the year of linux on the desktop, bro; Fun Shoe

No, I didn't. I have a lingering feeling they wouldn't accept my patch anyway, so I'm not going to bother.

# ? Apr 16, 2014 06:55

coffeetable: Feb 5, 2006; TELL ME AGAIN HOW GREAT BRITAIN WOULD BE IF IT WAS RULED BY THE MERCILESS JACKBOOT OF PRINCE CHARLES

YES I DO TALK TO PLANTS ACTUALLY

Crossquotin'

flyboi posted:

has this been posted yet
http://opensslrampage.org/

quote:

Do not feed RSA private key information to the random subsystem as entropy.

# ? Apr 17, 2014 21:27

Malloc Voidstar: May 7, 2007; Fuck the cowboys. Unf. Fuck em hard.

C code:

	if ((RAND_status() == 0) && rsa->d != NULL && rsa->d->d != NULL)
		{
		/* if PRNG is not properly seeded, resort to secret
		 * exponent as unpredictable seed */
		RAND_add(rsa->d->d, rsa->d->dmax * sizeof rsa->d->d[0], 0.0);
		}

Seems reasonable, what could be more random than your private key?

# ? Apr 17, 2014 21:42

revmoo: May 25, 2006; #basta

revmoo fucked around with this message at 17:49 on May 21, 2014

# ? Apr 18, 2014 18:46

Hammerite: Mar 9, 2007; And you don't remember what I said here, either, but it was pompous and stupid.; Jade Ear Joe

revmoo posted:

The system is currently down for hardware maintenance and should return online at <? echo strftime('%D %H:00',time() + (3600*2)); ?> EST/EDT. Please call us if you need assistance.

I'm the disingenuous estimated time for the system to become available.

# ? Apr 18, 2014 19:13

Subjunctive: Sep 12, 2006; ✨sparkle and shine✨

revmoo posted:

The system is currently down for hardware maintenance and should return online at <? echo strftime('%D %H:00',time() + (3600*2)); ?> EST/EDT. Please call us if you need assistance.

I guess they were looking to statically generate it to show during the outage?

That libssl evisceration is nonstop gold.

# ? Apr 18, 2014 20:59

Scaevolus: Apr 16, 2007

Apparently /dev/urandom has a spinlock that kills performance when read by multiple threads simultaneously.

But this is by design, according to the author:

Theodore Ts'o posted:

Doctor, doctor it hurts when I do this. Well, then don't do that!

The reason for the spinlock is to avoid having two reads from /dev/urandom return the same results. That would be undesirable...

If people are trying to read from /dev/urandom a huge number of times per second, then they're doing something wrong. The real issue, as Sebastian has pointed out, is that issue spawning a huge number of curl processes for each request. Then hopefully curl is using /dev/urandom to initialize its own userspace RNG (many crypto libraries do this; most *should* do this), so you're not trying to read from /dev/urandom for every single request. /dev/urandom is designed for security, not for speed.

(hint: you can get security AND speed simultaneously)

# ? Apr 19, 2014 07:22

Zombywuf: Mar 29, 2008

Scaevolus posted:

Apparently /dev/urandom has a spinlock that kills performance when read by multiple threads simultaneously.

But this is by design, according to the author:

Not entirely convinced by that article, mainly the part where the author uses Python to measure lock contention in an external process.

# ? Apr 19, 2014 11:34

Adbot: ADBOT LOVES YOU

# ? May 17, 2024 18:26

Jabor: Jul 16, 2010; #1 Loser at SpaceChem

Scaevolus posted:

(hint: you can get security AND speed simultaneously)

Is this why people implement their own memory allocator because they think the platform one isn't fast enough?

That said, there isn't a reason not to have per-cpu urandom state if someone wants to put in the work to implement it.

# ? Apr 19, 2014 12:48

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!

«‹›1503 »