From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl0-f48.google.com (mail-pl0-f48.google.com [209.85.160.48]) by dpdk.org (Postfix) with ESMTP id 32244AAF5 for ; Mon, 2 Apr 2018 18:25:20 +0200 (CEST) Received: by mail-pl0-f48.google.com with SMTP id v5-v6so2773627plo.4 for ; Mon, 02 Apr 2018 09:25:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=kutupBPiS/RQ1dwtSsn8wcccK/PoeyMvl7ImPsnq+aM=; b=Zfx+/n8QyPL2zKmTlMCxUdqBstIeUqAd2ILUftXACXZIMz5MJWAXrYFimsOu1v3yNQ KFvrmGqAj/I8/pd6Dkw6R3wZeO0bmQo/YeYbXV3Kgecse5Ua2YuZHSSo4Z0NhDd84qM5 aH9otjhWCigMLsIuvMLDBeimoRLpCmKLGAlwrQOy6AQByALcqL87nhlLLYnYf0U/vuSv 3eLFNBOIwFfXIKRzOGeEDMEchZhtEI/7Agif3Qlz5KpZeJpfbkal6NK3m58TmqkWBhMp m3J2AITS4HaX4sTxWwEGlxQ5+/cKRNQqfYccoXQ0C/XKyEG6cvwwLVKmPcnyYUkl29fv QHvw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=kutupBPiS/RQ1dwtSsn8wcccK/PoeyMvl7ImPsnq+aM=; b=UdJqGM0QcxtVprO2te/cS5TDz0FUjTOPe20+ezLUrileC1pYO6o3nidFld9ng7xXPU RcMJQxHUdV7K/+f1ABjP4ZdGXHCnLhw5jfh2ZydLxm+i8Bk0Mk6/ZxHE7AggMI4zyDNu HxM5109/Mvyr+dsEp/aYk0GJOYX14B5krQQvRlXMRuqkpzhGma67oAJMTbXrnEYBVm0K 8GBtDKbpzAZ0TkFbXig0Ks0UJ/uPwtHlylWi/VQ8JSw8bJD43D1o4yffb4FFP/N2i4/O OxMWi1eQLlaNNKJemYEeywrwypuVUKRMeZHW1LpBJjVIS2yP11X54WCw9/6r5kFtlyTU QmsQ== X-Gm-Message-State: AElRT7Fsid9/DkdvcnpHShH9G9HXGcNB7DBm5vMGOeyk/CYhOZMObXZQ 5GNb4BinANPhnmOtefa7Z8268w== X-Google-Smtp-Source: AIpwx4/BNowZ2IkOZSSLjKFrUAXOfRbhDY/3E4hxJMz+lL4Aae/RGlNq5OMKxd9+Uursd1PGr88z+A== X-Received: by 10.99.56.8 with SMTP id f8mr6742317pga.374.1522686319194; Mon, 02 Apr 2018 09:25:19 -0700 (PDT) Received: from xeon-e3 (204-195-71-95.wavecable.com. [204.195.71.95]) by smtp.gmail.com with ESMTPSA id g85sm1822622pfd.109.2018.04.02.09.25.18 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 02 Apr 2018 09:25:19 -0700 (PDT) Date: Mon, 2 Apr 2018 09:25:15 -0700 From: Stephen Hemminger To: Neil Horman Cc: =?UTF-8?B?R2HDq3Rhbg==?= Rivet , Tonghao Zhang , Timothy Redaelli , Maxime Coquelin , Andrew Rybchenko , "dev@dpdk.org" , Ferruh Yigit , Thomas Monjalon Message-ID: <20180402092515.3e5ccea4@xeon-e3> In-Reply-To: <20180331184855.GA4278@neilslaptop.think-freely.org> References: <93d00a28-d52d-25b3-42d0-84b1d95c756a@redhat.com> <20180330162830.1d7ccba2@redhat.com> <20180331133342.GA31292@neilslaptop.think-freely.org> <20180331150947.5ob35sxk2iw4f4fq@bidouze.vm.6wind.com> <20180331152755.GA3261@neilslaptop.think-freely.org> <20180331162141.nmg6awzep53fgakz@bidouze.vm.6wind.com> <20180331184855.GA4278@neilslaptop.think-freely.org> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 02 Apr 2018 16:25:20 -0000 On Sat, 31 Mar 2018 14:48:55 -0400 Neil Horman wrote: > On Sat, Mar 31, 2018 at 06:21:41PM +0200, Ga=C3=ABtan Rivet wrote: > > On Sat, Mar 31, 2018 at 11:27:55AM -0400, Neil Horman wrote: =20 > > > On Sat, Mar 31, 2018 at 05:09:47PM +0200, Ga=C3=ABtan Rivet wrote: =20 > > > > On Sat, Mar 31, 2018 at 09:33:43AM -0400, Neil Horman wrote: =20 > > > > > On Fri, Mar 30, 2018 at 10:47:09PM +0800, Tonghao Zhang wrote: =20 > > > > > > I rebuild it on ubuntu 17.10 and cash it. I use the 'RTE_SET_US= ED' to fix it. > > > > > >=20 > > > > > >=20 > > > > > > diff --git a/lib/librte_vhost/fd_man.c b/lib/librte_vhost/fd_ma= n.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 =3D read(readfd, charbuf, sizeof(charbuf)); > > > > > > + RTE_SET_USED(r); > > > > > > } > > > > > >=20 > > > > > > 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 =3D write(fdset->u.writefd, "1", 1); > > > > > > + RTE_SET_USED(r); > > > > > > } > > > > > > =20 > > > > >=20 > > > > > A better option might be to use _Pragma > > > > >=20 > > > > > Something like this perhaps > > > > >=20 > > > > > #define ALLOW_UNUSED(x) \ > > > > > _Pragma(push) \ > > > > > _Pragma(diagnostic ignored "-Wunused-result") \ > > > > > #x;\ > > > > > _Pragma(pop) > > > > >=20 > > > > > This is of course untested, so it probably needs some tweaking, b= ut 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) > > > > >=20 > > > > > Neil > > > > > =20 > > > >=20 > > > > It would be nice to avoid the definition of a useless variable. > > > > An alternative could be > > > >=20 > > > > if (read() < 0) { > > > > /* Failure here is acceptable for such and such reason. */ > > > > } > > > >=20 > > > > to ensure all-around compatibility, and the definition or another m= acro. > > > > Just a suggestion. > > > > =20 > > > That would be a good alternative, but I think its effectiveness is de= pendent 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 war= ning will > > > still be asserted. If it doesn't optimize the check out, then you ha= ve a > > > useless compare and jump instruction left in the code path. > > >=20 > > > Best > > > Neil > > > =20 > >=20 > > I tested quickly, I see no difference with the three methods: =20 >=20 > gcc seems to be sufficiently smart to optimize out the conditional, clang= not so > much: >=20 > #include > #include > #include >=20 > __attribute__((warn_unused_result)) > int wur(void) > { > printf("CALLING WUR!\n"); > return read(0, NULL, 0); > } >=20 > #define UNUSED_RESULT(x) if (x) {} >=20 > int main(void) > { > UNUSED_RESULT(wur()); > return 0; > } >=20 > [nhorman@neilslaptop ~]$ gcc -g -Wunused-result -Werror ./test.c > [nhorman@neilslaptop ~]$ objdump -d -S a.out > ./results > [nhorman@neilslaptop ~]$ cat results > ...=20 > 000000000040054b
: >=20 > #define UNUSED_RESULT(x) if (x) {} >=20 > int main(void) > { > 40054b: 55 push %rbp > 40054c: 48 89 e5 mov %rsp,%rbp > UNUSED_RESULT(wur()); > 40054f: e8 d3 ff ff ff callq 400527 > return 0; > 400554: b8 00 00 00 00 mov $0x0,%eax > } > 400559: 5d pop %rbp > 40055a: c3 retq > 40055b: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1) >=20 >=20 > [nhorman@neilslaptop ~]$ clang -g -Wunused-result -Werror ./test.c > [nhorman@neilslaptop ~]$ objdump -d -S a.out > ./results > [nhorman@neilslaptop ~]$ cat results=20 > ... > 0000000000400570
: > } >=20 > #define UNUSED_RESULT(x) if (x) {} >=20 > int main(void) > { > 400570: 55 push %rbp > 400571: 48 89 e5 mov %rsp,%rbp > 400574: 48 83 ec 10 sub $0x10,%rsp > 400578: c7 45 fc 00 00 00 00 movl $0x0,-0x4(%rbp) > UNUSED_RESULT(wur()); > 40057f: e8 ac ff ff ff callq 400530 > 400584: 83 f8 00 cmp $0x0,%eax > 400587: 0f 84 05 00 00 00 je 400592 > 40058d: e9 00 00 00 00 jmpq 400592 > 400592: 31 c0 xor %eax,%eax > return 0; > 400594: 48 83 c4 10 add $0x10,%rsp > 400598: 5d pop %rbp > 400599: c3 retq > 40059a: 66 0f 1f 44 00 00 nopw 0x0(%rax,%rax,1) >=20 >=20 > There is an additional compare and two jump statements there. I'm sure > eventually most compilers will figure out how to eliminate this, and it m= ight > even do so now with the right optimization flags, but I think its best to= just > organize the source such that no conditional branching is implied. Assum= ing the > intel compiler supports it (which I think it should, can someone with acc= ess to > it confirm), the _Pragma utility is probably the most clear way to do tha= t. >=20 > Regards > Neil Rather than wallpapering over the unused result, why not do real error chec= king? If the program was run in a non-Linux environment (such as WSL etc), maybe = an error could occur. Best to return an error; or at least call rte_exit().