Sloppy error checking in MS code

[Julian Field]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1



Ken A wrote:
> Glenn Steen wrote:
>> On 16/12/06, Matt Kettler <mkettler at evi-inc.com> wrote:
>>> Mike Jakubik wrote:
>>> > Recently on the postfix mailling lists, there was a debate on the 
>>> usage
>>> > of postfix  + mailscanner. I claimed that I've been using the two for
>>> > some time now, without any problems. However one of the folks on the
>>> > lists stated the following:
>>> >
>>> > ---
>>> > On Thursday December 14 2006 07:30, Mike Jakubik wrote:
>>> >> > Could you backup this documented fact please? Are you sure you 
>>> are not
>>> >> > referring to a bug in an obsolete version of mailscanner? If this
>>> > was an
>>> >> > issue, im quite certain some of my users would complain.
>>> >
>>> > Look at its code. There are literally hundreds of instances where a
>>> > status from a system call is just ignored (getline, print, close, 
>>> flush,
>>> > stat, mkdir, chown, unlink, exec, system..., sometimes even: open).
>>> > _Anything_ can happen, and you will never know what hit you,
>>> > much less be able to track it down easily, especially if the
>>> > problem is intermittent.
>>> >
>>> >  Mark
>>> > ---
>>> >
>>> > I'm not a perl expert, could someone verify if this is the case? 
>>> Can MS
>>> > really eat emails without any warning?
>>>
>>> Hmm, reading PFDiskStore.pm, it does look like many system calls 
>>> aren't checked.
>>>
>>> Some cases, it doesn't really matter, because a later system call is 
>>> checked,
>>> and that will fail if the previous one did. (ie: mkdir, followed by 
>>> a rename
>>> into that dir, where mkdir isn't checked, but rename is. Clearly if 
>>> the mkdir
>>> failed to make the directory, rename is going to fail and get caught 
>>> that way)
>>>
>>> There are some other cases where I can't tell if it's bad enough to 
>>> cause
>>> problems, or just bad form.
>>>
>>> I suspect that  the un-checked chown and chmod in this file could be 
>>> bad if they
>>> failed.. they *shouldn't* ever fail.. but at the same time, it would 
>>> be nice to
>>> check for failure and generate a message.
>>>
>>> It also would be nice if the unlink in DeleteUnlock() was checked, 
>>> much like it
>>> is in the SMDiskStore.pm for sendmail.
>>>
>>> That said, my code is NOT the latest, and Julian may have already 
>>> fixed both of
>>> the above..
>>>
>> I've done no such audit as you've started Matt... But really, if this
>> was a big problem, I'm more than certain we would've ... encoutered
>> ... it already.
>> I'm not saying that code audits shouldn't be done, and I'm usually a
>> stickler for sane error checking/handling. But I can't shake the
>> feeling this is motly just more of the usual FUD from the postfix
>> devel camp.
>>
>> Since switching to the HOLD method, I've only lost messages when I
>> fat-fingered the MS config... And Jules has subsequently
>> "idiot-proofed" those settings (the * Actions). Might be just dumb
>> luck, I suppose, that it hasn't happened, but I doubt it:-).
>>
>> That said, I'm really in forward of critically looking the PF code
>> over. I've been doing some looking, but then mostly to ascertain how
>> it works (or in some few cases... not), and to try determine whether
>> the PF crowd have any base for some of their allegations (unsafe
>> handling of queue files... AFAICS, this is untrue).
>> Unforunately I will have very limited time to help before X-mas/new 
>> years...
>> Then again, I'm not sure it's a problem needing immediate attention.
>>
>
> This kind of thing is really up to the author. I'm sure it's something 
> he's thought about and not done for a reason. One person's 'sloppy' is 
> another's 'efficient'. Programmers can tend to get a bit over zealous 
> about such issues. More often than not, the issue that has a 
> /potential/ to happen, never really happens in the software's 
> lifetime. So the only _real_ issue is a theoretical, academic argument 
> that most of us have no time for. :-\
Ken,
Thanks for backing me up. Yes, I don't check the return value of every 
single call I make, but show me a programmer who does? (No doubt someone 
will at this point). The most common point that is made is "what if the 
system runs out of space" at this point, and all sorts of things will be 
failing at this time, there isn't any need to create more errors at this 
point, they just create noise. Yes, I quite happily admit I don't check 
the result from everything I do. But if you want a practical piece of 
software that runs at a reasonable speed.

Don't forget. Every morning you make the assumption that the floor is 
there when you step out of bed. Show me the person who checks with a 
stick that the floor is where they think it is, before putting their 
weight on the first foot out of bed in the morning. Yes, you make 
assumptions too.

Jules.

Jules

- -- 
Julian Field MEng CITP
www.MailScanner.info
Buy the MailScanner book at www.MailScanner.info/store

MailScanner customisation, or any advanced system administration help?
Contact me at Jules at Jules.FM

PGP footprint: EE81 D763 3DB0 0BFD E1DC 7222 11F6 5947 1415 B654
For all your IT requirements visit www.transtec.co.uk



-----BEGIN PGP SIGNATURE-----
Version: PGP Desktop 9.5.2 (Build 4075)
Comment: Fetch my public key foot-print from www.mailscanner.info
Charset: ISO-8859-1

wj8DBQFFhEpdEfZZRxQVtlQRAmMbAKDhK1tvfzEssr/qAW4ZoitNlKU5hgCgg5nX
6Sg7uid31y+O0TBPmXSGCWc=
=7gKa
-----END PGP SIGNATURE-----

-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.
For all your IT requirements visit www.transtec.co.uk