FDOS / share

Installs file-sharing and locking capabilities for FreeDOS kernel. (Not compatible with other DOS kernels.)
11 stars 5 forks source link

Is there interest in adding an Uninstall option? #10

Closed ecm-pushbx closed 2 years ago

ecm-pushbx commented 3 years ago

Coming from the conversion of FDAPM I considered adding an Uninstall option, as well as the needed (AMIS or otherwise) multiplexer to do so, to SHARE. Is there interest in such an addition?

I could probably add most things needed (multiplexer in resident segment, UnhookInterruptSim and friends for the transient uninstaller) through the gcc_help assembly file. However, that would not cover the Turbo C case. Perhaps a new assembly source file can be added, to compile either to an OMF .obj for TCC or an ELF object for gcc?

Other than the standard "unhook interrupts, free memory block" uninstall process, I gather that the open SFT entries may hold values in the two fields dedicated to SHARE. I assume that an Uninstall option should clear these out to all-zeros or all-ones to match a system state akin to the kernel running without SHARE ever having been loaded.

PerditionC commented 3 years ago

Yes. Although share is small enough it is probably fine without ability to uninstall, however, if it's not overly difficult to do, then would be great to have. It would mean I could load share, start program that requires it, unload share. The next release will be built with GCC so that is the most important one to support. I do think some reorganizing into c and asm files may be useful to ease supporting Open Watcom, GCC and Borland compilers, but I haven't had time to really study the code to see if this is worthwhile.

ecm-pushbx commented 3 years ago

I implemented most of this, as well as some small tweaks installing the resident, in my AMIS branch. Like for FDAPM we use the traditional interrupt 2Fh multiplex interface to detect presence for installing, but use the AMIS interrupt 2Dh to search the resident for uninstalling. I added the switch /U (and an alias /R) to request uninstalling. The uninstaller is mostly my original assembly code taken from the TSR example, modified to preserve all 8086 registers except AX, which gets a return code. (Unlike the TSR example I went with a few hardcoded messages, not the detailed interrupt list output of the example.)

Assembling the source requires AMIS.MAC (shipped in the SHARE repo now) as well as the lmacros collection. All my contributions are Public Domain or available under the Fair License, compatible to any GNU GPL version of course.

I did not yet add the clearing of the SFT references to SHARE tables. These are apparently only set in the kernel so I will have to examine that to figure out how to do that. This should be addressed to complete this patch.

PerditionC commented 3 years ago

Cool looks good. How would you feel if I changed the vendor to dosc ? You did the work so I'm ok leaving it as ecm, just asking. I will have a look at clearing the kernel references when I get to my computer

ecm-pushbx commented 3 years ago

How about "DOS-C" or just "FreeDOS" for a vendor string? I'm not married to putting in my initials, I merely defaulted to it.

I added a commit to reset the SHARE record number of every open file. This is done after a successful uninstallation. It gets the SFT entry size dynamically using int 2F.1216, though current FreeDOS always uses 3Bh bytes per emtry. Then it calls int 21.52 and gets the first SFT table address from dword [es:bx + 4]. It appears to work fine generally. I had to create a FAT FS drive (empty diskette image done with bootimg) to actually test this because it appears that file sharing for dosemu2 mfs-redirected files is left to the redirector and never calls SHARE.

Still to do:

ecm-pushbx commented 3 years ago

Still to do:

* Document / link to kernel sources for the record number behaviour

* Check whether SHARE properly handles calls with unset record numbers (generally FFFFh, possibly other values >= 8000h).

It appears the kernel never calls SHARE for invalid record numbers, see dosfns.c

* Check whether the kernel properly frees an SFT entry in dosfns.c if SHARE denies opening it

This appears to be correct; while calling SHARE the SFT entry still has a zero for sft_count. The field is set to 1 only after SHARE returns with success.

* Patch kernel so as to reset its "SHARE is installed" flag if SHARE is unloaded (it appears not to do this yet)

I collected a few kernels and checked their IsShareInstalled functions as well as the the share_check function. The disassemblies are provided in the text files in this directory: https://pushbx.org/ecm/test/20211028/

I can add an assembly handler for int 2F.1000 that checks for this function then remembers the offset of the share_installed variable in the DOS data segment. With a small AMIS interface function we can expose this address to the uninstaller, which can clear the variable to correct the kernel so as not to think SHARE is still loaded.

Obviously, going forward the kernel should be patched so as to check for SHARE even if the share_installed flag is set, and to clear the flag if the check returns it is no longer installed. I suggest to add a test ax, "US" (Uninstallable SHARE) instruction after the int 2Fh that calls 2F.1000, so my handler can learn that the kernel does not need the manual intervention of the uninstaller to clear the share_installed flag.

ecm-pushbx commented 3 years ago

I tried compiling with the latest gcc ia16 and make all COMPILER=gcc but the resulting kernel crashes some time after init disks.

ecm-pushbx commented 3 years ago

Operator error. I ran a scriptlet to build a diskette image (using my bootimg and FAT12 boot sector loader) and accidentally had it include an autoexec.bat but, without my knowledge, this was overridden by the batch file found in the kernel 2043 release bin/ subdirectory. When I tried my gcc-ia16 kernel build it took the other autoexec.bat originally used for this scriptlet, which had a wrong filename and then chained to quit.com which duly terminated the machine. So that was a waste of 90+ minutes of debugging. (Though it did acquaint me with the kernel more than before.)

I tested the patch to the kernel and will open a Pull Request next.

ecm-pushbx commented 3 years ago

I just uploaded several new commits. Added the /S (show status, only the kernel variable patch status for now) and /O (do not install only operate if already resident) switches.

I also added a small AMIS interface to get the patch status, and handling in the int 2F.10 code to check for the sequences relevant to the patching. Tested on the 2042 kernel I have around and the gcc-compiled kernel I just built today with the change to clear the share_installed flag. On the older kernel, after accessing a file on a FAT FS (non-redirector) drive the /S display indicates that the patch is needed, and a successful /U will indicate that it zeroed the flag by way of patch.

Also, I added a loop to close all file handles before going resident. As explained in the comment this is to avoid system file handle leakage.

ecm-pushbx commented 3 years ago

Added more status display (free and total table amounts) and for that global variables (no longer static) to keep track of amounts, and an AMIS interface to read the resident's values.

I also added the comments referring to the sft_shroff field re initialisation. While doing that I figured the redirector may use the field itself, and we never use it then, so I made it check for redirector (and character device) SFT entries and skip those.

The only thing remaining before I open the Pull Request is whether to change the AMIS vendor string, and to what.

PerditionC commented 3 years ago

Wow, great work. I like some variation of DOSC but I have no problem merging without changing it.

ecm-pushbx commented 3 years ago

Set it to "DOS-C" now, PR is at https://github.com/FDOS/share/pull/11