Summary: | how r/w -> rb/wb breaks regression? | ||
---|---|---|---|
Product: | Ghostscript | Reporter: | Hin-Tak Leung <htl10> |
Component: | Graphics Library | Assignee: | Chris Liddell (chrisl) <chris.liddell> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | alex, htl10, sags5495 |
Priority: | P4 | ||
Version: | master | ||
Hardware: | PC | ||
OS: | Linux | ||
Customer: | Word Size: | --- | |
Bug Depends on: | |||
Bug Blocks: | 691520 | ||
Attachments: | configure.ac and a couple of other files patch |
Description
Hin-Tak Leung
2010-07-06 03:19:50 UTC
popen() doesn't like "wb" on Linux but accepts it on Windows. I wonder what's a good way to fix this? Perhaps, this should be yet another issue for autoconf. (In reply to comment #1) > popen() doesn't like "wb" on Linux but accepts it on Windows. > > I wonder what's a good way to fix this? Perhaps, this should be yet another > issue for autoconf. Don't know what's the best way, but any solution probably involves having some conditionals of the form #if TEXT_MODE_DIFF_BIN_MODE ... #else ... #endif somewhere, since the 'b' is required on some platforms but problematic on others. Created attachment 6519 [details]
configure.ac and a couple of other files patch
Add a test for the configure script to discover the current platform support the "b" option for popen(), and use a preprocessor directive to apply the result when required.
Posting here for Hin-Tak and/or Alex to cast eye over the proposed changes, any problems/objections, please shout, if I hear nothing, I'll assume we're all happy.
The only thing missing is a corresponding change for MSVC and other non-autoconf platforms. A typo: #define GP_FMODE_WB "b" should be #define GP_FMODE_WB "w" Oops, thanks for the correct SaGS! Alex, Can't we just rely on the default fallback I put in gp.h for platforms that a) don't use configure, and/or b) don't care about the "r"/"rb" difference? I ask because there are several (OS/2, vms etc) that I would have no way of testing changes to the make files. (In reply to comment #6) > Alex, Can't we just rely on the default fallback I put in gp.h for platforms > that a) don't use configure, and/or b) don't care about the "r"/"rb" > difference? mingw doesn't use configure, but still care r/rb . So probably something like if defined(__CYGWIN__) || defined(__MINGW32__) is still needed. (In reply to comment #7) > (In reply to comment #6) > > mingw doesn't use configure, but still care r/rb . So probably something like > if defined(__CYGWIN__) || defined(__MINGW32__) > is still needed. Is there a (good) reason for that? Other packages for it seem to rely on a configure script, so it seems reasonable for us to do so, and avoid adding platform specific preprocessor macros in the source. (In reply to comment #8) > (In reply to comment #7) > > (In reply to comment #6) > > > > mingw doesn't use configure, but still care r/rb . So probably something like > > if defined(__CYGWIN__) || defined(__MINGW32__) > > is still needed. > > Is there a (good) reason for that? Other packages for it seem to rely on a > configure script, so it seems reasonable for us to do so, and avoid adding > platform specific preprocessor macros in the source. There are platform-specific macros in various places - mostly in */*_.h's (e.g. stdio_.h, unistd_.h) so at least your new changes are compatible in the general direction and better than put them in *.c's. configure is a shell thing, which is somewhat extra to the mingw ethos. it is probably not important either way. I committed a change a while back which sorts this out. |