Issue 4831 - Optlink rejects paths with invalid characters based on HPFS filesystem instead of NTFS
Summary: Optlink rejects paths with invalid characters based on HPFS filesystem instea...
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: tools (show other issues)
Version: D2
Hardware: All Windows
: P2 blocker
Assignee: Walter Bright
URL:
Keywords: Optlink
: 5860 7800 8791 (view as issue list)
Depends on:
Blocks:
 
Reported: 2010-09-06 13:03 UTC by Xavier Bigand
Modified: 2017-01-10 01:41 UTC (History)
6 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Xavier Bigand 2010-09-06 13:03:39 UTC
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
Comment 1 Lionello Lunesu 2011-11-19 23:11:38 UTC
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.
Comment 2 Walter Bright 2011-12-14 02:03:34 UTC
As a workaround for the time being, try enclosing the path in "".
Comment 3 Andrej Mitrovic 2012-10-14 18:01:26 UTC
*** Issue 8791 has been marked as a duplicate of this issue. ***
Comment 4 Andrej Mitrovic 2012-10-14 18:01:40 UTC
*** Issue 5860 has been marked as a duplicate of this issue. ***
Comment 5 Andrej Mitrovic 2013-02-09 20:09:31 UTC
Walter Bright(In reply to comment #2)
> 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.
Comment 6 Andrej Mitrovic 2013-02-09 20:13:35 UTC
*** Issue 7800 has been marked as a duplicate of this issue. ***
Comment 7 Andrej Mitrovic 2013-02-12 18:08:57 UTC
https://github.com/DigitalMars/optlink/pull/1
Comment 8 Andrej Mitrovic 2013-02-16 16:42:09 UTC
(In reply to comment #7)
> https://github.com/DigitalMars/optlink/pull/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.
Comment 9 Jonathan Marler 2014-07-30 09:33:51 UTC
Created potential workaround.  Pull request here:

https://github.com/DigitalMars/optlink/pull/16
Comment 11 9999 2015-01-30 15:12:33 UTC
Seems like the merged pull is only a workaround. Why not pull the real fix instead?
https://github.com/DigitalMars/optlink/pull/1
Comment 12 Andrej Mitrovic 2015-01-30 15:16:07 UTC
(In reply to 9999 from comment #11)
> Seems like the merged pull is only a workaround. Why not pull the real fix
> instead?
> https://github.com/DigitalMars/optlink/pull/1

I don't think my fix was fully complete.
Comment 13 Jonathan Marler 2015-01-30 16:26:55 UTC
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.