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

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

 



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@xxxxxxxxxx    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

--
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]