Fix for Issue 7648. #3138

aka-demik · 2015-04-02T15:37:18Z

This fixes Issue 7648 - std.stdio expects file names to be encoded in CP_ACP on Windows instead of UTF-8.

schveiguy · 2015-04-02T18:11:26Z

Can you add a unittest for this situation?

mgwasus · 2015-04-03T15:55:31Z

I loaded master.phobos and compiled phobos.lib. I try to compile with new phobos.lib. In case of execution I receive an error. Windows 7 (32), cp1251. This error is and in 65 and 66 and 67 versions dmd.
Source text in Utf-8.



import std.stdio;

void main() {

File(«Ёлки Объект №876».txt, "w").writeln("Привет из D");

}

I correct this error the following patch in stdio.d



private FILE* fopen(in char[] name, in char[] mode = "r") @trusted // nothrow @nogc -> mgw disable for fromUtf8toAnsiW

{

import std.internal.cstring : tempCString;
version(Windows)
{
    // 02.04.2015 8:31:19 Адаптированно для dmd 2.067.0 
    wchar* fromUtf8toAnsiW(in char[] s, uint codePage = 0) @trusted  {
        import std.c.windows.windows: WideCharToMultiByte, GetLastError;
        import std.windows.syserror: sysErrorString;
        import std.conv: to;
        char[] result, rez; int readLen; auto ws = std.utf.toUTF16z(s);
        result.length = WideCharToMultiByte(codePage, 0, ws, -1, null, 0, null, null);
        if (result.length)  {
            readLen = WideCharToMultiByte(codePage, 0, ws, -1, result.ptr,  to!int(result.length), null, null);
            for(int i; i != result.length; i++) { rez ~= result[i]; rez ~= 0; } rez ~= 0; rez ~= 0;
        }
        if (!readLen || readLen != result.length)   {
            throw new Exception("Couldn't convert string: " ~ sysErrorString(GetLastError()));
        }
        return cast(wchar*)rez.ptr;
    }

    import std.internal.cstring : tempCStringW;
    // return _wfopen(name.tempCStringW(), mode.tempCStringW()); // mgw disable for  fromUtf8toAnsiW
    return _wfopen(fromUtf8toAnsiW(name), mode.tempCStringW());
}
else version(Posix)
{
    import core.sys.posix.stdio : fopen;
    return fopen(name.tempCString(), mode.tempCString());
}
else
{
    return .fopen(name.tempCString(), mode.tempCString());
}

}

aka-demik · 2015-04-03T17:15:46Z

Added a simple unit test.

schveiguy · 2015-04-03T17:31:07Z

@mgwasus so you are saying that this patch does NOT fix your issue?

aka-demik · 2015-04-03T17:32:30Z

to mgwasus: WideCharToMultiByte convert to local code page. So this may fail on windows with cp1251:

File("Sehenswürdigkeit.txt", "r").writeln("Hello from Germany.");

In any case, I think that your patch for "fopen" makes sense. Because now we can not use "fopen" on windows, if the file name contains non-ASCII characters.
But it does not solve Issue 7648.

aka-demik · 2015-04-03T17:34:21Z

I think mgwasus offered an alternative solution.

schveiguy · 2015-04-03T17:35:29Z

I'm going to defer to others on this regarding whether this fixes the issue, as language support is not my forte, but the implementation seems sound to me.

aka-demik · 2015-04-03T17:57:54Z

This PR is the implementation of @CyberShadow -s suggestion, from 7648 thread:

Vladimir Panteleev 2014-02-14 11:32:05 UTC
How about using CreateFile + windowsHandleOpen?

Maybe he could check?

aka-demik · 2015-04-03T18:29:40Z

Woops. Based on unittest in std\stdio.d:1219 fopen opens the file in FILE_SHARE_READ + FILE_SHARE_WRITE. I will fix it.

CyberShadow · 2015-04-03T20:44:47Z

Hi,

mgwasus' solution only applies to the DigitalMars C runtime, and it only works if the filename contains characters in the system codepage. It is an improvement but a minor one.
Using CreateFile/windowsHandleOpen allows us to bypass the C runtime's crappy filename parsing at the expense of duplicating some of its functionality (parsing mode flags, at least). Some regressions might appear (esp. with C runtime interoperability) if we don't match it's functionality exactly.
Another approach would be to ask for the source code to the DM C runtime from Walter Bright. He has given it out in the past to people who have shown intent to fix things in them. If DMC's _wfopen can be fixed to properly work with wide characters (instead of casting each char to the system CP like it does now), the Phobos-side patch would be much simpler.

About this patch:

trustedHandleToFD seems to be a copy-paste of windowsHandleOpen. Please refactor things instead of copy-pasting them.
I have a third-party implementation of the rest of mode parsing here. Might be worth comparing the implementations, discrepancies might indicate bugs in one or another.

aka-demik · 2015-04-03T21:30:50Z

I'll be ready for the second round on Monday.
Fixing DMC's _wfopen seems to me the right decision. But I can not estimate how difficult it is.

rainers · 2015-04-04T08:22:06Z

The functions that are explicitely mapped from Unicode back to the bad multibyte versions by the DMC runtime are CreateFile, DeleteFile, MoveFile, GetFileAttributes, SetFileAttributes, CreateDirectory, RemoveDirectory, FindFirstFile, FindNextFile, GetCurrentDirectory, SetCurrentDirectory, CreateProcess, GetCommandLine and GetModuleFileName. So there are quite a few other C runtime functions other than fopen that fail to work with non-ascii characters.
If dmc ditches Win95 support, it should be rather simple for @WalterBright to map the unicode functions to the correct API.

CyberShadow · 2015-04-04T08:24:33Z

I think DMC still supports MS-DOS, but as D supports XP and up, maybe @WalterBright would be OK with a DM libc branch/fork for D.

rainers · 2015-04-04T08:36:02Z

The DOS versions would not be affected, but some early Windows version might. For full compatibility a runtime check for availability of the W-Versions could be added, too.

CyberShadow · 2015-04-04T08:38:21Z

If you have time to handle this, I think you should email Walter Bright and get the code.

rainers · 2015-04-04T08:52:00Z

Actually I've got it a few years ago as part of the commercial compiler version, but never tried to build it. I'll have a look...

MGWL · 2015-04-04T13:45:24Z

I have loaded function _wfopen () from msvcrt.dll using functions LoadLibrary and GetProcAddress. This loaded dynamically function fine works with wide lines and creates files with any names. Thus, precisely mistake in _wfopen from dmc.

It is strange, why such serious mistake is not corrected till now.

CyberShadow · 2015-04-04T13:48:32Z

Yeah, by now it's not a secret that DM libc's "wide" functions simply convert the wide strings down to ANSI (and not even doing any sort of Unicode/codepage conversion, but simply by casting each wchar_t to a char).

rainers · 2015-04-04T15:25:40Z

The dmc lib does a WideCharToMultiByte conversion, but that only works for characters in the current code page.
I've emailed a fix to Walter, let's see whether it is accepted.

…indows GUI application

d-random-contributor · 2015-04-05T11:21:39Z

Unicode winapi is supported on win9x through MSLU

Cent-ify std.traits.

fix Issue 14422 - std.process: Pipes do not append to files on Win64

…s-14408 fix Issue 14408 - std.process: Can't start interactive process from Wind...

aka-demik · 2015-04-10T12:49:15Z

As I understand it, we waiting for a reply Walter Bright.
In case if we can not fix DMC, I corrected PR taking into account the wishes of CyberShadow.
Rebased. Just not sure that is correct.

aka-demik · 2015-04-22T10:55:28Z

Ping @rainers
Have passed two weeks. Is there no news from Walter Bright?

rainers · 2015-04-22T20:58:46Z

Sorry, I didn't hear back from @WalterBright regarding my second version. I don't know if it is ignored or accepted.

WalterBright · 2015-05-15T13:45:16Z

Sorry, I'd overlooked it. Will check it out.

WalterBright · 2015-05-16T01:01:28Z

dmd does not support Windows versions prior to XP (and not sure if we officially dumped XP or not). Hence, there is no need to deal with MSLU. There never was anyway - prior versions of Phobos handled Win95 correctly, but this support was removed.

I.e. just go ahead and call the Windows 'W' functions.

dmc does need to support Win95, but it'd be better to just work around it in Phobos.

CyberShadow · 2015-05-16T02:59:46Z

I.e. just go ahead and call the Windows 'W' functions.

I'm not sure we can implement this completely in Phobos. We need to basically completely reimplement fopen but in a way that it constructs a FILE* compatible with other C functions. For example, how would you implement the "a+" file mode, which reads from one point in the file and writes to another?

aka-demik · 2015-05-16T07:06:42Z

"a+" implemented by FILE_APPEND_DATA | GENERIC_READ flags.
I will add unittest that would confirm this.

CyberShadow · 2015-05-16T15:42:16Z

"a+" implemented by FILE_APPEND_DATA | GENERIC_READ flags.

Nice, didn't know that. I don't see this documented on MSDN anywhere.

aka-demik · 2015-05-18T15:27:29Z

Nice, didn't know that. I don't see this documented on MSDN anywhere.

On MSDN CreateFile i found comment

Opening File for APPEND access
You can get atomic append on local files by opening a file with FILE_APPEND_DATA access and without FILE_WRITE_DATA access.
This behavior is documented in Windows Driver Kit / Device and Driver Technologies / Installable File System / Reference / IO Manager Routines / IoCreateFileSpecifyDeviceObject

If only the FILE_APPEND_DATA and SYNCHRONIZE flags are set, the caller can write only to the end of the file, and any offset information about writes to the file is ignored. However, the file will automatically be extended as necessary for this type of write operation.

In any case, on vanilla dmd(v2.067.1) and ldc(0.15.2-beta1) read+write with "a+" also does not working. My testcase Can't check result on DPaste because Result: Runtime error / Return code: 25 (File size limit exceeded).
dmd result:

Test 1 - ok
Test 2 - fail
Test 3 - fail

ldc result:

Test 1 - ok
Test 2 - ok
std.exception.ErrnoException@e:\develop\apps\ldc2-0.15.2-beta1-mingw-x86\bin/../import\std\stdio.d(2223):  (Invalid argument)
----------------
...

I do not correctly understand the mode "a+"?
If I add a unittest for mode "a+" in this PR, it will not be passed for all configurations except DIGITAL_MARS_STDIO on Windows.

aka-demik · 2015-05-18T15:36:15Z

Mmm... I checked with tutorialspoint's DMD64 v2.066. Got:

Test 1 - ok                                                                                                                                
Test 2 - ok                                                                                                                                
Test 3 - ok

Apparently, it is a problem only on windows.

rainers · 2015-05-19T06:56:12Z

Ping @rainers
Have passed two weeks. Is there no news from Walter Bright?

Walter has built a new version of snn.lib with the patches, it is already in the dmc package found here http://www.digitalmars.com/download/freecompiler.html. This should fix _wfopen, _wunlink, _wrename, _wmkdir, _wrmdir and a few more.

aka-demik · 2015-05-19T07:37:47Z

Thank you. I looked at the name of the archive dm857c.zip, rather than the content. And I did not notice the update.

aka-demik · 2015-05-19T08:59:11Z

With the new DMC everything is working fine on the master branch (and on 2.067.1 to). Thanks to the preparatory steps in #788. I close this PR. And opened a new one to perform the cleanup doc's info about Issue 7648.
Perhaps we need to rebuild the current release (exe and zip) with the new version DMC?
Thanks!

CyberShadow · 2016-04-21T11:54:57Z

Walter has built a new version of snn.lib with the patches, it is already in the dmc package found here http://www.digitalmars.com/download/freecompiler.html. This should fix _wfopen, _wunlink, _wrename, _wmkdir, _wrmdir and a few more.

Which link on that page includes the new snn.lib? dm857c.zip still has the old one.

aka-demik · 2016-04-21T12:21:11Z

Hmm...
When i download "http://ftp.digitalmars.com/Digital_Mars_C++/Patch/dm857c.zip" on 19 May 2015 snn.lib was 557.0 Kib, 2015-05-17, md5:45920f522533547f58eac2ba77883ec3
But now by this link snn.lib is 560.5 Kib, 2013-02-26T05:19, md5:9357508e541067ea34056dade4381dd8

That is, the link to the old version again. Although previously it was new.
My copy of dm857c.zip 19 May 2015

aka-demik · 2016-04-21T15:27:54Z

http://ftp.digitalmars.com/snn.lib - new version of snn.lib.

CyberShadow · 2016-04-21T15:40:10Z

Would be nice to have permanent links to specific versions... I guess I'll just nab it from recent DMD zip files.

Fix for Issue 7648.

61c7f3d

Added unittest for Issue 7648.

70b2793

aka-demik mentioned this pull request Apr 3, 2015

serveStaticFiles cannot handle non ASCII file names on windows vibe-d/vibe.d#1050

Closed

fix Issue 14408 - std.process: Can't start interactive process from W…

237a1c1

…indows GUI application

CyberShadow and others added 6 commits April 7, 2015 15:29

fix Issue 14422 - std.process: Pipes do not append to files on Win64

b28eb9f

Merge pull request #3153 from redstar/centtraits

14a7508

Cent-ify std.traits.

Merge pull request #3160 from CyberShadow/pull-20150407-122150

eecc989

fix Issue 14422 - std.process: Pipes do not append to files on Win64

Merge pull request #3148 from CyberShadow/pull-20150405-052650-proces…

cdd73be

…s-14408 fix Issue 14408 - std.process: Can't start interactive process from Wind...

Workaround issue 7648 only for DIGITAL_MARS_STDIO on Windows.

ef85f25

Fix for Issue 7648.

36a6064

aka-demik added 3 commits April 10, 2015 15:20

Added unittest for Issue 7648.

c4dc488

Workaround issue 7648 only for DIGITAL_MARS_STDIO on Windows.

5c7ce2c

.

85b144b

Fix for text files.

21c37b1

aka-demik closed this May 19, 2015

aka-demik mentioned this pull request May 19, 2015

Cleanup for Issue 7648: std.stdio expects file names to be encoded in CP_ACP on Windows instead of UTF-8 #3296

Merged

Fix for Issue 7648. #3138

Fix for Issue 7648. #3138

Conversation

aka-demik commented Apr 2, 2015

schveiguy commented Apr 2, 2015

mgwasus commented Apr 3, 2015

aka-demik commented Apr 3, 2015

schveiguy commented Apr 3, 2015

aka-demik commented Apr 3, 2015

aka-demik commented Apr 3, 2015

schveiguy commented Apr 3, 2015

aka-demik commented Apr 3, 2015

aka-demik commented Apr 3, 2015

CyberShadow commented Apr 3, 2015

aka-demik commented Apr 3, 2015

rainers commented Apr 4, 2015

CyberShadow commented Apr 4, 2015

rainers commented Apr 4, 2015

CyberShadow commented Apr 4, 2015

rainers commented Apr 4, 2015

MGWL commented Apr 4, 2015

CyberShadow commented Apr 4, 2015

rainers commented Apr 4, 2015

d-random-contributor commented Apr 5, 2015

aka-demik commented Apr 10, 2015

aka-demik commented Apr 22, 2015

rainers commented Apr 22, 2015

WalterBright commented May 15, 2015

WalterBright commented May 16, 2015

CyberShadow commented May 16, 2015

aka-demik commented May 16, 2015

CyberShadow commented May 16, 2015

aka-demik commented May 18, 2015

aka-demik commented May 18, 2015

rainers commented May 19, 2015

aka-demik commented May 19, 2015

aka-demik commented May 19, 2015 • edited Loading

CyberShadow commented Apr 21, 2016

aka-demik commented Apr 21, 2016

aka-demik commented Apr 21, 2016

CyberShadow commented Apr 21, 2016

aka-demik commented May 19, 2015 •

edited

Loading