strcat+strcat+strcat == baaad"

[vkxzjq6lg at cswebmail.com]

"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: 
> 
>   1. 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.

> 
>   2. 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.
 
>   3. 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/