Bug 696101 - gsicc_manage opens file with spurious second 'b' for binary mode
Summary: gsicc_manage opens file with spurious second 'b' for binary mode
Status: NOTIFIED FIXED
Alias: None
Product: Ghostscript
Classification: Unclassified
Component: Graphics Library (show other bugs)
Version: master
Hardware: PC Windows 7
: P2 normal
Assignee: Chris Liddell (chrisl)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-07-20 01:43 UTC by norbert.janssen
Modified: 2015-08-08 16:29 UTC (History)
1 user (show)

See Also:
Customer: 661
Word Size: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
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