"Dimitrie O. Paun" wrote:
On December 5, 2002 11:16 am, Alexandre Julliard wrote:
It's the kind of thinking that leads to having an exception handler inside strlen() like Windows does. It's just plain wrong.
<nod/>. This is generally true for other things as well:
Many places in our code where we have invariant like pointer p is never NULL, yet we always have:
if (!p) return NULL; /* or whatever error condition */
Such a check is harmful (hides bugs), slow (useless runtime check), bloat (more generated code), ugly (clutters the code), at the very least. Just Don't Do It (TM) -- let the code crash.
Agreed that it's harmful because it hides what may be a real error. Lousy idea unless p really is allowed to have that value.
If it's a call exposed to external callers it needs to warn somehow, even to the console if necessary (assuming that Wine wants to give end users some feedback they can give back to the Wine developers and to the developer of the faulty application). Those warnings can vanish should there ever be a production build with debugging removed. If it's internal only it should be an assert or fixme.
Being too free with "let the code crash" is a bad idea. Microsoft got a lot of really bad heckling in the Windows 3.1 era for GPFs. Wine doesn't want to crash if it's reasonably harmless to continue (like for a failed screen update). Does want to report the problem, of course, rather than silently ignore it, otherwise it'll never get fixed.
- ZeroMemory when we don't need to, when we pass struct internally, yet same functions can be called externally with no guarantee that the struct has unused field zeroed out.
Agreed if it is a call to an internal only function. If it's to a function with an external API then it may need zeroing to get to the state required for that.
- In general, we have all sort of 'defensive programming' checks for invariants. Problem is, we never going to catch bugs in this area this way. Just use assert() if you feel a check is needed.
If it's a documented external interface it can't assert and die because the application making the call presumably works on real Windows. So it needs to be warned/fixme'd or whatever so end users can report the issue to Wine developers for fixing. If it's internal, assert is sort of OK, though as someone trying to get an application working I want a fixme rather than sudden death (except in file output interfaces!). I don't need perfect functionality. I do need to get the app to the point where it is safely functioning and an assert may kill it when it doesn't need to die.
In short, this defensive programming crap is just that: crap.
Do you agree with the way I've explained how I see the difference between redundant defensive programming and detecting and reporting problems which need to be fixed? If not, where do you differ and why?
James
___________________________________________________ The ALL NEW CS2000 from CompuServe Better! Faster! More Powerful! 250 FREE hours! Sign-on Now! http://www.compuserve.com/trycsrv/cs2000/webmail/