Re: [PATCH 4/4] m4_esyscmd: fdopen() with a text mode explicitly on OS/2

[KO Myung-Hun]

Eric Blake wrote:
> On 11/18/2014 08:54 PM, KO Myung-Hun wrote:
>> On OS/2 kLIBC, fdopen() creates a stream in a mode of a file 
>> descriptor. So specify "t" to open a stream in a text mode
>> explicitly on OS/2.
>> 
>> * src/builtin.c (m4_esyscmd): fdopen() in a text mode on OS/2. 
>> --- src/builtin.c | 7 ++++++- 1 file changed, 6 insertions(+), 1
>> deletion(-)
>> 
>> diff --git a/src/builtin.c b/src/builtin.c index e101838..7a73b36
>> 100644 --- a/src/builtin.c +++ b/src/builtin.c @@ -1019,7
>> +1019,12 @@ m4_esyscmd (struct obstack *obs, int argc, token_data
>> **argv) sysval = 127; return; } -  pin = fdopen (fd, "r"); +#if
>> OS2 +# define MODE_TEXT "t" +#else +# define MODE_TEXT "" 
>> +#endif
> 
> Eww.  Mid-function #ifdefs are evil.  I strongly prefer that we
> hoist it out of the function.

Kindly, Gary removed the evil. ^^

> Also, fdopen("rt") looks awkward, since 't' mode is non-standard.
> 
I also know, so I guarded it with OS2. On OS2, it is standard.

> Why do you need text mode?  What is the default if you omit 't',
> and why is binary mode not good enough?  I'm very reluctant to see
> this patch applied as-is without more reasoning and/or more 
> refactoring for easier maintenance.
> 

As you know, text mode means that CR/LF and LF are translated to each
other when reading and writing. That is, CR/LF to LF on reading, LF to
CR/LF on writing.

So if a stream is opened in binary mode, CR/LF is read as-is. But when
writing it to stdout, which is text mode, it is translated to CR/CR/LF
because LF is translated to CR/LF in text mode.

However, in text mode, CR/LF is translated to LF when reading. And
when writing to stdout, LF is translated to CR/LF as expected.

And for a default mode of fdopen(), as I said in a commit message,
fdopen() creates a stream in a mode of a file descriptor unless a
translation mode character such as "t" and "b". To assure text mode
regardless of a mode of a file descriptor, it is needed to set a
translation mode explicitly.

-- 
KO Myung-Hun

Using Mozilla SeaMonkey 2.7.2
Under OS/2 Warp 4 for Korean with FixPak #15
In VirtualBox v4.1.32 on Intel Core i7-3615QM 2.30GHz with 8GB RAM

Korean OS/2 User Community : http://www.ecomstation.co.kr