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

Optlink rejects paths with invalid characters based on HPFS filesystem instead of NTFS #43

Closed
wilzbach opened this issue Sep 6, 2010 · 14 comments

Comments

@wilzbach
Copy link
Owner

wilzbach commented Sep 6, 2010

Note: the issue was created automatically migrated from https://issues.dlang.org

Original bug ID: BZ#4831
From: Xavier Bigand <flamaros.xavier@gmail.com>
Reported version: D2
CC: andrej.mitrovich@gmail.com, ascend4nt@gmail.com, johnnymarler@gmail.com, lio+bugzilla@lunesu.com, mailnew4ster@gmail.com, spam@extrawurst.org

@wilzbach
Copy link
Owner Author

wilzbach commented Sep 6, 2010

Comment author: Xavier Bigand <flamaros.xavier@gmail.com>

Here is the error :

OPTLINK : Error 118: Filename Expected
Path=D:\Softs\D\dmd2\windows\bin;C:\Program Files\Microsoft SDKs\Windows\v6.0A\bin;C:\Program Files\NVIDIA Corporation\PhysX\Common;D:\Softs\VTune\CGGlbCache;D:\Softs\VTune\Analyzer\Bin;D:\Softs\VTune\Shared\Bin;D:\Softs\Python26\Lib\site-packages\PyQt4\bin;D:\Softs\Perl\site\bin;D:\Softs\Perl\bin;C:\WINDOWS\system32;C:\WINDOWS;C:\WINDOWS\System32\Wbem;C:\Program Files\Fichiers communs\Roxio Shared\DLLShared;C:\Program Files\Fichiers communs\Roxio Shared\9.0\DLLShared;D:\Softs\SDKs\Wii\NDEV\bin;D:\Softs\Microsoft Visual Studio 8\VC\bin;D:\Softs\doxygen\bin;D:\Softs\Graphviz2.24\bin;c:\Program Files\Microsoft SQL Server\90\Tools\binn;C:\WINDOWS\system32\WindowsPowerShell\v1.0;D:\Softs\TortoiseGit\bin;D:\Softs\QuickTime\QTSystem;D:\Softs\TortoiseSVN\bin;D:\Softs\gDEBugger\gDEBugger;D:\Softs\D\dmd\windows\bin;D:\Softs\D\dmd2\windows\bin;D:\Softs\D\dm\bin;D:\Projects\Tetraedge\Trunk\TeEngine_3\Tools\Libs\Qt;D:\Softs\FileVerifier++;D:\Softs\SDKs\Android\android-sdk-windows\tools;D:\Softs\apache-ant\bin

It seems D:\Softs\FileVerifier++\ with '+' character reveal the issue.

Here it's my original discussion :
http://www.dsource.org/forums/viewtopic.php?t=5585

@wilzbach
Copy link
Owner Author

Comment author: Lionello Lunesu <lio+bugzilla@lunesu.com>

This bug is reported since 2004 and I'm still hitting it. Took me too long to find out what's wrong. OPTLINK fails when linking debug info and PATH has "++". I wonder how the first part relates to the second.

Assigning to Walter.

@wilzbach
Copy link
Owner Author

Comment author: Walter Bright <bugzilla@digitalmars.com>

As a workaround for the time being, try enclosing the path in "".

@wilzbach
Copy link
Owner Author

Comment author: Andrej Mitrovic <andrej.mitrovich@gmail.com>

*** Issue 8791 has been marked as a duplicate of this issue. ***

@wilzbach
Copy link
Owner Author

Comment author: Andrej Mitrovic <andrej.mitrovich@gmail.com>

*** Issue 5860 has been marked as a duplicate of this issue. ***

@wilzbach
Copy link
Owner Author

wilzbach commented Feb 9, 2013

Comment author: Andrej Mitrovic <andrej.mitrovich@gmail.com>

Walter Bright

As a workaround for the time being, try enclosing the path in "".

I think we've established it doesn't quite work. However I think the lines in this .asm file could be changed to fix this:

https://github.com/DigitalMars/optlink/blob/master/parse/parse_fn.asm#L206

It refers to "illegal HPFS filename path", a google search reveals this is an ancient OS/2 file system: http://en.wikipedia.org/wiki/High_Performance_File_System

We should update this so only NTFS illegal chars are rejected (if you want to keep supporting OS/2 then we should at least branch out for D). These are the illegal ones:

/ ? < > \ : * | ”

Btw, I still can't build Optlink so I can't make a pull request for this.

@wilzbach
Copy link
Owner Author

wilzbach commented Feb 9, 2013

Comment author: Andrej Mitrovic <andrej.mitrovich@gmail.com>

*** Issue 7800 has been marked as a duplicate of this issue. ***

@wilzbach
Copy link
Owner Author

Comment author: Andrej Mitrovic <andrej.mitrovich@gmail.com>

DigitalMars/optlink#1

@wilzbach
Copy link
Owner Author

Comment author: Andrej Mitrovic <andrej.mitrovich@gmail.com>

DigitalMars/optlink#1

Now that thanks to Reiner I can build Optlink, I can see this pull was wrong. Optlink uses '+' on the commandline as an argument separator, but it should not reject such a path when it's e.g. enclosed in quotes. So both of these should work:

link kernel32.lib+phobos.lib

link kernel32.lib+phobos.lib+"some/path++/to"

The same holds true for "," and ";", and maybe some other special characters.

@wilzbach
Copy link
Owner Author

Comment author: Jonathan Marler <johnnymarler@gmail.com>

Created potential workaround. Pull request here:

DigitalMars/optlink#16

@wilzbach
Copy link
Owner Author

Comment author: Walter Bright <bugzilla@digitalmars.com>

DigitalMars/optlink@7c09639

@wilzbach
Copy link
Owner Author

Comment author: 9999 <mailnew4ster@gmail.com>

Seems like the merged pull is only a workaround. Why not pull the real fix instead?
DigitalMars/optlink#1

@wilzbach
Copy link
Owner Author

Comment author: Andrej Mitrovic <andrej.mitrovich@gmail.com>

(In reply to 9999 from comment BZ#11)

Seems like the merged pull is only a workaround. Why not pull the real fix
instead?
DigitalMars/optlink#1

I don't think my fix was fully complete.

@wilzbach
Copy link
Owner Author

Comment author: Jonathan Marler <johnnymarler@gmail.com>

Andrei I believe he's referring to my PR. The issue described by the description of this bug was fixed (having '+' signs in the path), however, based on the title I'm not sure it fixed everything. If there are any other characters in the path that would cause optlink to fail I can take a look. Do you know of any more cases to test?

Also, it is a workaround because optlink now ignores any paths containing a '+' character. I believe that making optlink accept paths with '+' character may be a much bigger change. I didn't want my first contribution to optlink to be too big so I opted for the workaround (which was around 15 lines of code). Maybe I'll spend some time on this problem later. If you have any more dire issues post them here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant