From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <nhorman@tuxdriver.com>
Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58])
 by dpdk.org (Postfix) with ESMTP id 85CC0324D
 for <dev@dpdk.org>; Sat, 31 Mar 2018 17:28:46 +0200 (CEST)
Received: from cpe-2606-a000-111b-40b7-215-ff-fecc-4872.dyn6.twc.com
 ([2606:a000:111b:40b7:215:ff:fecc:4872] helo=localhost)
 by smtp.tuxdriver.com with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.63)
 (envelope-from <nhorman@tuxdriver.com>)
 id 1f2IQe-0007sX-RF; Sat, 31 Mar 2018 11:28:31 -0400
Date: Sat, 31 Mar 2018 11:27:55 -0400
From: Neil Horman <nhorman@tuxdriver.com>
To: =?iso-8859-1?Q?Ga=EBtan?= Rivet <gaetan.rivet@6wind.com>
Cc: Tonghao Zhang <xiangxia.m.yue@gmail.com>,
 Timothy Redaelli <tredaelli@redhat.com>,
 Maxime Coquelin <maxime.coquelin@redhat.com>,
 Andrew Rybchenko <arybchenko@solarflare.com>,
 "dev@dpdk.org" <dev@dpdk.org>, Ferruh Yigit <ferruh.yigit@intel.com>,
 Thomas Monjalon <thomas@monjalon.net>
Message-ID: <20180331152755.GA3261@neilslaptop.think-freely.org>
References: <a28e7585-c312-8b0e-4e6d-1f6621a35feb@solarflare.com>
 <93d00a28-d52d-25b3-42d0-84b1d95c756a@redhat.com>
 <20180330162830.1d7ccba2@redhat.com>
 <CAMDZJNV62FL7WMgPsy0+pMYT47gDNV3vidFSRJqAix9bGHe1OQ@mail.gmail.com>
 <20180331133342.GA31292@neilslaptop.think-freely.org>
 <20180331150947.5ob35sxk2iw4f4fq@bidouze.vm.6wind.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20180331150947.5ob35sxk2iw4f4fq@bidouze.vm.6wind.com>
User-Agent: Mutt/1.9.2 (2017-12-15)
X-Spam-Score: -2.9 (--)
X-Spam-Status: No
Subject: Re: [dpdk-dev] Build is broken in dpdk-next-net
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Sat, 31 Mar 2018 15:28:46 -0000

On Sat, Mar 31, 2018 at 05:09:47PM +0200, Gaƫtan Rivet wrote:
> On Sat, Mar 31, 2018 at 09:33:43AM -0400, Neil Horman wrote:
> > On Fri, Mar 30, 2018 at 10:47:09PM +0800, Tonghao Zhang wrote:
> > > I rebuild it on ubuntu 17.10 and cash it. I use the 'RTE_SET_USED' to fix it.
> > > 
> > > 
> > > diff --git a/lib/librte_vhost/fd_man.c b/lib/librte_vhost/fd_man.c
> > > index 771675718..f11803191 100644
> > > --- a/lib/librte_vhost/fd_man.c
> > > +++ b/lib/librte_vhost/fd_man.c
> > > @@ -279,7 +279,8 @@ fdset_pipe_read_cb(int readfd, void *dat __rte_unused,
> > >                    int *remove __rte_unused)
> > >  {
> > >         char charbuf[16];
> > > -       read(readfd, charbuf, sizeof(charbuf));
> > > +       int r = read(readfd, charbuf, sizeof(charbuf));
> > > +       RTE_SET_USED(r);
> > >  }
> > > 
> > >  void
> > > @@ -319,5 +320,6 @@ fdset_pipe_init(struct fdset *fdset)
> > >  void
> > >  fdset_pipe_notify(struct fdset *fdset)
> > >  {
> > > -       write(fdset->u.writefd, "1", 1);
> > > +       int r = write(fdset->u.writefd, "1", 1);
> > > +       RTE_SET_USED(r);
> > >  }
> > > 
> > 
> > A better option might be to use _Pragma
> > 
> > Something like this perhaps
> > 
> > #define ALLOW_UNUSED(x) \
> > _Pragma(push) \
> > _Pragma(diagnostic ignored "-Wunused-result") \
> > #x;\
> > _Pragma(pop)
> > 
> > This is of course untested, so it probably needs some tweaking, but this method
> > avoids the need to declare an additional stack variable, which i don't think can
> > be eliminated due to the cast.  I believe that this method should also work
> > accross compilers (the gcc and clang compilers support this, and i think the
> > intel compiler should as well)
> > 
> > Neil
> > 
> 
> It would be nice to avoid the definition of a useless variable.
> An alternative could be
> 
>    if (read() < 0) {
>        /* Failure here is acceptable for such and such reason. */
>    }
> 
> to ensure all-around compatibility, and the definition or another macro.
> Just a suggestion.
> 
That would be a good alternative, but I think its effectiveness is dependent on
when the compiler does with the return value check. Without any code inside the
conditional, the compiler may optimize the check out, meaning the warning will
still be asserted.  If it doesn't optimize the check out, then you have a
useless compare and jump instruction left in the code path.

Best
Neil