From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH 25/29] libvchan: check for fcntl failures in select-type sample application Date: Wed, 30 Oct 2013 17:13:38 +0000 Message-ID: <52713E42.7060607@citrix.com> References: <1383119525-26033-1-git-send-email-mattjd@gmail.com> <1383119525-26033-26-git-send-email-mattjd@gmail.com> <52713C94.1050001@tycho.nsa.gov> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <52713C94.1050001@tycho.nsa.gov> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Daniel De Graaf Cc: xen-devel@lists.xen.org, Matthew Daley , Ian Jackson , Ian Campbell , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org On 30/10/13 17:06, Daniel De Graaf wrote: > On 10/30/2013 03:52 AM, Matthew Daley wrote: >> Coverity-ID: 1055041 >> Signed-off-by: Matthew Daley >> --- >> tools/libvchan/node-select.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/tools/libvchan/node-select.c b/tools/libvchan/node-select.c >> index 6c6c19e..c6914ab 100644 >> --- a/tools/libvchan/node-select.c >> +++ b/tools/libvchan/node-select.c >> @@ -105,8 +105,10 @@ int main(int argc, char **argv) >> exit(1); >> } >> >> - fcntl(0, F_SETFL, O_NONBLOCK); >> - fcntl(1, F_SETFL, O_NONBLOCK); >> + if (fcntl(0, F_SETFL, O_NONBLOCK) == -1 || fcntl(1, F_SETFL, >> O_NONBLOCK) == -1) { >> + perror("fcntl"); >> + exit(1); >> + } >> >> libxenvchan_fd = libxenvchan_fd_for_select(ctrl); >> for (;;) { >> > > To be completely correct, a call to F_GETFL would be required first, with > the result ORed with O_NONBLOCK and passed to F_SETFL. That is a separate > existing bug in the code, however, so this patch is still an improvement > as-is. > > Is the fcntl on line 156 different in some way that does not trigger this > Coverity check? > Hmm - that error wasn't flagged in the slightest by Coverity. I would agree that it suffers from the same problem and needs appropriately-similar fixing. It is possible Coverity only flagged the first instance of all ignored return values from fcntl(1,...) library calls. Some of the checkers seem to have logic which decides that something consistently wrong/questionable might be by design. I suspect that if the above were fixed, then the latter would identified in the next scan. ~Andrew