From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH 17/28] libxl: gettimeofday doesn't return an errno on failure Date: Thu, 26 Sep 2013 09:51:39 +0100 Message-ID: <1380185499.29483.9.camel@kazak.uk.xensource.com> References: <1379475484-25993-1-git-send-email-mattjd@gmail.com> <1379475484-25993-18-git-send-email-mattjd@gmail.com> <1380110649.23688.122.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Matthew Daley Cc: Stefano Stabellini , Ian Jackson , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org On Thu, 2013-09-26 at 11:11 +1200, Matthew Daley wrote: > On Thu, Sep 26, 2013 at 12:04 AM, Ian Campbell wrote: > > On Wed, 2013-09-18 at 15:37 +1200, Matthew Daley wrote: > > > > http://pubs.opengroup.org/onlinepubs/9699919799/functions/gettimeofday.html > > agrees with this but the Linux manpages gettimeofday(2) disagrees: > > gettimeofday() and settimeofday() return 0 for success, or -1 for > > failure (in which case errno is set appropriately). > > > > They may just have confused themselves by lumping get in with set, but > > at least one error code (EFAULT) seems like it could apply to get too. > > I think my lack of description and my use of the "MUST" macro is > confusing you here. I agree that they can return values signifying > failure, but indeed they don't return error codes directly, instead > they return -1 and set errno. > > > > > Since the spec says it cannot fail I think there's no harm in reporting > > errno if it does fail. > > > > is the CHK_ERRNO macro correct? > > #define CHK_ERRNO( 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); \ > > } \ > > }) > > > > It seems to report the reutrn code and not errno, so it will always say > > -1 won't it? > > > > I think the actual coverity error "chk_errno" is passed to a parameter > > that cannot be negative." stems from this not the lack of errno, because > > coverity seems to know that strerror cannot take a negative number. > > Right, and this is what I was trying to avoid by changing to using the > MUST macro, but I suppose the lack of errno reportage is unacceptable > (and I guess that MUST is meant for internal xl functions that return > a exit()able return code, not just -1. Really, I think another macro > is needed that gets errno not from the call's return value, but errno > directly? I think the problem is that CHK_ERRNO is actually used to check both libc functions (which return -1 and set errno) and libxl function (which return -LIBXL_ERROR), and does neither of them properly: xl_cmdimpl.c: CHK_ERRNO( libxl_read_exactly(ctx, restore_f xl_cmdimpl.c: CHK_ERRNO( libxl_read_exactly(ctx, resto xl_cmdimpl.c: CHK_ERRNO(( logfile = open(fullname, O_WRONL xl_cmdimpl.c: CHK_ERRNO(( nullfd = open("/dev/null", O_RDO xl_cmdimpl.c: CHK_ERRNO(daemon(0, 1) < 0); xl_cmdimpl.c: CHK_ERRNO(asprintf(&config_source, "