Define before use should be an enforced rule in production code

This post is a bit of a rant... but just a bit.

I've been at Frontbridge/Exchange Hosted Services for a while.  We were a startup in 2000 (long before my time) and like any startups, the way to get going quickly is to use LAMP technology - Linux (Operating System), Apache (web server), MySQL (database) and PHP (for web scripting).  Since these tools are all available free of charge, startups can use these as the backbone of their platforms.  Frontbridge was no different.

After the Microsoft buyout, we slowly started to migrate over.  Microsoft has lots of expertise with its own technology and it's way easier to recruit developers for that stuff than for the open source stuff; it simply makes sense to use native Microsoft technology.  Nothing new is written in MySQL, it's all done in SQL Server.  Nothing new is done in PHP, it's all done in ASP and C#.  Of course, the old stuff is still in LAMP.  We have to either maintain the old system and new system at the same time, or port everything over.  We did the former for a while with the goal of doing the latter.

Which brings me to my rant (note to my internal readers, there's no pun intended there... well, maybe just a little).  We have a lot of stuff in PHP and this past week we've been moving a bunch of stuff over to backup servers and getting additional redundancy built in.  To that end, since I've been there for a while and know how much of the old Linux system works, I am on the crew that is moving stuff over.  Much of this stuff is scripts written in Perl and PHP.

Now, all of the Perl stuff that I wrote personally migrated over just fine.  Just copy the script and a config and bada-boom, bada-bing, it's up and running.  But the PHP stuff is another story.  I'm not good with PHP.  But I had to migrate a script over.  I read through the code for the script and it looked simple enough.  So I copied it over and ran it.  There were piles and piles of errors written to the screen!  It worked on the old server just fine but not the new server.

I opened up the code and looked again.  I looked at the error; I backtracked to a variable that wasn't set properly, that is, it wasn't defined before it was used.  So I looked to where it was set.  It was set in a subroutine with an if-else statement.  Fair enough, where was it initialized?  I hunted it down and found it wasn't initialized anywhere.  Basically, the script assumed that variable was initialized.  If you try to do a MySQL insert or print to stdout and the variable isn't initialized, then errors get printed to the screen.  It took me a long time to figure that out.

Now here's the thing-- this script worked just fine on the old server which is running an older version of PHP.  The new server was running a newer version of it.  So there were no errors on the old version, but there were on the new version.  No one has touched this code in three years, and nobody who wrote it is still with the company.  And I'm not a PHP coder.  The bottom line is that we have a lot of old scripts that assume either the existence of files on the file system or use variables that are not initialized before use.  That caused me a lot of pain.  All I wanted was to move the script to another server, I didn't plan on debugging scripts that we want to migrate off but can't at the moment.

The moral of the story is this: initialize your variables to default values and don't assume that stuff exists.  If you ever end up working for me and you don't do this I will hunt you down and beat you with a stick.

Comments (4)
  1. David Cawley says:

    A few months ago I came across a bug where the code itself hadn’t changed but something had broken – a process would fail trying to open /etc/protocols.

    It turned out to be an incompatibility between glibc 2.7 and a 2.6.9 kernel. We had previously used glibc 2.6 but wanted to upgrade to the more recent release. We had to build it explicitly requesting support for older kernels to maintain backward compatability.

    Although our variables are initialized (don’t beat us with a stick) and files aren’t assumed to be there, we still hit on this bug. The moral of the story was to be very aware that version changes of any part of the system can break things even if the code remains static. As was the case in the bumping of your php version.

  2. Ryan says:

    Any time an interpreted language goes up a major version number there’s usually a change in backwards compatibility.  Usually these are documented extremely well in the release notes and can be accounted for by simply going over what’s changing and updating it to the new standards.  If it’s been three years it might be several major revisions later, and the work load grows enormously.

    Neglecting code for years because it works with a really old version of PHP is not best practice.  Keeping up to date with updates is important, as is keeping code current.  Especially in something like PHP where there are fairly regular security patches.  Even internal-only tools need a measure of safe-guarding.

  3. Winner says:

    Afaik variables are always initialized to NULL, independet of the php version. The big exception is register_globals=on, which is probably disabled by default in newer versions and enabled in older versions (I’m not sure about that as we always had set it to off).

    If you set the warning level high enough you also get notified when you access an uninitialized variable, which helps with debugging.

Comments are closed.

Skip to main content