Bug 696101

Summary: gsicc_manage opens file with spurious second 'b' for binary mode
Product: Ghostscript Reporter: norbert.janssen
Component: Graphics LibraryAssignee: Chris Liddell (chrisl) <chris.liddell>
Status: NOTIFIED FIXED    
Severity: normal CC: chris.liddell
Priority: P2    
Version: master   
Hardware: PC   
OS: Windows 7   
Customer: 661 Word Size: ---

Description norbert.janssen 2015-07-20 01:43:31 UTC
We are in the process of migrating to VS2015/64bit and encountered the following problem.
file_prepare_stream is adding a 'b' to the fileaccess-mode (for opening a file in binary mode). However, if the access_mode already contains a "rb", this will end-up in a "rbb". And this causes a crash when running on a Win10 (yes!) 64bit system.
The compiler seems to not complain (why should it), but the crtlib for Win10 crashes.

Propose to not  use the 'b' when opening files/streams in generic code, but let the file_prepare_stream do the strcat(mode, gp_fmode_binary_suffix)
I.e. sfopen calls (on our system) romfs_open_file(), that calls file_prepare_stream() that adds the 'b'.
Comment 1 norbert.janssen 2015-07-20 01:51:28 UTC
I saw that there are lots of places where an "rb" is passed as access_mode.
Perhaps it is better to check for a 'b' in the mode before adding the gp_fmode_binary_suffix.
This would be in

zfile.c::ztempfile()
sfxcommon.c::file_prepare_stream()
Comment 2 norbert.janssen 2015-07-20 02:32:28 UTC
ztempfile() is also protected by parse_file_access_string() which will only pass a 'a', 'r',  'w' or "a+", "r+", "w+".
So this only leaves sfxcommon.c::file_prepare_stream()

    /* Open the file, always in binary mode. */
    strcpy(fmode, file_access);
    if (strrchr(file_access, 'b') == null)
    {
        strcat(fmode, gp_fmode_binary_suffix);
    }
Comment 3 Chris Liddell (chrisl) 2015-07-30 10:33:28 UTC
My preference would be to fix the users of the API in question - since the API always opens files in binary mode (where applicable), it seems best, to me, to ensure that no callers add the 'b', and put protection in place to ensure we catch any new code which does so - add a warning and an assert to catch any callers adding the 'b'.
Comment 4 Chris Liddell (chrisl) 2015-08-06 02:31:21 UTC
Fixed in:

http://git.ghostscript.com/?p=ghostpdl.git;a=commitdiff;h=1fae53a7