From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [Patch v2] xl: Fix CHK_ERRNO Date: Mon, 9 Dec 2013 17:34:11 +0000 Message-ID: <52A5FF13.2020901@eu.citrix.com> References: <1386601950-11394-1-git-send-email-andrew.cooper3@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1386601950-11394-1-git-send-email-andrew.cooper3@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Andrew Cooper , Xen-devel Cc: Ian Jackson , Ian Campbell List-Id: xen-devel@lists.xenproject.org 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 > CC: Ian Campbell > CC: Ian Jackson > CC: George Dunlap 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 > > --- > > 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;