On 10/15/2010 01:32 PM, Eric Blake wrote:
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. Ok.
+++ 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/ Ok.
+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.
Your other mail...
+ +# define VIR_CLOSE(FD) \ + virClose(&(FD)) + + +#endif /* __VIR_FILES_H */ +
No change in src/libvirt_private.syms?
Was waiting for the linker to complain and it did not.
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 :(
Ok, will do.
@@ -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
ok.
+ + 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")); }
Replacing the example. Stefan