code practices: Re: Linux Steam players should not move the Steam directory

Michael Paoli Michael.Paoli at cal.berkeley.edu
Fri Jan 16 15:28:37 PST 2015


Amazing what some, uh, "programmers" (or those attempting to do that)
will fail to check.

Once upon a time, production system, some more junior level system
administrators had been taking care of the hosts - and with not (quite?)
enough oversight.  So, one of those production applications got migrated
to a new host - substantially different (mostly newer upgraded)
hardware, but still same (or nearly same?) basic OS version ... but due
to fair bit of hardware differences (notably a lot more disks and disk
space - notably to accommodate the growing application), things
appropriately and rather necessarily, weren't laid out exactly as they
were before.  Anyway, one of the bits of code in a crontab driven job
looked an awful lot like this:
#!/bin/sh
cd /some/log/directory/somewhere
find * -type f -mtime +30 -exec rm -f \{\} \;
... oh, and of course run by root, and root's HOME directory: /
Guess what happened the very first time that cd failed after the
migration?  <sigh>
Wee bit of attention to detail and as little as two extra characters in
the code (e.g.: &&, || exit, or set -e) could've saved the day, ... but
no, the day (and then some) was lost, when the host dutifully committed
seppuku precisely as it had been instructed to do, and we were left to
repair the damage, and determine cause.  And I think that crontab job
only ran monthly ... so all seemed perfectly fine for week(s) or more,
... until ... yeah, not pretty.  Anyway, also best not to be remembered
as the one who destroyed critical production with sloppy code (same
would also have occurred pre-migration if that cd ever failed - e.g. if
that filesystem wasn't mounted when the crontab job ran).

Another case, not so ancient, I'd written some very functional working
prototype code.  It was quite production quality, but the requester
probably wanted to tweak the look and feel slightly, and some other
details.  But the requester deemed my code "too complicated", and wanted
to rip out about 2/3 of the code.  Notably most all the error checking
and safety checking bits.  I told said requester they could do that,
but at their own risk (and to production, as they were going to also be
running this code against production), and all those "we don't have to
worry about that, that won't be the case", and lots of other checks,
they wanted to remove.  But by removing them, they pretty much ensured
unexpected results, up to and including major disaster, would occur if
the code was run with those checks removed, if any of those "we don't
have to worry about that, that won't be the case" conditions (along
with some others) ever occurred where the code was run (even as simple
as rerunning the same code a second time, when it need only be run once
on any given host).  I told them they could take any and/or all of
those checks out ... at their own risk/peril ... but if they remove any
of them, remove my name from the code.

I've also, far far too frequently seen highly abominable code by 3rd
party vendors - and yes, including for Linux.  Some of 'em even *sell*
"product"/code that I wouldn't even barely deem to be Alpha quality -
let alone Beta, or, egad, installed into and running in production!

Some places won't let code like that get anywhere *near* production! :-)
Others, uhm, well, managers override and ... well, sh*t happens - sooner
or later.

http://www.rawbw.com/~mp/unix/sh/#Good_Programming_Practices



> From: "Rick Moen" <rick at deirdre.net>
> Subject: Re: Linux Steam players should not move the Steam directory
> Date: Fri, 16 Jan 2015 14:28:00 -0800

> On Fri, Jan 16, 2015 at 2:04 PM, Samir Faci <samir at esamir.com> wrote:
>> The issue is that
>>
>> "$STEAMROOT/"   becomes  / when it's unset.
>
> OK, yeah.  But in context that's just as bad.
>
>> What they should have done is check if the parameter is set and error out
>> way before using the variable.
>
> Quite so.  An environment variable used in a recursive rm should not
> be simply assumed to be set.
>
> People who do that probably can't be trusted to do other basic things
> like input validation, bounds checking, and so on, either.




More information about the sf-lug mailing list