[ovs-dev] [PATCH] ovsdb-client: Set binary on FDs when doing backup/restore

[Ben Pfaff]

On Thu, Mar 08, 2018 at 08:09:07PM +0200, Alin Gabriel Serdean wrote:
> Add some needed consitentcy on Windows for STD_IN/OUT file descriptors
> when doing backup and restore.
> 
> Reported-at:https://mail.openvswitch.org/pipermail/ovs-dev/2018-January/343518.html
> Suggested-by: Ben Pfaff <blp at ovn.org>
> Signed-off-by: Alin Gabriel Serdean <aserdean at ovn.org>

Thanks!  I like this a lot better than the previous approach.

I am not sure that I like the idea of combining checking for a tty with
setting the mode.  I see that we are doing both together, but they seem
only loosely related.  I guess that I would be happier with a function
that does only the mode-set and left the isatty call to the caller.

I believe that there is a bug in check_and_set_stdout(): it always works
on stdout, ignoring the fd passed in.  That matches the "stdout" in its
name, but I think it's unintentional.  I guess that it should take a
FILE *, fflush that stream, and then _setmode() on fileno(stream) (or
_fileno(stream))?  Something like this:

static void
set_binary_mode(FILE *stream OVS_UNUSED)
{
#ifdef _WIN32
    fflush(stream);
    if (_setmode(_fileno(stream), O_BINARY) == -1) {
        ovs_fatal(errno, "could not set binary mode on fd %d",
                  _fileno(stream));
    }
#endif
}

Thanks,

Ben.