Re: [PATCH] xl: Fix CHK_ERRNO

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 10/12/13 15:13, Ian Jackson wrote:
> Andrew Cooper writes ("Re: [PATCH] xl: Fix CHK_ERRNO"):
>> On 09/12/13 14:55, Ian Campbell wrote:
>>> [Andrew Cooper:]
>>>> 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.
> This is a bit confusing.  Why do you write "a _positive_ errno"
> (emph. mine) ?  errno values are always positive.  In the libxl LOG*
> macros we call a style where an errno value is passed explicitly
> "ERRNOVAL".
>
> You propose:
>
>     #define CHK_POSERRNO( call ) ({                                         \
>             int chk_errno = (call);                                         \
>             if (chk_errno > 0) {                                            \
>                 fprintf(stderr,"xl: fatal error: %s:%d: %s: %s\n",          \
>                         __FILE__,__LINE__, strerror(chk_errno), #call);     \
>                 exit(-ERROR_FAIL);                                          \
>             }                                                               \
>         })
>
> This is what I would call CHK_ERRNOVAL.  (But I think it should
> abort() if the returned value is negative, not treat it as success!)
>
>>> Would be better to call POSERRNO LIBXLERR or something, rather than
>>> accidentally imply that it was related to "errno" somehow, I think.
> I think there should be a CHK_LIBXL or something too, but that's not
> needed right now because all the CHK_* call sites are either
> (return -1, set errno) or (return errno value).
>
> I think the former macro would better be called CHK_SYSCALL, because
> it's the system call return convention.  CHK_ERRNO would do.
>
> Ian.

In v2 of the patch, CHK_POSERRNO was renamed to CHK_LIBXLERR, but I can
certainly extend it to abort() if negative.

I can also rename CHK_ERRNO to CHK_SYSCALL which does make it somewhat
more descriptive.

~Andrew