In DOS, the second handle sees the file size as what it was before the first handle wrote to the file (from the directory entry, I guess). In DOSBox, the second handle sees the revised size of the file in blocks of 4kB, i.e. any residual portion less than 4kB is not yet written to disk. It smells like a write buffer is employed by fwrite() or the host OS, so the behavior could vary between C runtime libraries and platforms.

This all uses one file handle opened for writing, and another for reading from the same file, right?

If you are referring to the flags, both handles on the file are read/write. If you mean how the program uses the handles, I haven't tried to determine if it purposes the handles separately for reading and writing. It definitely uses INT 21/4202 to get the size of the file from the second handle, and uses that value in various other seeks and writes, and when it reads the wrong size because the first handle is not entirely flushed to disk, it damages the save files.

An approach that might work (in theory): when a seek based on the end-of-file as origin occurs, a sweep is made through all open handles, and if another handle is found to be for the same file, flush it before doing the seek. I don't know if the file handle structs will support it as they are now. However, it may depend on if the size of the file is determined from the size at the time the file is opened vs. when the actual seek takes place.

Another possible solution that appears to fix the problem with Antara:

1 bool localDrive::FileOpen(DOS_File * * file,char * name,Bit32u flags) {
2     const char* type;
3     switch (flags&0xf) {
4     case OPEN_READ:type="rb"; break;
5     case OPEN_WRITE:type="rb+"; break;
6     case OPEN_READWRITE:type="rb+"; break;
7     default:
8         DOS_SetError(DOSERR_ACCESS_CODE_INVALID);
9         return false;
10     }
11     char newname[CROSS_LEN];
12     strcpy(newname,basedir);
13     strcat(newname,name);
14     CROSS_FILENAME(newname);
15     dirCache.ExpandName(newname);
16
17     FILE * hand=fopen(newname,type);
18 //  Bit32u err=errno;
19     if (!hand) { 
20         if((flags&0xf) != OPEN_READ) {
21             FILE * hmm=fopen(newname,"rb");
22             if (hmm) {
23                 fclose(hmm);
24                 LOG_MSG("Warning: file %s exists and failed to open in write mode.\nPlease Remove write-protection",newname);
25             }
26         }
27         return false;
28     }
29+    if ((flags&0x70)==0x40) setvbuf(hand,NULL,_IONBF,0);
30
31     *file=new localFile(name,hand);
32     (*file)->flags=flags;  //for the inheritance flag and maybe check for others.
33 //  (*file)->SetFileName(newname);
34     return true;
35 }

It does have a pronounced effect on performance with my "lots of small writes" test, but that's to be expected; and that test isn't really representative of how games and apps write to disk, because it would kill performance in real DOS as well to do it that way.

Here is an implementation of the idea I mentioned before. It targets the issue better than blindly flushing every write or disabling buffers, and doesn't have the write speed drawback of those approaches. So far it has been stable and effective in my tests.

Looks okay at first glance.
Is this really enough for the game ? To have file size updated on Open ?
I once messed around with something like this when fixing starflight 1 and 2 with FCBs.

It appears to be sufficient; I have saved and restored many games without problems. I guess it would be good to test with saves made deeper into the game, but I don't see why the issue wouldn't remain the same.

Was there anything in your solution for FCBs that could be applied to this case?

the games wrote the a fcb, closed it, and reopened and wanted the time/date to be accurate
revision 673 in the svn

If you want to implement my line of reasoning which I had at that time, then you would need store the new file size on writes and use that info when opening instead of reading it from the disk. think a flush is much simpler.

How do i use the Open_again.diff file from within dosbox ? or windows? There is no .com file with this one and i am not that good with code and visua studio yet.
Do i rename the file or something ? Or is this file type only for linux ? Or is there a program i need to apply this? i never seen the filetype before anyway...

After examining the code in drive_local.cpp further, I noticed the last_action variable is used in logic to perform a seek of the current position when changing between read and write operations; and when a normal seek occurs the last action is set to NONE so seeking the current position won't be done at the next read or write. There are no comments in the code about it, but I think the seek of the current position is used to perform a flush (which is a side-effect); and there IS a comment in the code about fflush causing problems in Visual C, which may support the conclusion.

The attached patch is a revision that uses a fseek instead of fflush like the other local file methods, and also makes use of the last_action==WRITE condition to limit where the flush is performed. The first version of the patch would, in the case of making saves in Antara, flush a few times because handles were writable yet had not been written to. Overall this revised patch is a bit more consistent and integrated with existing code, and also a bit more efficient by avoiding unnecessary flushing.

you might want to check the result of the dynamic cast. It is somewhat far fetched, but opening z:\command.com and c:\command.com might trigger it.

The reason I use the dynamic cast is to avoid creating dummy flush methods in all dosfile classes, but I understand what you mean about making sure there's a valid pointer.

Heh, let's hope nothing is trying to write to command.com 😉

Actually, I spent some time trying to figure out how to deal with the drive, because the filename ("name" var) passed into the drive methods has the drive letter removed because it's implied. I wanted to have a condition like (Files->GetDrive()==drive && Files->IsName(name)), but couldn't figure out how the drive object knows what its drive number is. BTW, I'm using the file array scan in the unlink method as an example, and it doesn't deal with drive either...

I think ioctl calls have the drive letter in their result (lower part of the status word)

but the pointer should be checked as saveguard nonetheless as accessing a failed dynamic cast is accessing a nullpointer.

Not sure what you mean by ioctl calls in relation to a drive class. What method or property of drive classes has the drive number?

I understand about checking the dynamic cast pointer, it's no problem. However, cdromDrive::FileOpen() does not check the result of a dynamic cast, and I was using it as an example.

Added the drive check and casted pointer protection. Would be nice if there's an easier way for a drive class method to know its drive number.

well an easier way is not very hard to add.
But currently this is the only function that requires it. So lets keep to the current system.
btw maybe init drive to 0xff or DOS_DRIVES instead 0. This way it can't match by accident.
With our current code a drive should always be in the Drives[] table, but who knows what the future will bring.

Yeah, I thought about the init value (since I even put one at all in case the loop came up with nothing), but I was thinking "it has to be in there..."

Well, it's not a big patch by any means, but it certainly has evolved a bit under your critical eye from the one-liner hacks I started with. 😀

A little optimization, because most of the work is done iterating through the file array. The drive test is true most of the time, but the filename test is very rarely true. With the drive test first, it performs both tests most of the time; but by putting the filename test first, the drive test is only rarely performed because of short-circuiting.