Int21 fixes - Update

Jukka Heinonen jhei at iki.fi
Sun Aug 24 07:27:50 CDT 2003


I have to admit I'm a bit disappointed with this patch:
- Patch name should really be more descriptive.
- Changelog is not up to date with patch.
- This patch cannot be compiled with a C compiler. This makes it
  look like this patch has not been tested at all...
- There are logic errors in the patch.
- You should really either clear CFLAG at start of each
  subfunction or clear it on success. Using both approaches 
  makes the work of future int21 workers more difficult.
  There may be cases where you need to use the other approach
  but those cases should be rare.
  

Here is a quick review of the patch. There could
be more bugs still left in the patch:


ioctl : You really should consider all IOCTL subfunctions before trying to modify
        CFLAG behaviour. Putting one top level RESET_CFLAG with FIXME comment would
        be fine but fixing one branch and leaving other branches alone is not.
0x2d : not ok. First of all, illegal time is not an error. Some programs seem
       to use it as installation check, see subfunction 0x2b.
       Second thing is that you only reset CFLAG here and do not set it.
       Third thing is that I don't think this subfunction touches CFLAG at all.
0x38 : not ok. It would be pointless to return an error here because
       it only makes fewer programs work. Just add RESET_CFLAG and leave
       the logic as it is. Besides you have broken bSetDOSExtendedError handling here.
0x39-0x43 : ok
0x45 : not ok. Missing brackets.
0x46-47 : ok
0x48 : ok (see 0x5c)
0x4a : This is not valid C. RESET_CFLAG cannot happen before 
       variable declarations. Move RESET_CFLAG to start of subfunction.
0x4b : ok
0x56 : ok
0x57 : ok
0x5b : ok
0x5c : not ok (cosmetic bug). If you decide to use 
       if-then-bSetDOSExtendedError-else-RESET_CFLAG you really should try to use it
       everywhere. Or you could use RESET_CFLAG as first operation in each subfunction.
       But try not to use mixed approaches, if it can be helped.

-- 
Jukka Heinonen <http://www.iki.fi/jhei/>



More information about the wine-devel mailing list