A few comments about this patch by Eric Pouech.
Structure DosHeap must not be moved to winedos. There already exists per drive media ID byte which should be used and biosdate, while it may be needed by some programs, won't do any good in a separate 16-bit segment. Also, INT21_CreateHeap is unnecessary.
IOCTL 0x0860 and CreateBPB should include FIXME comments because it does not check real drive types nor real drive geometry. The same is true with other IOCTLs that return fake values.
INT21_NetworkFunc should return NETBIOS name. There is a separate Win32 API for returning proper names, don't use gethostname.
INT21_GetDriveAllocInfo should pass drive as a parameter, rewriting registers makes code less readable.
As far as I know, TRUENAME implementation is broken. Add big FIXME notes to that subfunction.
Use INT21_MapDrive instead of manual conversion from zero=default into zero='A' convention, that is why it is there.
Use wide characters and manual wide character conversions. DOS routines use OEM code page and not ANSI code page and Wine uses wide characters as default character set, anyway. Also, adding drive number to 'A' may work incorrectly with illegal drives and 8-bit character set but it works fine with wide character set.
Jukka Heinonen wrote:
A few comments about this patch by Eric Pouech.
Structure DosHeap must not be moved to winedos. There already exists per drive media ID byte which should be used and biosdate, while it may be needed by some programs, won't do any good in a separate 16-bit segment. Also, INT21_CreateHeap is unnecessary.
IOCTL 0x0860 and CreateBPB should include FIXME comments because it does not check real drive types nor real drive geometry. The same is true with other IOCTLs that return fake values.
INT21_NetworkFunc should return NETBIOS name. There is a separate Win32 API for returning proper names, don't use gethostname.
INT21_GetDriveAllocInfo should pass drive as a parameter, rewriting registers makes code less readable.
As far as I know, TRUENAME implementation is broken. Add big FIXME notes to that subfunction.
Use INT21_MapDrive instead of manual conversion from zero=default into zero='A' convention, that is why it is there.
Use wide characters and manual wide character conversions. DOS routines use OEM code page and not ANSI code page and Wine uses wide characters as default character set, anyway. Also, adding drive number to 'A' may work incorrectly with illegal drives and 8-bit character set but it works fine with wide character set.
half of what you (rightfully) suggest to fix had been broken for very long in msdos/int21.c (so don't blame me too much... at least not for that half) I hope anyway I didn't step on your toes with this patch... Do you have some work going on in this area ?
A+
On Mon, Nov 10, Eric Pouech wrote:
half of what you (rightfully) suggest to fix had been broken for very long in msdos/int21.c (so don't blame me too much... at least not for that half)
Yes, I know msdos/int21.c contains lots of broken code and I'm pretty sure your patch did not add any new bugs. I just would prefer that instead of moving broken code around, broken code would be removed from msdos/int21.c and fixed code would be added to winedos/int21.c or at least comments would be added to broken code saying that code is broken and it needs to be fixed. This is because Wine DOS emulation was horrible mess before I started working on it and it is still a big mess after all my work and I really don't want to see patches that make the mess worser than it is.
I hope anyway I didn't step on your toes with this patch... Do you have some work going on in this area ?
Well, I have been planning to write a patch that migrates those int21 subfunctions but unfortunately I have not had the time to do it. And as DOS event handling is currently more or less broken, I guess migrating int21 subfunctions has even lower priority than it had before.
So, if you want to submit patches that migrate code to winedos, I won't really mind, as long as you try to either fix or comment obviously broken code (Ralf Browns Interrupt List is really helpful) and you try to make as much use as possible of existing winedos code.
Use wide characters and manual wide character conversions. DOS routines use OEM code page and not ANSI code page and Wine uses wide characters as default character set, anyway.
Regarding this part, I think we'd better rely for int21 support to the SetFileApisToOEM and SetFileApisToANSI APIs (and or course implementing them correctly). I'd suggest changing the oem_files_api to a krnl_file_api_codepage (or something like this) which would be set to either CP_ANSI or CP_OEM and use this variable in most of file related A functions (instead of our current CP_ANSI). Then we would set OEM APIs on int21 entry (and reset to the old mode on exit). I assume we hold a lock while executing the int21 API, so preventing some multi-threading issues on this. Any comment before I go on? A+
Eric Pouech pouech-eric@wanadoo.fr writes:
I'd suggest changing the oem_files_api to a krnl_file_api_codepage (or something like this) which would be set to either CP_ANSI or CP_OEM and use this variable in most of file related A functions (instead of our current CP_ANSI). Then we would set OEM APIs on int21 entry (and reset to the old mode on exit). I assume we hold a lock while executing the int21 API, so preventing some multi-threading issues on this.
I don't think we want to switch the global flag on every int21 call, that's fairly ugly IMO. If int21 needs OEM then it should use OEM explicitly, which is pretty much what it's doing already AFAICS. I see no reason to change that.
I don't think we want to switch the global flag on every int21 call, that's fairly ugly IMO. If int21 needs OEM then it should use OEM explicitly, which is pretty much what it's doing already AFAICS. I see no reason to change that.
I do think it's as ugly as duplicating all the A => W calls from kernel32. A+
-
Alexandre Julliard -
Eric Pouech -
Jukka Heinonen