dosemu2 / fdpp

FreeDOS plus-plus, 64bit DOS
GNU General Public License v3.0
198 stars 18 forks source link

DJGPP _media_type() returns 1 for MFS on FDPP #156

Closed andrewbird closed 3 years ago

andrewbird commented 3 years ago

I'm trying to refine some lock test on dosemu and I have to check for filesystem type FAT/MFS somehow. With DJGPP there's a library call _media_type() that looks able to help. Here's the man page http://www.delorie.com/djgpp/doc/libc/libc_561.html. So as I understand it, it should return 1 for fixed disk, 0 for removable, and -1 others. This is the case with the other DOSes I'm testing:

Test DR-DOS-7.01 MFS Media type                                                  ... ok (  3.10s)
Test FR-DOS-1.20 MFS Media type                                                  ... ok (  2.37s)
Test MS-DOS-6.22 MFS Media type                                                  ... ok (  3.20s)
Test PP-DOS-GIT MFS Media type                                                   ... FAIL

< snip >
C:\>testit.bat
testit.bat

C:\>d:
D:\>rem Internal share
D:\>c:\meditype
INFO: _media_type(0) returned [1]
D:\>rem end

======================================================================
FAIL: Test PP-DOS-GIT MFS Media type                                                  
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test/test_dos.py", line 3555, in test_mfs_media_type
    media_type(self, "MFS")
  File "/clients/common/dosemu2.git/test/func_media_type.py", line 55, in media_type
    self.assertIn("[-1]", results) # fail
AssertionError: '[-1]' not found in 'testit.bat\r\r\n\r\r\nC:\\>d:\r\r\nD:\\>rem Internal share\r\r\nD:\\>c:\\meditype\r\r\nINFO: _media_type(0) returned [1]\r\n\r\r\nD:\\>'

So we can see that FDPP returns 1 (fixed) for MFS and the other DOSes return -1.

andrewbird commented 3 years ago

Poking about in the djgpp sources this seems to be the impl.


#include <dpmi.h>
#include <dos.h>

int
_media_type(int drive_num)
{
  __dpmi_regs r;

  r.x.ax = 0x4408;
  r.h.bl = drive_num;
  __dpmi_int(0x21, &r);

  if (r.x.flags & 1)
    return -1;
  return r.x.ax;    /* returns 1 for fixed disks, 0 for removable */
}
andrewbird commented 3 years ago

It looks like it was intentional 5f06d9e32, so not sure what to say. It's incompatible with the others but it was a fix to help windows installer.

stsp commented 3 years ago

One thing that is obvious is that other doses were not tested with all drives being remote, while fdpp is going to work in that condition. So pretending remote drive is fixed, makes sense. OTOH #113 says it worked perfectly with ms-dos, and now you claim ms-dos does not do this?

stsp commented 3 years ago

Also I think its quite wrong to test for local/remote. Why not to apply some creativity to figure out if the locking behaviour is good? If overlapping locks conflict - good. If not conflict - unlock a few bytes in an overlapping region, and if success - see if they are readable from another fd. Doing separate tests means that you do not consider mfs behavior as an extension, but rather as something incompatible.

andrewbird commented 3 years ago

Mmm, I wonder if that function can ever return 0 for floppy etc, as it looks to me that it either returns fixed or drive doesn't exist.

Doing separate tests means that you do not consider mfs behavior as an extension, but rather as something incompatible.

Not separate tests per se, but it does determine whether to flag it as fail, or just print info and continue testing.

Yep, it's true I have no love for it. Second locks from the same handle on FATFS succeed or fail due to range conflicts, whereas on MFS they always succeed no matter what and the caller can't tell from the result whether it was due to the extension or not.

Why not to apply some creativity to figure out if the locking behaviour is good?

It's a dance that few people, if any, are going to do after they get a lock. To my mind extensions should only be incompatible with the standard by choice. If an app wants range coalescing, the call should be augmented somehow to select the new behaviour, a normal call should produce the standard behaviour.

stsp commented 3 years ago

Lock coalescing is neither a documented nor forbidden behaviour. What you call "standard behaviour" is only based on your own reverse-engineering attempts. Standards are not based on reverse-engineering.

People who apply the lock to the file, do not care about a lock coalescing - its an implementation detail. What they do care about is whether the locked region cannot be read from other fd and cannot be re-locked from other fd. IMHO you can come up with any sorts of extensions as long as this is satisfied.

I would be very, very surprised if some sane program tries the overlapping locks on same fd and willingly expects them to fail. No, the program never does anything that willingly fail. What should fail is the lock when ANOTHER program also took one. And if it did so, it did on another FD. So I claim that lock conflicts on the same FD do never matter in practice. Even locktest.exe doesn't check for any specific policy of that. Who uses conflicting locks on same FD? No one does. What is it good for? - Absolutely nothing! :)

Well, conflicting locks on same fd test has very little value anyway. Read() tests on same fd should probably be enough, and no need to check for any extensions.