Hardly a major issue, but on an NTFS formatted drive, try the following:
touch example:code.d
dmd example:code.d
DMD will generate a "code.exe" file, and use that as the module name. While again very trivial, this is incorrect - the filename is indeed "example:code.d", it is an NTFS stream tied to the file named "example".
The usefulness of this bug is only on autogenerated files and make scripts, which might create a file on Windows with a ":" in it, and then break in a hard to track down way. I found it only checking on another bug.
Minor patch upcoming.
-[Unknown]
Comment #1 by bugs-d — 2009-03-28T23:05:50Z
Created attachment 295
Properly handle an NTFS stream filename.
This patch will consequently spit out an appropriate error unless the file has a "module" line in it, since ":" is not a valid character in identifiers.
Comes free from correctly parsing in |FileName::name()|.
-[Unknown]
Comment #2 by braddr — 2009-03-29T00:48:36Z
Food for thought, I don't use windows much so my memory is foggy and quite possibly out of date, but consider the old-school device names 'com1:'. How, if at all, do those interact with this code?
Also, your change is essentially a much more round about way of writing the same code as was there before, no?
Before:
case '\\':
case ':':
return e + 1;
After:
case ':':
if (e == str + 1)
return e + 1;
// Intentional fall-through.
case '\\':
return e + 1;
Rearranged to duplicate the fallthrough block to better illustrate things:
case ':':
if (e == str + 1)
return e + 1;
return e + 1;
case '\\':
return e + 1;
So, case ':':
return e + 1;
case '\\':
return e + 1;
And we're right back to the original code.
So, uh.. submit the wrong patch? Did I misread something important and subtle in the patch?
Comment #3 by bugzilla — 2009-03-29T01:35:57Z
Brad makes a good point, there are aux:, lpt1:, prn:, com1:, etc. But they all share the characteristic that the ':' is the last character. So, here's my patch:
char *FileName::name(const char *str)
{
char *e;
size_t len = strlen(str);
e = (char *)str + len;
for (;;)
{
switch (*e)
{
#if linux || __APPLE__ || __FreeBSD__
case '/':
return e + 1;
#endif
#if _WIN32
case '/':
case '\\':
return e + 1;
case ':':
/* The ':' is a drive letter only if it is the second
* character or the last character,
* otherwise it is an ADS (Alternate Data Stream) separator.
* Consider ADS separators as part of the file name.
*/
if (e == str + 1 || e == str + len - 1)
return e + 1;
#endif
default:
if (e == str)
break;
e--;
continue;
}
return e;
}
}
Comment #4 by bugs-d — 2009-03-29T01:46:42Z
(In reply to comment #2)
> So, uh.. submit the wrong patch? Did I misread something important and subtle
> in the patch?
Sorry, you're right. I screwed up this patch, my bad. I had to backtrack and make a copy of the original source tree after I started, so I ended up with the wrong code (and forgot to retest after making patch.)
The way Walter ordered the code was what I had intended, heh.
(In reply to comment #3)
> Brad makes a good point, there are aux:, lpt1:, prn:, com1:, etc. But they all
> share the characteristic that the ':' is the last character. So, here's my
> patch:
Yes, and this is a much better patch than mine. I see that you return empty for "prn:" etc., and that spits out an invalid filename error, which is probably the original intent of this code's structure. I didn't realize that.
Thanks,
-[Unknown]