Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lfn patch #105

Closed
ghost opened this issue Aug 11, 2015 · 30 comments
Closed

lfn patch #105

ghost opened this issue Aug 11, 2015 · 30 comments

Comments

@ghost
Copy link

@ghost ghost commented Aug 11, 2015

Ported lfn patch (by Wengier) to dosbox-x (posted at http://pastebin.com/CBDbj9GA). However, the patch requires a few additional changes which are not yet in the patch:

A. Set -fpermissive (using mingw) to bypass error in drive_cache.cpp: invalid conversion of void* to dir_information*.

B. hardware.cpp, lines 151,152 require a change for compatibility with modified parameters (commented out for now):


    //bool testRead = read_directory_first(dir, tempname, is_directory );
    /*for ( ; testRead; testRead = read_directory_next(dir, tempname, is_directory) ) {
        char * test=strstr(tempname,ext);
        if (!test || strlen(test)!=strlen(ext)) 
            continue;
        *test=0;
        if (strncasecmp(tempname,file_start,strlen(file_start))!=0) continue;
        Bitu num=atoi(&tempname[strlen(file_start)]);
        if (num>=last) last=num+1;
    }*/

C. Patch is missing a change to shell_cmds.cpp (removal of that bracket to close a loop):


    if (!optB) WriteOut(MSG_Get("SHELL_CMD_DIR_INTRO"),sargs);
-   }

D. And another edit, but must verify whether parameter value should be true and/or false:


        Bit8u targetdrive = (args[0] | 0x20)-'a' + 1;
        unsigned char targetdisplay = *reinterpret_cast(&args[0]);
-       if(!DOS_GetCurrentDir(targetdrive,dir)) {
+       if(!DOS_GetCurrentDir(targetdrive,dir,true)) {

E. An optional change: isinf() not defined by mingw (see fpu_instructions.h).

note: github auto-formatting is not ideal.

@ghost
Copy link
Author

@ghost ghost commented Aug 12, 2015

Updated patch here: http://pastebin.com/HpSAdVqZ. This has the above changes for A through D, although A, B, and D could use further verification.

Also, noted that mingw32 doesn't have isinf(), but does have std::isinf():


-       if (isinf(fpu.regs[st].d) && isinf(fpu.regs[other].d)) {
+       if (std::isinf(fpu.regs[st].d) && std::isinf(fpu.regs[other].d)) {
joncampbell123 added a commit that referenced this issue Aug 16, 2015
@joncampbell123
Copy link
Owner

@joncampbell123 joncampbell123 commented Aug 16, 2015

I applied the patch. It seems to have broken path lookup. It's also a bit Windows-centric in that some new functions were defined for Win32 but not non-Win32 targets. I'm patching that up now.

@joncampbell123
Copy link
Owner

@joncampbell123 joncampbell123 commented Aug 16, 2015

I'm sorry. The LFN patch seems to completely break DOSBox-X's ability to lookup or access files from anything other than drive Z:
I'm also concerned about the Windows-centric nature of the patch. It adds some new functions but only for Win32 targets, thus causing a compile failure on non-Windows targets.
It will need some work to integrate into DOSBox-X.
Would you be willing to git clone from my master branch and work out a better patch?

I am impressed that when applied it was able to directory list the various shell scripts in the same directory on Linux and emulate the ~1 names that were so common in the Windows 95 days.

I like the idea of adding a ver= parameter to DOSBox-X to set the DOS version, so I'll go ahead and integrate that part of the patch, with my addition to allow specifying the minor DOS version as well (i.e. ver=6.22 to emulate MS-DOS 6.22).

@ghost
Copy link
Author

@ghost ghost commented Aug 16, 2015

I would be glad to try to adapt it for other OS, although I'm typically in a win32 environment. However, I can try to adapt it for Linux -- it may not require those extra functions which are windows specific anyways. I appreciate all your work on this!

@joncampbell123
Copy link
Owner

@joncampbell123 joncampbell123 commented Aug 16, 2015

I just finished the ver= option. Please make sure the patch submitted does not try to add that to the source. Thanks for the help!

@ghost
Copy link
Author

@ghost ghost commented Aug 16, 2015

I'll make sure of that, and thanks again. :)

@ghost
Copy link
Author

@ghost ghost commented Aug 17, 2015

Made minor changes so it is compatible with your latest commit on dos version#: http://pastebin.com/xpjPSnLP. With the patch, and building in mingw32, DOSBox-X seems to have functional LFN (such as the "copy" command).

I wonder if the issues are solely related to non-win32 clients instead of any particular incompatibility with DOSBox-X. If so, I could eventually setup a linux client for testing, but do you think it is reasonable to submit a win32 only patch and define the lines as such throughout the code?

@joncampbell123
Copy link
Owner

@joncampbell123 joncampbell123 commented Aug 17, 2015

It applies better, but again when compiled on Linux, DOSBox-X can't open files other than drive Z:

Also yet again, two functions are defined only for WIN32 target and not for Linux causing a compile failure:

bool read_directory_first2(dir_information* dirp, char* entry_name, bool& is_directory);
bool read_directory_next2(dir_information* dirp, char* entry_name, bool& is_directory);

Something about this patch completely breaks path lookup. If I mount a directory as drive C: it can see the files but any attempt to open files fails.

I suggest setting up a VirtualBox or QEMU VM with Linux, installing a C++ compiler and the SDL 1.2 libraries, and trying to compile within the VM to see what I mean. Make sure you install GCC 4.8, automake, autoconf, m4, the C++ compiler, libSDL 1.2. Remember that most distributions make two packages for a library, often one with just the binary, and the other with headers and libraries needed for development.

The parts of the patch that DO work are impressive though!

@ghost
Copy link
Author

@ghost ghost commented Aug 17, 2015

I appreciate the walkthrough! I added those functions and unfortunately never tested on non-win32, so I mistakenly placed within a definition block. That should be simple to fix once a VM is setup. Also, your analysis of the "path lookup" should help find the problem in the code, especially since it's on non-Z drives. I would guess the issue is related to the way the path is stored in some variable, such as using "" instead of "/". It may take some time, but I'm glad to have the help.

@i30817
Copy link

@i30817 i30817 commented Aug 24, 2015

I'm interested in applying this patch to my ppa if it can be made 'linux compatible'. Damn dosbox fragmentation anyway.

I'd also love the ability to boot windows 95 from a local mounted drive, not a hdd image and to allow it to see local mounted images (Cds and drives), but that is a different matter.

@ghost
Copy link
Author

@ghost ghost commented Aug 30, 2015

I just got the lfn patch to work in dosbox-x in linux. I have to remake the patch to remove the next2, first2 functions which may take some time, but the previously attached patch should work without any debugging (however, it requires modifying the next2, first2 functions which were required by mingw32).

@ghost
Copy link
Author

@ghost ghost commented Aug 30, 2015

Please try this one:
http://pastebin.com/953mi3eG

It applies against yesterday's dosbox-x source code. It's verified in ubuntu linux. However, I have a lot of other changes once this is confirmed to work.

Also, please verify that "ver" is set at command line in dosbox-x. If not, then set ver=7.0 in dosbox.conf file.

@joncampbell123
Copy link
Owner

@joncampbell123 joncampbell123 commented Aug 30, 2015

"This paste has been removed"

That was fast. Please let me know when it is available again.

@ghost
Copy link
Author

@ghost ghost commented Aug 30, 2015

Sorry! Here it is: http://pastebin.com/GvQxpKmt.

@ghost
Copy link
Author

@ghost ghost commented Aug 30, 2015

I verified the above lfn code in dosbox-x and ubuntu linux (gcc 4.8.1). I also have the new code that you uploaded today in a parsable format. I didn't include the updated portions because I haven't tested them fully, but they work in a custom build under Windows.

I also made several changes not yet uploaded, such as fully moving the lfn detection routine from xms.cpp to dos.cpp, consistent with your "dos version" patch. I also removed the *_next2 and *_prev2 and commented out a line of code which can be modified at a later time. I was trying to preserve a new function in dosbox-x which involves images or screenshots.

If you'd like the updated portions sooner, then I can send this patch which applies to a custom build, but at least it will be simple to update dosbox-x. Otherwise, if it helps, then I can verify that the updated code applies cleanly in dosbox-x and then upload.

@joncampbell123
Copy link
Owner

@joncampbell123 joncampbell123 commented Aug 30, 2015

Hm... I'm still running into the issue that I can directory list, but not run or open files. I noticed "del" and "ren" works though. Drive Z: works.

I am mounting a folder to drive c: in my dosbox.conf

mount c: .

I also tried making the path absolute:

mount c: /mnt/main/emu/test

However I see what the problem is. I did an strace on the running DOSBox instance and I noticed that the problem seems to stem from the fact that I am typing the DOS program name in lowercase (nssi.exe), but the DOS program was extracted to the folder on Linux from a ZIP file that named it in uppercase (NSSI.EXE). The DOSBox code prior to your patch handled the case difference just fine, the new code does not.

So on my system, because the DOS executable is named NSSI.EXE and not nssi.exe, I cannot just type "nssi" and run the program, I have to type "NSSI".

On Linux, file operations are case sensitive. That means the system treats "HELLO" and "Hello" and "hello" as different file names. Windows treats file names as case insensitive, which is probably why this patch did not cause problems there. When you tested on Linux, all DOS names were lowercase, right? All the LFN emulation needs to do is restore the case insensitive file name matching (even on Linux) and the problem should be resolved.

Does that help with the issue?

Again it's interesting that the builtin DEL and REN commands are not affected by this, but opening files and running executables is.

@ghost
Copy link
Author

@ghost ghost commented Aug 30, 2015

I did limited testing since I was trying to upload a result as soon as I could. I thought I could fix the untested issues later so that you would have lfn code that applies to dosbox-x (instead of trying to merge new code bits with unknown locations in dosbox-x).

In my testing, I used the copy command and made sure I could then view the new file with "more". I mounted the drive like this only:
mount c ./

I did not test the case sensitivity of names inside dosbox-x. I may have used name completion only so that problem went unnoticed.

I'll try to fix the remaining issues today. I'll run tests to detect all issues noted above, too. :)

@ghost
Copy link
Author

@ghost ghost commented Aug 31, 2015

Updated the patch here: http://pastebin.com/dPecXsmp.

It has the latest patch from the forum along with the two changes discussed there to introduce upper case filenames. Also, added many fixes and a few workarounds to compile in linux. This patch applies to the dosbox-x source code and verified that the lfn is working.

Note that workarounds include removing code for detecting Windows (0x1605) so the function matches the original lfn code; temporarily assigning variables for a couple of auxiliary functions (mentioned earlier in this thread); and possible one or two others. Please also verify the CMD_COPY function against the patches from the forum.

This patch will make it simpler to test the lfn in dosbox-x and then verify against the forum's patches and then continue patching from that step.

@i30817
Copy link

@i30817 i30817 commented Aug 31, 2015

Does it apply on dosbox upstream svn too?

@i30817
Copy link

@i30817 i30817 commented Sep 11, 2015

Also could you please use something persistent to store the patches, like attaching them here (if possible) or in google drive? All your pastebin links are dead.

@Rondom
Copy link

@Rondom Rondom commented Sep 9, 2017

Has anyone saved a copy of that patch? @joncampbell123?

The submitter deleted his GitHub-account. If anyone remembers who the submitter was (maybe from GitHub email notifications stored in one's inbox), maybe one can also contact them.

@i30817
Copy link

@i30817 i30817 commented Sep 10, 2017

Thanks, even though i no longer need it (my problem was different).

@sergeevabc
Copy link

@sergeevabc sergeevabc commented Nov 24, 2018

Err… So, does DOSBox-X have long filenames support or not?

@joncampbell123
Copy link
Owner

@joncampbell123 joncampbell123 commented Nov 24, 2018

No, sorry. Forgot this issue even existed, let me grab the patch.

@joncampbell123
Copy link
Owner

@joncampbell123 joncampbell123 commented Nov 24, 2018

It's going to need some work to incorporate, since the patch is probably based on some really old DOSBox code.

@Wengier
Copy link
Collaborator

@Wengier Wengier commented Feb 17, 2020

I have managed to adapt the patch for the latest DOSBox-X release (v0.82.26). Some newer changes from the DOSBox LFN patch are also incorporated here. Tested under Windows 10 (Visual Studio 2015) and Linux (Fedora 30) platforms. The LFN to seems to be functional (LFN is enabled when DOS version >=7.0). Also tested NSSI.EXE and Windows 3.1 (as well as WFW 3.11) inside DOSBox-X, both appear to work fine. No problems encountered when I tried them.

The source code diff is available from the link:
http://individual.utoronto.ca/wengier/files/dosbox-x-v0.82.26-lfn-patch.diff

And the pre-compiled binary for Windows (64-bit) is also available from the link:
http://individual.utoronto.ca/wengier/files/dosbox-x-lfn.zip

@ghost
Copy link

@ghost ghost commented May 30, 2020

I'm not able to apply that diff using patch or git - can you please send it as a pull request @Wengier?

@Wengier
Copy link
Collaborator

@Wengier Wengier commented May 30, 2020

@voltagex LFN support is already included in DOSBox-X 0.83.1 or later. Set ver=7.1 in dosbox-x.conf to enable it at start. Also, in DOSBox-X 0.83.2 (which will come soon) you can enable it directly with LFN=true in dosbox-x.conf.

@ghost
Copy link

@ghost ghost commented May 30, 2020

Oops! Thanks. Should this issue be closed now?

@Wengier Wengier closed this Sep 2, 2020
@Wengier Wengier self-assigned this Sep 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants