Re: [PATCHv2 5/4] nonblocking: reduce dependency

[Eric Blake]

On 04/17/2011 06:35 PM, Bruno Haible wrote:
>>
>> +    nonblocking: reduce dependency
>> +    * tests/test-nonblocking.c: Only test sockets when in use.
> 
> The intention was good. But this patch does not work for two reasons:
> 
> 1) The preprocessor macro GNULIB_SOCKET is nowhere defined. Therefore it is
>    equivalent to a #if 0. Your patch effectively disabled ioctl() support
>    on mingw. As a consequence, I see this test failure:
> 

> 2) We want that set_nonblocking_flag DOES use ioctlsocket() when the argument
> is a socket. But we DON'T want that linking with the 'nonblocking' module
> requires linking with the Win32 socket libraries. This is an apparent dilemma.
> 
> The solution is the same as for the close-hook. We can introduce another list
> for 'ioctl' hooks, or use a single hook for both 'close' and 'ioctl'. I find
> the latter more appropriate, because the use-case for both is the same.

Thanks for tackling that.  I tested on mingw, but apparently only on a
subset of all possible combinations (since my mingw setup is already
painfully slow for running tests on), and using the new hook does seem
more useful.

> Here's a proposed patch:
> 
> 
> From b7d23617875e1498f3b3ea8c6e6e31d6d9195bf9 Mon Sep 17 00:00:00 2001
> From: Bruno Haible <address@hidden>
> Date: Mon, 18 Apr 2011 02:34:47 +0200
> Subject: [PATCH] Generalize close-hook to fd-hook.

Looks nice.

> 
> ---
>  lib/close-hook.c         |   91 -------------------------------
>  lib/close-hook.h         |   72 ------------------------
>  lib/close.c              |    2 +-
>  lib/fd-hook.c            |  135 
> ++++++++++++++++++++++++++++++++++++++++++++++
>  lib/fd-hook.h            |   94 ++++++++++++++++++++++++++++++++

Git can represent file moves in a much more compact representation,
which is additionally useful because it shows the few lines that you had
to tweak as part of the rename.  I have:

[diff]
  renames = true

in my ~/.gitconfig, to turn on the git rename detection by default.  The
only reason that git doesn't do it by default is for historical
compatibility with older diff/patch programs that didn't understand git
rename-style patches, but that's less relevant these days.

-- 
Eric Blake   address@hidden    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature