z80playground / cpm-fat

CP/M for the Z80 Playground that runs on the FAT disk format
GNU General Public License v3.0
20 stars 9 forks source link

Unable to delete files as user x #47

Closed gd-99 closed 3 years ago

gd-99 commented 3 years ago

I don't seem to be able to "era" files as a non user 0. When "logged in " to a drive as user 0, I am able to delete files - era filname.ext.

To test this I have created a folder say "2" in drive "N" on my PC, I am able to change drive to N: then change user to user 2 and see any files placed in 2. I am also able to pip a file into this directory (N2:>) and use the copied files. However I am unable to delete any files copied over to folder 2.

When I change to user 0 (USER 0), so now I am N0:> I can pip files over and era them.

Is this just me or am I doing something wrong?

An oblique question, is there a utility that will display all user directories (1, 2, 3,...n) for a single or all drives A-P?

Many thanks.

gd-99 commented 3 years ago

Physician, heal thyself! I think I may have found a fix for the USER n bug. In essence when "logged in" as USER 1 - 15 (not 0), I found I could create /move /rename files on any of the drives A - P but couldn't then delete these files or any files placed there using the PC. The bug didn't manifest itself as USER 0. Feedback from doing an "ERA" in all cases indicated success, however as USER n (but not 0) by following an ERA with a DIR showed the file was still there. I assume this issue is seen by others and not something to do with my board.

In my implementation of this CP/M-FAT, I found I could solve the issue so that no matter which USER I was using I could correctly delete (ERA) files by changing a couple of lines in the BDOS.ASM file.

My fix: Edit BDOS.ASM and scroll down to the around line 600 under the BDOS_Delete_File: routine.

Go down to: ld a, 0 ld (delete_flag), a ; Store a success result ld de, temp_fcb

I have commented (;) out the two lines ;ld de, temp_fcb
;call copy_fcb_to_filename_buffer

...and inserted: pop de
push de call copy_fcb_to_filename_buffer_preserving_spaces

It may well be possible to do away with the pop and pushing with a bit of a rewrite of the routine? I was also not clear as to which copy_fcb... I needed. I opted for the preserving spaces version as this is used right at the beginning of the delete routine. Would welcome some guidance here.

Recompile to a bin file and copy onto the USB flash drive /memory stick. Reboot the Z80 and all should be fine.

To test, I simply select a drive - n: and do a save 1 test.txt to create a dummy file, do a dir to check a file got created, then do an era test.txt. Dir should confirm the file has gone. Repeat doing a user 1 first.

I would suggest testing this on a spare memory stick before using it for real, as I haven't exhaustively tested the solution and the fix could have unintended consequences, while erasing files...

It would be great if the fix could be double checked to see it is doing what it is meant to do and isn't dangerous to any other operations on the "disk".

z80playground commented 3 years ago

I'll look into this shortly!

z80playground commented 3 years ago

Yes, definitely can confirm that the bug still exists in release 1.09

gd-99 commented 3 years ago

Hi, thanks for the confirmation. Just to add a bit more detail. My fix above does work, however I have a feeling - not tested - the underlying issue may be that in the open_cpm_disk_directory: function (in BDOS ~line 480) the DRIVE_NAME variable gets used to set both the drive name (A - P) and then if USER is anything other than 0, then DRIVE_NAME is used to store the USER number.

So if user is 0, then the the procedure bails out with DRIVE_NAME set to the correct drive and all appears dandy. When USER is >0 then returning from the procedure DRIVE_NAME is not set to the correct drive and think this has something to do with where the delete fails.

My fix - |I think| works because by resetting the FCB to what was called at intry rather than the temp version, still has the correct drive name set, rather than the set to USER number. I think the fix is safe as the original call to DIR: has to be true (succeed) to be allowed to proceed to the actual delete bit of code.

z80playground commented 3 years ago

What happens is that the first byte of the FCB is used in two different ways under CP/M. When specifying a file it specifies the drive letter. But when you get a FCB back from a directory-search, it specifies the user who owns the file. I only discovered this in the last couple of months. It is a bit stupid, because both the user number and drive letter could have been put into that byte, with no problem, but that was not the way they did it! I can see the problem, I will fix it now and do a new release. Also worth testing if "ERA b:file.txt" works. I haven't tried that yet.

z80playground commented 3 years ago

Oh, and remember that you can "ERA *.TXT" too, so there are lots of combinations to try.

z80playground commented 3 years ago

I think it is fixed in release 1.10. Can you test please?

gd-99 commented 3 years ago

Tested and confirmed working /fixed.

Many thanks.