* [Patch v2] xl: Fix CHK_ERRNO
@ 2013-12-09 15:12 Andrew Cooper
2013-12-09 17:34 ` George Dunlap
0 siblings, 1 reply; 2+ messages in thread
From: Andrew Cooper @ 2013-12-09 15:12 UTC (permalink / raw)
To: Xen-devel; +Cc: George Dunlap, Andrew Cooper, Ian Jackson, Ian Campbell
The macro CHK_ERRNO() was being used to check two different error schemes, and
succeeded at neither.
Split the macro into two; CHK_ERRNO() for calls which return -1 and set errno
on error, and CHK_LIBXLERR() for calls which return a positive errno.
In both cases, ensure that strerror() now gets called with the error integer.
Coverity ID: 1055570 1090374 1130516
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: George Dunlap <george.dunlap@eu.citrix.com>
---
Changes in v2:
* s/CHK_POSERRNO/CHK_LIBXLERR/
---
tools/libxl/xl_cmdimpl.c | 39 ++++++++++++++++++++++++++-------------
1 file changed, 26 insertions(+), 13 deletions(-)
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 4977a53..45cb658 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -41,15 +41,25 @@
#include "libxlutil.h"
#include "xl.h"
-#define CHK_ERRNO( call ) ({ \
+/* For calls which return positive errno on failure */
+#define CHK_LIBXLERR( call ) ({ \
int chk_errno = (call); \
- if (chk_errno < 0) { \
+ if (chk_errno > 0) { \
fprintf(stderr,"xl: fatal error: %s:%d: %s: %s\n", \
__FILE__,__LINE__, strerror(chk_errno), #call); \
exit(-ERROR_FAIL); \
} \
})
+/* For calls which return -1 and set errno on failure */
+#define CHK_ERRNO( call ) ({ \
+ if ((call) == -1) { \
+ fprintf(stderr,"xl: fatal error: %s:%d: %s: %s\n", \
+ __FILE__,__LINE__, strerror(errno), #call); \
+ exit(-ERROR_FAIL); \
+ } \
+ })
+
#define MUST( call ) ({ \
int must_rc = (call); \
if (must_rc < 0) { \
@@ -1982,8 +1992,9 @@ static uint32_t create_domain(struct domain_create *dom_info)
if (rc) return rc;
}
- CHK_ERRNO( libxl_read_exactly(ctx, restore_fd, &hdr,
- sizeof(hdr), restore_source, "header") );
+ CHK_LIBXLERR( libxl_read_exactly(
+ ctx, restore_fd, &hdr, sizeof(hdr),
+ restore_source, "header") );
if (memcmp(hdr.magic, savefileheader_magic, sizeof(hdr.magic))) {
fprintf(stderr, "File has wrong magic number -"
" corrupt or for a different tool?\n");
@@ -2008,8 +2019,10 @@ static uint32_t create_domain(struct domain_create *dom_info)
}
if (hdr.optional_data_len) {
optdata_begin = xmalloc(hdr.optional_data_len);
- CHK_ERRNO( libxl_read_exactly(ctx, restore_fd, optdata_begin,
- hdr.optional_data_len, restore_source, "optdata") );
+ CHK_LIBXLERR( libxl_read_exactly(
+ ctx, restore_fd, optdata_begin,
+ hdr.optional_data_len, restore_source,
+ "optdata") );
}
#define OPTDATA_LEFT (hdr.optional_data_len - (optdata_here - optdata_begin))
@@ -3337,9 +3350,9 @@ static void save_domain_core_writeconfig(int fd, const char *source,
/* that's the optional data */
- CHK_ERRNO( libxl_write_exactly(ctx, fd,
+ CHK_LIBXLERR( libxl_write_exactly(ctx, fd,
&hdr, sizeof(hdr), source, "header") );
- CHK_ERRNO( libxl_write_exactly(ctx, fd,
+ CHK_LIBXLERR( libxl_write_exactly(ctx, fd,
optdata_begin, hdr.optional_data_len, source, "header") );
fprintf(stderr, "Saving to %s new xl format (info"
@@ -3695,11 +3708,11 @@ static void migrate_receive(int debug, int daemonize, int monitor,
fprintf(stderr, "migration target: Ready to receive domain.\n");
- CHK_ERRNO( libxl_write_exactly(ctx, send_fd,
- migrate_receiver_banner,
- sizeof(migrate_receiver_banner)-1,
- "migration ack stream",
- "banner") );
+ CHK_LIBXLERR( libxl_write_exactly(ctx, send_fd,
+ migrate_receiver_banner,
+ sizeof(migrate_receiver_banner)-1,
+ "migration ack stream",
+ "banner") );
memset(&dom_info, 0, sizeof(dom_info));
dom_info.debug = debug;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [Patch v2] xl: Fix CHK_ERRNO
2013-12-09 15:12 [Patch v2] xl: Fix CHK_ERRNO Andrew Cooper
@ 2013-12-09 17:34 ` George Dunlap
0 siblings, 0 replies; 2+ messages in thread
From: George Dunlap @ 2013-12-09 17:34 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel; +Cc: Ian Jackson, Ian Campbell
On 12/09/2013 03:12 PM, Andrew Cooper wrote:
> The macro CHK_ERRNO() was being used to check two different error schemes, and
> succeeded at neither.
>
> Split the macro into two; CHK_ERRNO() for calls which return -1 and set errno
> on error, and CHK_LIBXLERR() for calls which return a positive errno.
>
> In both cases, ensure that strerror() now gets called with the error integer.
>
> Coverity ID: 1055570 1090374 1130516
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: George Dunlap <george.dunlap@eu.citrix.com>
Clearly a bug fix, if a minor one. It does touch the domain creation as
well as save/restore path, but is fairly simple and should also be very
well tested between now and the release.
Release-acked-by: George Dunlap <george.dunlap@eu.citrix.com>
>
> ---
>
> Changes in v2:
> * s/CHK_POSERRNO/CHK_LIBXLERR/
> ---
> tools/libxl/xl_cmdimpl.c | 39 ++++++++++++++++++++++++++-------------
> 1 file changed, 26 insertions(+), 13 deletions(-)
>
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 4977a53..45cb658 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -41,15 +41,25 @@
> #include "libxlutil.h"
> #include "xl.h"
>
> -#define CHK_ERRNO( call ) ({ \
> +/* For calls which return positive errno on failure */
> +#define CHK_LIBXLERR( call ) ({ \
> int chk_errno = (call); \
> - if (chk_errno < 0) { \
> + if (chk_errno > 0) { \
> fprintf(stderr,"xl: fatal error: %s:%d: %s: %s\n", \
> __FILE__,__LINE__, strerror(chk_errno), #call); \
> exit(-ERROR_FAIL); \
> } \
> })
>
> +/* For calls which return -1 and set errno on failure */
> +#define CHK_ERRNO( call ) ({ \
> + if ((call) == -1) { \
> + fprintf(stderr,"xl: fatal error: %s:%d: %s: %s\n", \
> + __FILE__,__LINE__, strerror(errno), #call); \
> + exit(-ERROR_FAIL); \
> + } \
> + })
> +
> #define MUST( call ) ({ \
> int must_rc = (call); \
> if (must_rc < 0) { \
> @@ -1982,8 +1992,9 @@ static uint32_t create_domain(struct domain_create *dom_info)
> if (rc) return rc;
> }
>
> - CHK_ERRNO( libxl_read_exactly(ctx, restore_fd, &hdr,
> - sizeof(hdr), restore_source, "header") );
> + CHK_LIBXLERR( libxl_read_exactly(
> + ctx, restore_fd, &hdr, sizeof(hdr),
> + restore_source, "header") );
> if (memcmp(hdr.magic, savefileheader_magic, sizeof(hdr.magic))) {
> fprintf(stderr, "File has wrong magic number -"
> " corrupt or for a different tool?\n");
> @@ -2008,8 +2019,10 @@ static uint32_t create_domain(struct domain_create *dom_info)
> }
> if (hdr.optional_data_len) {
> optdata_begin = xmalloc(hdr.optional_data_len);
> - CHK_ERRNO( libxl_read_exactly(ctx, restore_fd, optdata_begin,
> - hdr.optional_data_len, restore_source, "optdata") );
> + CHK_LIBXLERR( libxl_read_exactly(
> + ctx, restore_fd, optdata_begin,
> + hdr.optional_data_len, restore_source,
> + "optdata") );
> }
>
> #define OPTDATA_LEFT (hdr.optional_data_len - (optdata_here - optdata_begin))
> @@ -3337,9 +3350,9 @@ static void save_domain_core_writeconfig(int fd, const char *source,
>
> /* that's the optional data */
>
> - CHK_ERRNO( libxl_write_exactly(ctx, fd,
> + CHK_LIBXLERR( libxl_write_exactly(ctx, fd,
> &hdr, sizeof(hdr), source, "header") );
> - CHK_ERRNO( libxl_write_exactly(ctx, fd,
> + CHK_LIBXLERR( libxl_write_exactly(ctx, fd,
> optdata_begin, hdr.optional_data_len, source, "header") );
>
> fprintf(stderr, "Saving to %s new xl format (info"
> @@ -3695,11 +3708,11 @@ static void migrate_receive(int debug, int daemonize, int monitor,
>
> fprintf(stderr, "migration target: Ready to receive domain.\n");
>
> - CHK_ERRNO( libxl_write_exactly(ctx, send_fd,
> - migrate_receiver_banner,
> - sizeof(migrate_receiver_banner)-1,
> - "migration ack stream",
> - "banner") );
> + CHK_LIBXLERR( libxl_write_exactly(ctx, send_fd,
> + migrate_receiver_banner,
> + sizeof(migrate_receiver_banner)-1,
> + "migration ack stream",
> + "banner") );
>
> memset(&dom_info, 0, sizeof(dom_info));
> dom_info.debug = debug;
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2013-12-09 17:34 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-09 15:12 [Patch v2] xl: Fix CHK_ERRNO Andrew Cooper
2013-12-09 17:34 ` George Dunlap
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).