* [Qemu-devel] [Bug 1798780] [NEW] hw/usb/dev-mtp.c:1616: bad test ? @ 2018-10-19 9:07 dcb 2019-01-18 17:18 ` Peter Maydell 2019-04-24 6:07 ` [Qemu-devel] [Bug 1798780] Re: hw/usb/dev-mtp.c:1616: bad test ? Thomas Huth 0 siblings, 2 replies; 8+ messages in thread From: dcb @ 2018-10-19 9:07 UTC (permalink / raw) To: qemu-devel Public bug reported: 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)) { ** Affects: qemu Importance: Undecided Status: New -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1798780 Title: hw/usb/dev-mtp.c:1616: bad test ? Status in QEMU: New Bug description: 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)) { To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1798780/+subscriptions ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [Bug 1798780] [NEW] hw/usb/dev-mtp.c:1616: bad test ? 2018-10-19 9:07 [Qemu-devel] [Bug 1798780] [NEW] hw/usb/dev-mtp.c:1616: bad test ? dcb @ 2019-01-18 17:18 ` Peter Maydell 2019-01-22 12:38 ` Bandan Das 2019-04-24 6:07 ` [Qemu-devel] [Bug 1798780] Re: hw/usb/dev-mtp.c:1616: bad test ? Thomas Huth 1 sibling, 1 reply; 8+ messages in thread From: Peter Maydell @ 2019-01-18 17:18 UTC (permalink / raw) To: Bug 1798780; +Cc: QEMU Developers, Bandan Das, Gerd Hoffmann 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [Bug 1798780] [NEW] hw/usb/dev-mtp.c:1616: bad test ? 2019-01-18 17:18 ` Peter Maydell @ 2019-01-22 12:38 ` Bandan Das 2019-01-22 12:41 ` [Qemu-devel] [PATCH] usb-mtp: replace the homebrew write with qemu_write_full Bandan Das 0 siblings, 1 reply; 8+ messages in thread From: Bandan Das @ 2019-01-22 12:38 UTC (permalink / raw) To: Peter Maydell; +Cc: Bug 1798780, QEMU Developers, Gerd Hoffmann Peter Maydell <peter.maydell@linaro.org> writes: > 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 for the suggestion, Peter. I agree, this isn't non-blocking and qemu_write_full() can be used. Following up with a patch. Bandan > thanks > -- PMM ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH] usb-mtp: replace the homebrew write with qemu_write_full 2019-01-22 12:38 ` Bandan Das @ 2019-01-22 12:41 ` Bandan Das 2019-01-24 8:01 ` Gerd Hoffmann 0 siblings, 1 reply; 8+ messages in thread From: Bandan Das @ 2019-01-22 12:41 UTC (permalink / raw) To: Peter Maydell, Gerd Hoffmann; +Cc: Bug 1798780, QEMU Developers qemu_write_full takes care of partial blocking writes, as in cases of larger file sizes Suggested-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Bandan <bsd@redhat.com> --- hw/usb/dev-mtp.c | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c index 31fa83589b..53e20e2161 100644 --- a/hw/usb/dev-mtp.c +++ b/hw/usb/dev-mtp.c @@ -1630,25 +1630,16 @@ static char *utf16_to_str(uint8_t len, uint16_t *arr) /* Wrapper around write, returns 0 on failure */ static uint64_t write_retry(int fd, void *buf, uint64_t size, off_t offset) { - uint64_t bytes_left = size, ret; + uint64_t ret = 0; if (lseek(fd, offset, SEEK_SET) < 0) { goto done; } - while (bytes_left > 0) { - ret = write(fd, buf, bytes_left); - if ((ret == -1) && (errno != EINTR || errno != EAGAIN || - errno != EWOULDBLOCK)) { - break; - } - bytes_left -= ret; - buf += ret; - } + ret = qemu_write_full(fd, buf, size); done: - return size - bytes_left; + return ret; } static void usb_mtp_update_object(MTPObject *parent, char *name) -- 2.20.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] usb-mtp: replace the homebrew write with qemu_write_full 2019-01-22 12:41 ` [Qemu-devel] [PATCH] usb-mtp: replace the homebrew write with qemu_write_full Bandan Das @ 2019-01-24 8:01 ` Gerd Hoffmann 2019-01-24 8:19 ` Bandan Das 0 siblings, 1 reply; 8+ messages in thread From: Gerd Hoffmann @ 2019-01-24 8:01 UTC (permalink / raw) To: Bandan Das; +Cc: Peter Maydell, Bug 1798780, QEMU Developers 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 <peter.maydell@linaro.org> > Signed-off-by: Bandan <bsd@redhat.com> Hmm, doesn't apply, and git fails to do a 3way merge too due to unknown sha1. cheers, Gerd ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] usb-mtp: replace the homebrew write with qemu_write_full 2019-01-24 8:01 ` Gerd Hoffmann @ 2019-01-24 8:19 ` Bandan Das 2019-01-24 8:31 ` Gerd Hoffmann 0 siblings, 1 reply; 8+ messages in thread From: Bandan Das @ 2019-01-24 8:19 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: Bug 1798780, Peter Maydell, QEMU Developers Gerd Hoffmann <kraxel@redhat.com> 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 <peter.maydell@linaro.org> >> Signed-off-by: Bandan <bsd@redhat.com> > > 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. Should I resend a v2 on top of master and send a v3 for the write buffer breakup patches ? > cheers, > Gerd ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] usb-mtp: replace the homebrew write with qemu_write_full 2019-01-24 8:19 ` Bandan Das @ 2019-01-24 8:31 ` Gerd Hoffmann 0 siblings, 0 replies; 8+ messages in thread From: Gerd Hoffmann @ 2019-01-24 8:31 UTC (permalink / raw) To: Bandan Das; +Cc: Bug 1798780, Peter Maydell, QEMU Developers On Thu, Jan 24, 2019 at 03:19:03AM -0500, Bandan Das wrote: > Gerd Hoffmann <kraxel@redhat.com> 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 <peter.maydell@linaro.org> > >> Signed-off-by: Bandan <bsd@redhat.com> > > > > 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [Bug 1798780] Re: hw/usb/dev-mtp.c:1616: bad test ? 2018-10-19 9:07 [Qemu-devel] [Bug 1798780] [NEW] hw/usb/dev-mtp.c:1616: bad test ? dcb 2019-01-18 17:18 ` Peter Maydell @ 2019-04-24 6:07 ` Thomas Huth 1 sibling, 0 replies; 8+ messages in thread From: Thomas Huth @ 2019-04-24 6:07 UTC (permalink / raw) To: qemu-devel ** Changed in: qemu Status: Fix Committed => Fix Released -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1798780 Title: hw/usb/dev-mtp.c:1616: bad test ? Status in QEMU: Fix Released Bug description: 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)) { To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1798780/+subscriptions ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-04-24 6:20 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-10-19 9:07 [Qemu-devel] [Bug 1798780] [NEW] hw/usb/dev-mtp.c:1616: bad test ? dcb 2019-01-18 17:18 ` Peter Maydell 2019-01-22 12:38 ` Bandan Das 2019-01-22 12:41 ` [Qemu-devel] [PATCH] usb-mtp: replace the homebrew write with qemu_write_full Bandan Das 2019-01-24 8:01 ` Gerd Hoffmann 2019-01-24 8:19 ` Bandan Das 2019-01-24 8:31 ` Gerd Hoffmann 2019-04-24 6:07 ` [Qemu-devel] [Bug 1798780] Re: hw/usb/dev-mtp.c:1616: bad test ? Thomas Huth
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.