Re: [PATCHv2 24/27] build: don't hand-roll cloexec code

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2011/7/8 Eric Blake <eblake@xxxxxxxxxx>:
> No need to repeat common code.
>
> * src/util/bridge.c (brInit): Use virSetCloseExec.
> (brSetInterfaceUp): Prefer unsigned flags.
> * src/uml/uml_driver.c (umlSetCloseExec): Delete.
> (umlStartVMDaemon): Use util version instead.
> ---
>
> v2: new patch
>
>  src/uml/uml_driver.c |   19 +++----------------
>  src/util/bridge.c    |   19 +++++--------------
>  2 files changed, 8 insertions(+), 30 deletions(-)

> diff --git a/src/util/bridge.c b/src/util/bridge.c
> index 7204e64..a6b5768 100644
> --- a/src/util/bridge.c
> +++ b/src/util/bridge.c
> @@ -72,25 +72,16 @@ int
>  brInit(brControl **ctlp)
>  {
>     int fd;
> -    int flags;
>
>     if (!ctlp || *ctlp)
>         return EINVAL;
>
>     fd = socket(AF_INET, SOCK_STREAM, 0);
> -    if (fd < 0)
> -        return errno;
> -
> -    if ((flags = fcntl(fd, F_GETFD)) < 0 ||
> -        fcntl(fd, F_SETFD, flags | FD_CLOEXEC) < 0) {
> -        int err = errno;
> -        VIR_FORCE_CLOSE(fd);
> -        return err;
> -    }
> -
> -    if (VIR_ALLOC(*ctlp) < 0) {
> +    if (fd < 0 ||
> +        virSetCloseExec(fd) < 0 ||
> +        VIR_ALLOC(*ctlp) < 0) {
>         VIR_FORCE_CLOSE(fd);
> -        return ENOMEM;
> +        return errno;
>     }

Is it guaranteed that calloc will set ENOMEM, or do we need some
gnulib module to guarantee this?

>     (*ctlp)->fd = fd;
> @@ -599,7 +590,7 @@ brSetInterfaceUp(brControl *ctl,
>                  int up)
>  {
>     struct ifreq ifr;
> -    int flags;
> +    unsigned int flags;

flags is used used with ifr.ifr_flags that is signed (actually it's a
short). So I'd prefer renaming it to ifflags and keep it as int.

ACK, with that questions/comments answered/addressed.

-- 
Matthias Bolte
http://photron.blogspot.com

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]