hw/usb/dev-mtp.c:1616: bad test ?

[dcb]

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)) {

[Peter Maydell (pmaydell)]

On Fri, 19 Oct 2018 at 10:22, dcb <email address hidden> 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

[Gerd Hoffmann (kraxel-redhat)]

On Tue, Jan 22, 2019 at 07:41:16AM -0500, Bandan Das wrote:
>
> qemu_write_full takes care of partial blocking writes,
> as in cases of larger file sizes
>
> Suggested-by: Peter Maydell <email address hidden>
> Signed-off-by: Bandan <email address hidden>

Hmm, doesn't apply, and git fails to do a 3way merge too due to unknown
sha1.

cheers,
  Gerd

[Gerd Hoffmann (kraxel-redhat)]

On Thu, Jan 24, 2019 at 03:19:03AM -0500, Bandan Das wrote:
> Gerd Hoffmann <email address hidden> writes:
>
> > On Tue, Jan 22, 2019 at 07:41:16AM -0500, Bandan Das wrote:
> >>
> >> qemu_write_full takes care of partial blocking writes,
> >> as in cases of larger file sizes
> >>
> >> Suggested-by: Peter Maydell <email address hidden>
> >> Signed-off-by: Bandan <email address hidden>
> >
> > Hmm, doesn't apply, and git fails to do a 3way merge too due to unknown
> > sha1.
>
> Oops, sorry, I realize now this is on top of the write buffer breakup patches.

Hmm, they are queued up already, so that should have worked.

> Should I resend a v2 on top of master and send a v3 for the write buffer breakup
> patches ?

Can you just send a single series with both this fix and the breakup
patches, against latest master?

thanks,
  Gerd

[Peter Maydell (pmaydell)]

Fixed by commit 49f9e8d660d4 which will be in QEMU 4.0.