[libvirt] [PATCH] introduce VIR_CLOSE to be used rather than close()

Eric Blake eblake at redhat.com
Fri Oct 15 17:32:16 UTC 2010


On 10/15/2010 10:15 AM, Stefan Berger wrote:
>
> Index: libvirt-acl/src/util/files.c
> ===================================================================
> --- /dev/null
> +++ libvirt-acl/src/util/files.c

> + */
> +
> +#include <unistd.h>

Oops, need to include <config.h> first.

> +++ libvirt-acl/src/util/files.h
> @@ -0,0 +1,37 @@

> +#ifndef __VIR_FILES_H_
> +# define __VIR_FILES_H_
> +
> +# include "internal.h"
> +
> +/* Don't call these directly - use the macros below */

s/these/this/

> +int virClose(int *fdptr);

Needs an ATTRIBUTE_NONNULL(1).  I'm also debating whether it needs 
ATTRIBUTE_RETURN_CHECK; normally, close() fails in very few situations, 
and in cleanup paths, you tend to already have another error more 
important so you can ignore the secondary close() failures.  So I'm 
probably 80-20 against adding ATTRIBUTE_RETURN_CHECK.

> +
> +# define VIR_CLOSE(FD) \
> + virClose(&(FD))
> +
> +
> +#endif /* __VIR_FILES_H */
> +

No change in src/libvirt_private.syms?

> Index: libvirt-acl/HACKING
> ===================================================================
> --- libvirt-acl.orig/HACKING
> +++ libvirt-acl/HACKING

Yuck - we STILL don't have the auto-generation in place; edits should be 
to docs/hacking.html.in, and HACKING _should_ be generated from that. 
Until then, you need to touch both files :(


> @@ -318,6 +318,17 @@ routines, use the macros from memory.h
> VIR_FREE(domain);
>
>
> +File handling
> +=============
> +
> +Use of the close() API is deprecated in libvirt code base to help avoiding
> +double-closing of a filedescriptor. Instead of this API, use the macro
> from
> +files.h
> +
> + - eg close a filedescriptor

s/filedescriptor/file descriptor/g


> +
> + VIR_CLOSE(fd);

Although I'm 80-20 against warning on unused results, we might as well 
at least make the example demonstrate that the results are valid:

if (VIR_CLOSE(fd) < 0) {
     virReportSystemError(errno, _("failed to close file"));
}

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org




More information about the libvir-list mailing list