Re: [Qemu-devel] [Bug 1798780] [NEW] hw/usb/dev-mtp.c:1616: bad test ?

From: Peter Maydell <peter.maydell@linaro.org>
To: Bug 1798780 <1798780@bugs.launchpad.net>
Cc: QEMU Developers <qemu-devel@nongnu.org>,
	Bandan Das <bsd@redhat.com>, Gerd Hoffmann <kraxel@redhat.com>
Subject: Re: [Qemu-devel] [Bug 1798780] [NEW] hw/usb/dev-mtp.c:1616: bad test ?
Date: Fri, 18 Jan 2019 17:18:23 +0000	[thread overview]
Message-ID: <CAFEAcA9vczSpf+xd+3so1YKAW-d4RKY7iVEtMvJrgSNbRjQ+Zg@mail.gmail.com> (raw)
In-Reply-To: <153994006052.15678.15962440455123359491.malonedeb@gac.canonical.com>

On Fri, 19 Oct 2018 at 10:22, dcb <1798780@bugs.launchpad.net> wrote:
> hw/usb/dev-mtp.c:1616:52: warning: logical ‘or’ of collectively
> exhaustive tests is always true [-Wlogical-op]
>
> Source code is
>
>                 if ((ret == -1) && (errno != EINTR || errno != EAGAIN ||
>                                     errno != EWOULDBLOCK)) {
>
> Maybe better code
>
>                 if ((ret == -1) && (errno != EINTR && errno != EAGAIN &&
>                                     errno != EWOULDBLOCK)) {

Hi Gerd, Bandan -- I was going through older launchpad bugs and
noticed that this one about a dubious conditional in dev-mtp.c is
still unfixed.

Is the file descriptor being used here one that's in non-blocking
mode? If so, then busy-waiting in a loop while the write() returns
EWOULDBLOCK is probably not what you wanted. If it's not then
there's no need to check for EAGAIN or EWOULDBLOCK, I think.
Consider using qemu_write_full() instead of open-coding
the retry loop ?

thanks
-- PMM