Re: [Xen-devel] [PATCH] xl: Fix CHK_ERRNO

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2013-12-09 at 14:48 +0000, 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_POSERRNO() for calls which return a positive errno.

Would be better to call POSERRNO LIBXLERR or something, rather than
accidentally imply that it was related to "errno" somehow, I think.

> 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@xxxxxxxxxx>
> CC: Ian Campbell <Ian.Campbell@xxxxxxxxxx>
> CC: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
> CC: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
> ---
>  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..4709a0f 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_POSERRNO( 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_POSERRNO( 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_POSERRNO( 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_POSERRNO( libxl_write_exactly(ctx, fd,
>          &hdr, sizeof(hdr), source, "header") );
> -    CHK_ERRNO( libxl_write_exactly(ctx, fd,
> +    CHK_POSERRNO( 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_POSERRNO( 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;



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel