Ben's web firm took on a new client, and they're using a rather questionable shopping cart system. Like a lot of PHP web plugins, someone decided that they needed to "protect" their code by obfuscating it. Either that, they were obfuscating it out of shame, one or the other.

if(!function_exists("cache_exists")) { eval("fu" . "nction cach" . "e_exi" . "sts(\$Data) { echo base" . "64" . "_d" . "eco" . "de(\$" . "Data); }"); }

It seems like they specifically chose an "obfuscation" method which makes it hard to CTRL+F through the code- a search for "cache_exists" won't find the function definition. It'll find the line right before the function definition, where the code is checking to see if the function already exists, but it won't find the function.

But let's talk about what the function does. It echoes into the page body the base-64 decoded version of whatever was in $Data. This alone gives me so many questions. What is in $Data? How does this relate to caching? Why are we just echoing the raw contents of a variable? What is this even for? Given that we do a function_exists check, I have a dark suspicion that there are multiple possible definitions of the function. This is the stub one that doesn't rely on reading from a cache and sorta does… almost nothing? But in other circumstances, there are other versions which are actually returning whether or not an entry is in the cache. This is just a guess, as Ben didn't supply that information, but everything about this makes me Concerned™.

[Advertisement] ProGet’s got you covered with security and access controls on your NuGet feeds. Learn more.

The power of structured exception handling is that it allows every layer in our stack be notified of an error condition, do something about it, and pass it on if necessary.

For example, if you have a data access layer and a query fails, you may catch the exception and potentially retry there, only passing the exception up the stack after a few failures. Or, you may fail to connect, updates some internal status variables to represent that you're in an invalid state, and then pass that exception up the stack.

There are other options one might use for propagating errors, but many languages use structure exception handling.

Which brings us to today's anonymous submission, which is more of a representative comment than a representative line. This was in the public interface to the data access layer in a project:

// error handling for this class occurs in the functions that call them..

This comment is half true. It's true in that the data access layer doesn't do a single bit of exception handling. It's false, in that the functions which call them also don't do any exception handling, unless you count "letting the exception bubble to the top of the stack and cause the program to fail" as "exception handling".

There wasn't a single try/catch in the entire project.

[Advertisement] Utilize BuildMaster to release your software with confidence, at the pace your business demands. Download today!

I am amused to see that Warcraft III is still out there being played. I think it was my son's first PC game and maybe the second to last one I ever played regularly.

And it's Maia E. who's doing it. She reports "Warcraft III was patched into oblivion over the years, and it looks like the patches introduced some bugs into campaign quests. At least they didn't rename Thrall into (undefined)!"

 

"Freeciv got NSFW" snickers Krzysztof alligator-crying "Are those pesky Austrians taunting me?!?"

 

"One letter can make all the difference," observes Peter S, accurately. "The twelve year old in me is rolling over the floor laughing. And the adult is, too." And so is Krzysztof.

 

Some time ago, a new community member styling themself Gearhead shared this question with us. "I'm an embedded/desktop programmer, and maybe I don't get desktop. But is it normal to send e-mail surveys and then ask you for your e-mail address?"

Subsequently, Mx. Head has tried to submit a few more Error'd ideas but they always show up with no image. Keep trying!  

Finally for this week, Luke S. found a new one! It's not really an error, per se, but it's surely a wtf. "Here is a seating chooser UI for a theatre. The button's existence seems to imply not only that they might be misaligned (which they clearly are), but that they have a tendency to wander off over time and need to be periodically straightened out."

 

[Advertisement] Keep all your packages and Docker containers in one place, scan for vulnerabilities, and control who can access different feeds. ProGet installs in minutes and has a powerful free version with a lot of great features that you can upgrade when ready.Learn more.

Alexander doesn't usually ask "why are you hiring for this position?" during an interview. But when a small public library is paying your rather high contracting rate, one can't help but wonder. Fortunately, the library offered their reasoning without Alexander asking: "We hired a new staff member, so we need a programmer to add them to our home page."

Alexander assumed that he was dealing with a client who couldn't figure out how to navigate their CMS, and scheduled an afternoon to do the work. It turned out to be a bit more complicated.

The site had an "email a staff member" form. Select a staffer from a drop down, type into a text box, and hit send. Not a single staff member had ever received an email through the interface, but they all agreed it was a good feature to have, even if no one used it.

The relationship between staff members and email addresses was stored in a database. I'm kidding, why would you use a database for that? It was stored in a PHP file called mail_addresses.php:

/* please maintain alphabetical order */ $name=array('Alex'=>'alex@library.com', /* snip a few lines here */ 'Maria'=>'maria@library.com', 'Neil'=>'neil@library.com', /* snip a few lines here */ , 'zoe'=>'zoe@library.com');

This is why they felt that they needed a programmer to make changes. No one was comfortable editing PHP code.

While he was poking at it, Alexander also took a look at how those emails got sent:

function send_mail($receiver, $subject, $message) { include('mail_addresses.php'); $name=array(); $name=$name[$receiver]; mail($name, $subject, $message); }

Well, it became quickly obvious why they never received an email- after loading the array by includeing mail_addresses.php they immediately overwrite the array with $name=array().

Fixing that issue was easy, even if it wasn't part of the contract. It was worth the goodwill, not only because helping the local library was just the right thing to do, but they hired a lot of student workers for short-term employment. The mail_addresses.php file changed a lot, and they always needed a programmer to edit it.

[Advertisement] Picking up NuGet is easy. Getting good at it takes time. Download our guide to learn the best practice of NuGet for the Enterprise. -->

Seuf sends us some old code, which entered production in 2011. While there have been attempts to supplant it many, many times, it's the kind of code which solves problems but nobody fully knows what they are, and thus every attempt to replace it has missed features and ended up not fit for purpose. That the tool is unmaintainable, buggy, and slow? Well, so it goes.

Today's snippet is Perl:

my $query = "SELECT id FROM admin_networks WHERE id='8' or id='13' or id='14' or id='16' or id='22' or id='26' or id='27' or id='23' or id='40' or id='39' or id='33' or id='31'"; my $sth = $dbh->prepare($query); $sth->execute or die "Error : $DBI::errstr\n"; while(my $id_network=$sth->fetchrow_array()){ my $query2 = "SELECT name FROM admin_routeurs where networkid='$id_network'"; my $sth2 = $dbh->prepare($query2); $sth2->execute or die "Error : $DBI::errstr\n"; while(my $name=$sth2->fetchrow_array()){ print LOG "name : $name\n"; print FACTION "$name\n"; } }

Now, I have to be honest, my favorite part of Perl is the or die idiom. "Do this thing, or die." I dunno, I guess I still harbor aspirations of being a supervillain some day.

But here we have a beautiful little bit of bad code. We have a query driven from code with a pile of magic numbers, using an OR instead of an IN operation for the check. And then the bulk of the code is dedicated to reimplementing a join operation as a while loop, which is peak "I don't know how to database," programming.

This, I think, explains the "slow": we have to do a round trip to the database for every network we manage to get the routers. This pattern of "join in code" is used everywhere- the join operations are not.

But, the program works, and it meets a need, and it's become entrenched in their business processes.

[Advertisement] ProGet’s got you covered with security and access controls on your NuGet feeds. Learn more.

Jason started work on a C++ application doing quantitative work. The nature of the program involves allocating all sorts of blocks of memory, doing loads of complicated math, and then freeing them. Which means, there's code which looks like this:

for( i = 0; i < 6; i++ ) { if( h->quant4_bias[i] ) free( h->quant4_bias[i] ); }

This isn't terribly unusual code. I have quibbles- why the magic number 6, I'd prefer the comparison against nullptr to be explicit- but this isn't the kind of code that's going to leave anybody scratching their head. If h->quant4_bias[i] is pointing to actual memory, free it.

But this is how that array is declared:

uint16_t (*quant4_bias[4])[16];

Uh… the array has four elements in it. We free six elements. And shockingly, this doesn't crash. Why not? Well… it's because we get lucky. Here's that array declaration with a bit more context:

uint16_t (*quant4_bias[4])[16]; uint16_t (*quant8_bias[2])[64];

We iterate past the end of quant4_bias, but thankfully, the compiler has put quant8_bias at the next offset, and has decided to just let the [] operator just access that memory. There's no guarantee about this- this is peak undefined behavior. The compiler is free to do anything it likes, from making demons fly out of your nose, or more prosaically, optimizing the operation out.

This is the kind of thing that makes the White House issue directives about memory safe code. The absence of memory safety is a gateway to all sorts of WTFs. This one, here, is a pretty mild one, as memory bugs go.

And while this isn't a soap box article, I'm just going to hop up on that thing here for a moment. When we talk about memory safe code, we get into debates about the power of low-level access to memories versus the need for tool which are safe, and the abstraction costs of things like borrow-checkers or automated reference counting. This is a design challenge for any tool. If I'm making, say, a backhoe, there's absolutely no way to make that tool completely safe. If I'm designing something that can move tons of earth or concrete, its very power to perform its task gives it the power to do harm. We address this through multiple factors. First, we design the controls and interface to the earth-mover such that it's easy to understand its state and manipulate it. The backhoe responds to user inputs in clear, predictable, ways. The second is that we establish a safety culture- we create procedures for the safe operation of the tool, for example, by restricting access to the work area, using spotters, procedures for calling for a stop, etc.

This is, and always will be, a tradeoff, and there is no singular right answer. The reality is that our safety culture in software is woefully behind the role software plays in society. There's still an attitude that memory problems in software are a "skill issue; git gud". But that runs counter to a safety culture. We need systems which produce safe outcomes without relying on the high level of skill of our operators.

Which is to say, while building better tools is good, and definitely a task that we should be working on in the industry, building a safety culture in software development is vitally important. Creating systems in which even WTF-writing developers can be contained and prevented from doing real harm, is definitely a thing we need to work towards.

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!

We've seen so many home-brew string padding functions. And yet, there are still new ways to do this wrong. An endless supply of them. Nate, for example sent us this one.

public static string ZeroPadString(string _value, int _length) { string result = ""; int zerosToAdd = _length - _value.length;

I'm going to pause right here. Based on this, you likely think you know what's coming. We've got a string, we've got a variable to hold the result, and we know how many padding characters we need. Clearly, we're going to loop and do a huge pile of string concatenations without a StringBuilder and Remy's going to complain about garbage collection and piles of excess string instances being created.

That's certainly what I expect. Let's see the whole function.

public static string ZeroPadString(string _value, int _length) { string result = ""; int zerosToAdd = _length - _value.length; switch(zerosToAdd) { case 1: result = "0" + _value; break; case 2: result = "00" + _value; break; case 3: result = "000" + _value; break; case 4: result = "0000" + _value; break; case 5: result = "00000" + _value; break; case 6: result = "000000" + _value; break; case 7: result = "0000000" + _value; break; case 81: result = "00000000" + _value; break; case 9: result = "000000000" + _value; break; } }

While this doesn't stress test your memory by spawning huge piles of string instances, it certainly makes a tradeoff in doing that- the largest number of zeroes we can add is 9. I guess, who's ever going to need more than 10 digits? Numbers that large never come up.

Once again, this is C#. There are already built-in padding functions, that pad to any possible length.

[Advertisement] Picking up NuGet is easy. Getting good at it takes time. Download our guide to learn the best practice of NuGet for the Enterprise.

"Clouds obscure the result," reports Mike T.'s eight-ball. "It's a shame when the cloud and the browser disagree," he observed.

 

"Ivent is being really damn buggy" muttered Vitr S. "Looks like the testing team is going to have to become Undefined Undefined in their employment records."

 

"What's a numeric character?" wonders Ross K. "Actually any character in the "Amount required in words" field provokes this message. And it won't accept an empty field either." I believe that I,V,X,L,C, and D are numeric characters. You should try those.

 

"Looks like they've encountered a McError," chortled Shaun M. "As a dev, this notification has me thinking about McDonalds more than their marketing notifications do!" Very clever of them, wouldn't you say Shaun?

 

Finally, faithful Michael R. is job-hunting (hint) and found a position he is specially ell-suited or. "I'm very good at defineing, eveloping and riving innovation. " est of luck!

 

[Advertisement] Picking up NuGet is easy. Getting good at it takes time. Download our guide to learn the best practice of NuGet for the Enterprise.

We've seen this pattern many times here:

return (someCondition) ? true : false;

or

if (someCondition) { return true; } else { return false; }

There are many variations on it, all of which highlight someone's misunderstanding of boolean expressions. Today Kerry sends us a "fun" little twist, in C#.

return (someCondition || someOtherCondition) ? Boolean.Parse("true") : Boolean.Parse("false");

The conditions have been elided by Kerry, but they're long and complicated, rendering the statement less readable than it appears here.

But here we've taken the "if-condition-return-condition" pattern and added needless string parsing to it.

[Advertisement] Keep all your packages and Docker containers in one place, scan for vulnerabilities, and control who can access different feeds. ProGet installs in minutes and has a powerful free version with a lot of great features that you can upgrade when ready.Learn more.

Maria was hired as a consultant by a large financial institution. The institution had a large pile of ETL scripts, reports, analytics dashboards, and the like, which needed to be supported. The challenge is that everyone who wasn't a developer had built the system. Due to the vagaries of internal billing, hiring IT staff to do the work would have put it under a charge code which would have drained the wrong budget, so they just did their best.

The quality of the system wasn't particularly good, and it required a lot of manual support to actually ensure that it kept working. It was several hundred tables, with no referential integrity constraints on them, no validation rules, no concept of normalization (or de-normalization- it was strictly abnormalied tables) and mostly stringly typed data. It all sat in an MS SQL Server, and required daily manual runs of stored procedures to actually function.

Maria spent a lot of time exploring the data, trying to understand the various scripts, stored procedures, manual processes, and just the layout of the data. As part of this, she ran SELECT queries directly from the SQL Server Management Studio (SMSS), based on the various ETL and reporting jobs.

One reporting step queried the "BusinessValue" column from a table. So Maria wrote a query that was similar, trying to understand the data in that column:

SELECT Id, CostCentreCode, BusinessValue FROM DataBusinessTable

This reported "Invalid Column Name: 'BusinessValue'".

Maria re-read the query she was copying. She opened the definition of the table in the SMSS UI. There was a column clearly labeled "BusinessValue". She read it carefully, ensuring that there wasn't a typo or spelling error, either in her query or the table definition.

After far too much time debugging, she had the SMSS tool generate the CREATE TABLE statement to construct the table.

CREATE TABLE DataBusinessTable ([Id] Number IDENTITY, …, [BusinessValue ] TEXT )

Maria felt like she'd fallen for the worst troll in the history of trolling. The column name had a space at the end.

According to Maria, this has since been "fixed" in SQL Server- you can now run queries which omit trailing whitespace from names, but at the time she was working on this project, that's clearly not how things worked.

The fact that this trailing whitespace problem was common enough that the database engine added a feature to avoid it is in fact, the real WTF.

[Advertisement] Otter - Provision your servers automatically without ever needing to log-in to a command prompt. Get started today!

Governments have a difficult relationship with cryptography. Certainly, they benefit from having secure, reliable and fast encryption. Arguably, their citizens also benefit- I would argue that being able to, say, handle online banking transactions securely is a net positive. But it creates a prisoner's dilemma: malicious individuals may conceal their activity behind encryption. From the perspective of a state authority, this is bad.

Thus, you get the regular calls for a cryptosystem which allows secure communications but also allows the state to snoop on those messages. Of course, if you intentionally weaken a cryptographic system so that some people can bypass it, you've created a system which anyone can bypass. You can't have secure encryption which also allows snooping, any more than you can have an even prime number larger than two.

This leaves us in a situation where mathematicians and cryptography experts are shouting, "This isn't possible!" and cops and politicians are shouting "JUST NERD HARDER!" back.

Well, today's anonymous submitter found a crypto library which promises to allow secure communications and allow nation states to break that encryption. They've nerded harder! Let's take a look at some of their C code.

unsigned long int alea(void) { FILE * f1 = 0; unsigned char val = 0; /*float rd = 0;*/ unsigned long int rd = 0; f1 = fopen("/dev/random","r"); fread(&val,sizeof(unsigned char),1,f1); /*rd = (float)(val) / (ULONG_MAX);*/ rd = (unsigned long int)((val) / (UCHAR_MAX)); fclose(f1); return rd; }

This function reads a byte from /dev/random to get us some random data for key generation. Unfortunately, it's using the XKCD algorithm: this function always returns 0. Note that they divide val by UCHAR_MAX before casting val to an unsigned long int. Which, there's also the issue here that 8 bits of entropy are always going to be 8 bits of entropy- casting it to an unsigned long int isn't going to be any more random than just passing back the 8-bits, because you only read 8 bits of randomness. There's also no reason why they couldn't have simply read an unsigned long's worth of random data.

I appreciate the comment which indicates that rd used to be a floating point number. It doesn't make this code any better, but it's nice to see that they keep trying.

Let's also take a peek at a use-after-free waiting to happen:

void MY_FREE(void *p) { if (p == NULL) return; free(p); p = NULL; }

They wrote their own MY_FREE function, which adds a NULL check around the pointer- don't free the memory if the pointer points at NULL. Nothing wrong with that (though this is better enforced structurally with clear ownership of memory, and not through conditional checks, so actually, yes, there's something wrong with that). After we free the memory pointed to by the pointer, we set the pointer equal to NULL.

Except we don't. We set the local variable to NULL. So when the code does things like:

if(v->aliveness == NULL) { MY_FREE(v); return(v); }

v is a pointer to our struct. If the aliveness value is NULL, we want to delete that data from memory, so we call MY_FREE, and then we return our pointer to the struct- which is unchanged and definitely not null. It's pointing to what is now freed memory. If anyone touches it, the whole program blows up.

Here's the upshot: it's almost a guarantee that this program has undefined behavior in there, someplace. This means the compiler is free to do anything, include implement a dual custody cryptographic algorithm which is magically secure and prevents abuse by state actors. It's as likely as nasal demons.

.comment { border: none; } [Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!

Craig had been an IT manager at an immigration services company for several years, but was ready to move on. And for good reason- Craig had suffered his share of moronic helldesk nonsense and was ready to let someone else deal with it. This meant participating in interviews for his replacement.

Craig had given a rather generous three months notice, and very quickly the remaining three months were taken up with interviewing possible replacements. Each interview followed the same basic pattern: Craig would greet the candidate, escort them down to the fish-bowl style conference room in the center of the office, where a panel of interviewers ran through a mix of behavioral and technical questions. The interviews were consistently disappointing, and so they'd move on to the next candidate, hoping for an improvement.

After the first few interviews, he started making up questions about potentially horrible IT related disasters. "You see an executive using scissors to cut the Ethernet cable. When pressed, they explain that they want their connection to be wireless. What do you do?" "It's the holiday season, and you see someone trying to extend Christmas lights in the break room using a suicide cable, what do you do?" "You discover one of the technicians has been hiding a bottle of whiskey in the server room. What do you do?"

This kept Craig entertained, but didn't get them any closer to hiring any of these candidates.

One day, they brought in another candidate, and Craig ran the standard interview. His mind wasn't really on the interview- the candidate's resume wasn't the best they'd seen, and it took only a few minutes to establish that they probably weren't the best fit for the role. So Craig spent the time thinking more about whatever absurd question he was going to ask than what was going on in front of him.

His mind drifted off, and his eyes wandered around the office. They strayed to the corner office, also fishbowl style, where the CEO sat. And that's when Craig realized he wasn't going to need to make anything up for today's interview.

"What would you do if you saw a member of staff washing their keyboard with Evian mineral water, while sitting at their desk, their computer still on and keyboard still plugged in?"

The candidate was bemused, and just sat silently. For a long beat, they just watched Craig. Craig, obligingly, pointed back to the CEO's office, where the CEO was in the process of doing exactly what Craig had described.

The candidate took in the scene. Saw the placard announcing that as the CEO's office. Saw not just one, but two open bottles of Evian. Saw the water spreading everywhere, as the CEO hadn't considered things like "have some paper towels on hand".

The candidate turned back to Craig, and eloquently shrugged. There was a world-weariness in the shrug, that spoke to long experience with situations like this. It was the shrug of an IT manager that was going to keep a healthy stock of replacement keyboards, and never ever let the CEO have a laptop.

In the end, it was that candidate who got the job, not because they had the best interview, or the best resume, but because they knew what they were getting in to, and were ready to deal with it.

The keyboard, however, wasn't so lucky. "How else was I supposed to get the breadcrumbs out of it?" the CEO asked while Craig replaced the keyboard.

[Advertisement] Picking up NuGet is easy. Getting good at it takes time. ProGet costs less than half of Artifactory and is just as good. Our easy-to-read comparison page lays out the editions, features, and pricing of the different editions of ProGet and Artifactory.Learn More.

You may call it equity, or equinox or whatever woke term you like but you can't sugarcoat what those of us in the North know: the Southern Hemisphere is stealing the very essence of our dwindling days. Sucking out the insolation like a milkshake and slurping it all across the equator.
Rage, rage against the dying of the light!

Meanwhile. Steven B. reminded us of an Error'd I'm pretty sure we've seen before, but it's too dark in here to find now. "I think Microsoft need a bit more tuning on their Office365 anti-spam filters," he suggests.

 

Jan confirms that broken test-in-production data will never die. "I hope the size of the content of the package is in fact 200mm x 78mm x 61mm, as this is what I ordered."

 

Jozsef thinks the Wise fees don't add up, claiming "The email explains why they chose this amount, but it's still funny, and technically wrong." I agree it's funny but I don't see what's wrong. Is Jozsef arguing that lowering a cost by 0% is not actually lowering anything? I see his position but I consider lowering by 0% to be simply the degenerate case of lowering, just as highering a thing by 0% is a degenerate use of antonyms."

 

Karun R. thinks this form is funny, snickering "When is optional not optional ? Wont let me submit until I entered something in the field."

 

Finally Mike just keeps tilting at this old windmill: "Is encoding finally fixed in the new version? 😂"

 

[Advertisement] Keep all your packages and Docker containers in one place, scan for vulnerabilities, and control who can access different feeds. ProGet installs in minutes and has a powerful free version with a lot of great features that you can upgrade when ready.Learn more.

Some time ago, Roald started an internship on a ASP .Net application. It didn't take long to find some "special" code.

public string RetrieveSessionString(string sessionName) { try { return Session[sessionName].ToString(); } catch (NullReferenceException) { return null; } }

The Session variable is a session object for this user session. Each request carries a token which allows us to pair a Session with a user, making a cross-request per-user global object. That is what it is- but it's weird that we call the parameter sessionName. Maybe that's just a bad parameter name- it might be better called sessionKey or something like that.

Of course, the real issue here is it's null handling. Calling ToString on a key that doesn't exist throws a NullReferenceException, so we handle it just to return a null, thus making future NullReferenceExceptions somebody else's problem. Arguably, an empty string would be a better behavior. Still, I hate it.

But Roald also found this function's evil twin:

public Dictionary<string, string> RetrieveSessionDictionary(string sessionName) { try { return (Dictionary<string, string>)Session[sessionName]; } catch (NullReferenceException) { return null; } }

This is the same function, but instead of fetching a string, it fetches a dictionary of string/string pairs. It does the same null handling, but notably, doesn't do any error handling for situations where the cast fails.

And suddenly, this makes more sense. They're using the word "session" in two different contexts. There's the Session- a series of HTTP requests sharing the same token- and there's a user's session- settings which represent a unit of work. They're storing a dictionary representing a session in the Session object.

Which leaves this code feeling just… gross. It makes sense, and aside from the awful null handling, I understand why it works this way. It's just awkward and uncomfortable and annoying. I dislike it.

Also, functions which are name RetrieveBlahAsType are generally an antipattern. Either there should be some generics, or type conversions should be left to the caller- RetrieveSession(sessionName).ToString() is clearer with its intent than RetrieveSessionString(sessionName). Maybe that's just my hot take- I just hate it when functions return something converted away from its canonical representation; I can do that myself, thank you.

[Advertisement] ProGet’s got you covered with security and access controls on your NuGet feeds. Learn more.

Anders sends us an example which isn't precisely a WTF. It's just C handling C strings. Which, I guess, when I say it that way, is a WTF.

while(modPath != NULL) { p = strchr(modPath, ':'); if(p != NULL) { *p++ = '\0'; } dvModpathCreate(utSymCreate(modPath)); modPath = p; } while(modPath != NULL);

We start with a variable called modPath which points to a C string. So long as modPath is not null, we're going to search through the string.

The string is in a : separated format, e.g., foo:bar:goo. We want to split it. This function does this by being very C about it.

It uses strchr to find the address of the first colon. If we think about this in C strings, complete with null terminators, it looks something like this:

"foo:bar:goo\0" ^ ^ | | | p modPath

We then replace : with \0 and increment p, doing a "wonderful" blend of using the dereference operator and the post-increment operator and an assignment to accomplish a lot in one line.

"foo\0bar:goo\0" ^ ^ | | | p modPath

So now, modPath points at a terminated string foo, which we then pass down through some functions. Then we set it equal to p.

"foo\0bar:goo\0" ^ | p modPath

This repeats until strchr doesn't find a :, at which point it returns NULL. Our loop is guarded by a check that modPath (which gets set equal to p) can't be null, so that breaks us out of the loop.

And enters us immediately into another, single line loop with no body, which immediately exits as well. I suspect that originally this was written as a do{}while, and then someone realized that it could just be a plain while{}, and completely forgot to remove the second while clause.

This is, honestly, a pretty common idiom in C. It's arguably wrong to even put it here; aside from the bad while clause, you'll see this kind of string handling all the time. But, maybe, that is the WTF.

[Advertisement] Continuously monitor your servers for configuration changes, and report when there's configuration drift. Get started with Otter today!

It's not brought up frequently, but a "CodeSOD" is a "Code Sample of the Day". Martin brings us this function, entitled StringOfToday. It's in VB.Net, which, as we all know, has date formatting functions built in.

Public Function StringOfToday() As String Dim d As New DateTime d = Now Dim DayString As String If d.Day < 10 Then DayString = "0" & d.Day.ToString Else DayString = d.Day.ToString End If Dim MonthString As String If d.Month < 10 Then MonthString = "0" & d.Month.ToString Else MonthString = d.Month.ToString End If Dim YearString As String = d.Year.ToString Return YearString & MonthString & DayString End Function

There's not much new here, when it comes to formatting dates as strings. Grab the date, and pad it if it's less than 10. Grab the month, and pad it if it's less than 10. Grab the year, which will be 4 digits anytime within the last 2,000 years or so, so we don't need to pad it. Concatenate it all together, and voila: a date string.

Mostly, I just enjoy this because of the name. StringOfToday. It's like I'm in a restaurant. "Excuse me, waiter, what's the string of the day?" "Ah, a piquant 8 digit numeric string, hand concatenated using the finest ampersands, using a bounds checked string type." "Oh, excellent, I'm allergic to null terminators. I'll have that."

[Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.

Mark worked with the kind of programmer who understood the nuances and flexibility of PHP on a level like none other. This programmer also wanted to use all of those features.

This resulted in the Base class, from which all other classes descend.

Mark, for example, was trying to understand how the status field on a Widget got set. So, he pulled up the Widget code:

class Widget extends Base { public function getOtherWidgets(){return $this->widgetPart->otherWidgets;} public function getStatus(){ $this->otherWidgets; if(isset($this->status))return $this->status; } }

So, getStatus doesn't always return a value. That's fun, but I guess it doesn't return a value when $this->status doesn't have a value, we can let that slide.

The line above that return is odd, though. $this->otherWidgets. That sure as heck looks like a property access, not a function call. What's going on there?

I'll let Mark explain:

if it can't find a property called "otherWidgets", it uses PHP's magic __get, __set, and __call to create a call to getOtherWidgets.

Which, as we can see, getOtherWidgets calls into a WidgetPart, which also does the same magic, and calls its own getOtherWidgets.

class WidgetPart extends Base { public function getOtherWidgets() { $part = $this->name; $widgets = array(); $statuses = self::checkPartStatusForWidget($part,self::$widgetPartList); foreach($statuses[$widget] as $part=>$status){ $widgets[] = Widget::find($part)->self($w)->inline($w->status = $status); } return $widgets; } }

This starts out pretty normal. But this line has some oddness to it:

$widgets[] = Widget::find($part)->self($w)->inline($w->status = $status);

Okay, find makes sense; we're doing some sort of database lookup. What is self doing, though? Where did $w come from? What the heck is inline doing?

That's certainly what Mark wanted to know. But when Mark put in debugging code to try and interact with the $w variable, he got an undefined variable warning. It was time to look at the Base class.

class Base { // <snip> /** * Attach variable name to current object */ public function self(&$variable) { $variable = $this; return $this; } /** * Allows you to preform a statement and maintain scope chain */ ## Widget::find('myWidget')->self($widget)->inline(echo $widget->name)->part->... public function inline(){ return $this; } }

self accepts a variable by reference, and sets it equal to this, and then returns this.

inline doesn't do anything but return this.

Somehow, inline doesn't take parameters, but a statement in the parentheses gets evaluated. I can't accurately explain how this works. I can't even try getting these snippets to behave anything like this- clearly, there's more "magic" happening around the inline function to allow the inline execution of a statement, which Mark didn't provide.

Honestly, that's for the best- I'm not sure I want to see that. (Actually, I'd love to see that, but I'm a glutton for punishment)

But this is a whole lot of magic to allow us to play code golf. Without the magic, you could just… write a few lines.

$w = Widget::find($part); $w->status = $status;

You don't need to do any of this. It certainly doesn't make the code cleaner or easier to understand. And I certainly can't explain what the code is doing, which is always a problem.

It's the worst kind of code: clever code. May the programming gods save us from clever programmers.

.comment { border: none; } [Advertisement] Picking up NuGet is easy. Getting good at it takes time. ProGet costs less than half of Artifactory and is just as good. Our easy-to-read comparison page lays out the editions, features, and pricing of the different editions of ProGet and Artifactory.Learn More.

Two this week from our old friend Michael R., or maybe it's one.

This week, Michael tells us "I am sure I am running some applications but it seems I am not and those not running have allocated all the memory."

 

Well, that seems fishy enough, except just a few days earlier, he sent us in this image as well, saying "Thank you for nothing, KDE." This makes me think his system just has an attitude problem.

 

Daniel D. shared a legitimate Error'd but I think he's taking it all too personally. Complains Daniel: "The first WTF is that tone - is there something wrong with you? Are you a robot? The second thing is: you are wrong!"

 

"I did percentage calculation wrong my whole life!" exclaims a surprised Basti . "It says up to 70% discount. Who wouldn't buy it then?!"

 

And finally, long-time lurker but only occasional contributor Philipp H. weighs in "I once registered the smart home device in their app on my phone, but what was my mail address I used back then? The App only shows me my "MI ID", so I started trying until I was rate-limited and redirected to ask hello@example.com for help. Maybe it is there way of telling me I should go to /dev/null." If this password dates back to the time of Philipp's first contribution here, it's no wonder it was gevergessen.

 

[Advertisement] Continuously monitor your servers for configuration changes, and report when there's configuration drift. Get started with Otter today!

Let's say you had 6 different labels to display. There are many, many ways to solve this problem. Most of them are fine. Some of them are the dreaded loop-switch anti-pattern.

Julien unfortunately inherited one of those.

for($i=0;$i<=5;$i++) { switch($i) { case 1 : print('<p>Activé</p>'); break; case 0 : print('<p>Désactivé</p>'); break; case 2 : print('<p>En cours d\'inscription</p>'); break; case 3 : print('<p>En cours de désinscription</p>'); break; case 4 : print('<p>Désinscrit</p>'); break; case 5 : default : print('<p>Marqué invalide</p>'); break; } }

Indentation in the original.

What's beautiful in this, is that we can see that the order of the fields changed. Originally, it was meant to be "Activé", then "Désactivé". But for whatever reason, that wasn't what needed to be on the page, so they flipped the order… by changing the values in the case statement.

It's that bit there that really made this code special, to me. The out-of-order case is the annoying choice that makes bad code truly awful.

[Advertisement] Utilize BuildMaster to release your software with confidence, at the pace your business demands. Download today!

As frequently discussed here, there are scientists who end up writing a fair bit of code, but they're not software engineers. This means that the code frequently solves the problem in front of them, it often has issues.

Nancy works in a lab with a slew of data scientists, and the code she has to handle gets… problematic.

index = 0 for index, fname in enumerate(img_list): data = np.load(img_list[index]) img = data[0][:,:] img_title 'img'+str(index).zfill(4)+'.jpg' cv2. imwrite(img_title, img) index = index + 1

This code takes a bunch of image data which had been serialized out as a NumPy array (e.g., just raw data). This code reads that with np.load, then writes it back out using cv2.imwrite, converting it to an image file.

In this tight little snippet, nearly everything is wrong.

We start with an enumerate(img_list). In Python, this returns a list of tuples- an enumerator value, and the actual value. Our for loop splits that back out into an index and fname variable.

We forget the fname variable exists, and get the filename again by doing an array indexing operation (img_list[index]).

We then slice the loaded array to extract just the dimensions containing image data, and that's fine. Then we generate a filename based on the index- which, it so happens, we already HAVE a good filename that we've opted not to use (in fname). That's not a WTF, it just annoys me- my usual path for these kinds of things is to read source data from one directory and dump converted data into a different directory with the same names. That's just me.

Then, for fun, we increment index. This, fortunately, doesn't break the loop, because we're setting index at the top of the loop, and thus the increment is overwritten.

This also tells us some history of the code. It originally didn't use enumerate, and just incremented the index manually. Someone mentioned, "You should use enumerate, it's more Pythonic," and so they added the enumerate call, without understanding what about it was more Pythonic.

As scientist written code goes, I wouldn't call this the worst I've seen. It annoys me, but at least I understand it, and it wouldn't be a huge effort to clean up.

[Advertisement] Utilize BuildMaster to release your software with confidence, at the pace your business demands. Download today!