diff mbox

[v3,03/10] sheepdog: check return values of qemu_co_recv/send correctly

Message ID 1374741125-31859-4-git-send-email-morita.kazutaka@lab.ntt.co.jp
State New
Headers show

Commit Message

MORITA Kazutaka July 25, 2013, 8:31 a.m. UTC 
If qemu_co_recv/send doesn't return the specified length, it means
that an error happened.

Signed-off-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
---
 block/sheepdog.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

Comments

Liu Yuan July 25, 2013, 8:46 a.m. UTC | #1 
On Thu, Jul 25, 2013 at 05:31:58PM +0900, MORITA Kazutaka wrote:
> If qemu_co_recv/send doesn't return the specified length, it means
> that an error happened.
> 
> Signed-off-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
> ---
>  block/sheepdog.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/block/sheepdog.c b/block/sheepdog.c
> index 6a41ad9..c6e9b89 100644
> --- a/block/sheepdog.c
> +++ b/block/sheepdog.c
> @@ -489,13 +489,13 @@ static coroutine_fn int send_co_req(int sockfd, SheepdogReq *hdr, void *data,
>      int ret;
>  
>      ret = qemu_co_send(sockfd, hdr, sizeof(*hdr));
> -    if (ret < sizeof(*hdr)) {
> +    if (ret != sizeof(*hdr)) {
>          error_report("failed to send a req, %s", strerror(errno));
>          return ret;
>      }
>  
>      ret = qemu_co_send(sockfd, data, *wlen);
> -    if (ret < *wlen) {
> +    if (ret != *wlen) {
>          error_report("failed to send a req, %s", strerror(errno));
>      }
>  
> @@ -548,7 +548,7 @@ static coroutine_fn void do_co_req(void *opaque)
>      qemu_aio_set_fd_handler(sockfd, restart_co_req, NULL, have_co_req, co);
>  
>      ret = qemu_co_recv(sockfd, hdr, sizeof(*hdr));
> -    if (ret < sizeof(*hdr)) {
> +    if (ret != sizeof(*hdr)) {
>          error_report("failed to get a rsp, %s", strerror(errno));
>          ret = -errno;
>          goto out;
> @@ -560,7 +560,7 @@ static coroutine_fn void do_co_req(void *opaque)
>  
>      if (*rlen) {
>          ret = qemu_co_recv(sockfd, data, *rlen);
> -        if (ret < *rlen) {
> +        if (ret != *rlen) {
>              error_report("failed to get the data, %s", strerror(errno));
>              ret = -errno;
>              goto out;
> @@ -671,7 +671,7 @@ static void coroutine_fn aio_read_response(void *opaque)
>  
>      /* read a header */
>      ret = qemu_co_recv(fd, &rsp, sizeof(rsp));
> -    if (ret < 0) {
> +    if (ret != sizeof(rsp)) {
>          error_report("failed to get the header, %s", strerror(errno));
>          goto out;
>      }
> @@ -722,7 +722,7 @@ static void coroutine_fn aio_read_response(void *opaque)
>      case AIOCB_READ_UDATA:
>          ret = qemu_co_recvv(fd, acb->qiov->iov, acb->qiov->niov,
>                              aio_req->iov_offset, rsp.data_length);
> -        if (ret < 0) {
> +        if (ret != rsp.data_length) {
>              error_report("failed to get the data, %s", strerror(errno));
>              goto out;
>          }
> @@ -1075,7 +1075,7 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
>  
>      /* send a header */
>      ret = qemu_co_send(s->fd, &hdr, sizeof(hdr));
> -    if (ret < 0) {
> +    if (ret != sizeof(hdr)) {
>          qemu_co_mutex_unlock(&s->lock);
>          error_report("failed to send a req, %s", strerror(errno));
>          return -errno;
> @@ -1083,7 +1083,7 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
>  
>      if (wlen) {
>          ret = qemu_co_sendv(s->fd, iov, niov, aio_req->iov_offset, wlen);
> -        if (ret < 0) {
> +        if (ret != wlen) {
>              qemu_co_mutex_unlock(&s->lock);
>              error_report("failed to send a data, %s", strerror(errno));
>              return -errno;

These checks are wrong because signed int will be converted to unsgned int. E.g,

ret = -1;
(ret < sizeof(hdr)) will always be false, since -1 is converted to unsigned int.

Thanks
Yuan
MORITA Kazutaka July 25, 2013, 8:53 a.m. UTC | #2 
At Thu, 25 Jul 2013 16:46:36 +0800,
Liu Yuan wrote:
> 
> On Thu, Jul 25, 2013 at 05:31:58PM +0900, MORITA Kazutaka wrote:
> > If qemu_co_recv/send doesn't return the specified length, it means
> > that an error happened.
> > 
> > Signed-off-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
> > ---
> >  block/sheepdog.c | 16 ++++++++--------
> >  1 file changed, 8 insertions(+), 8 deletions(-)
> > 
> > diff --git a/block/sheepdog.c b/block/sheepdog.c
> > index 6a41ad9..c6e9b89 100644
> > --- a/block/sheepdog.c
> > +++ b/block/sheepdog.c
> > @@ -489,13 +489,13 @@ static coroutine_fn int send_co_req(int sockfd, SheepdogReq *hdr, void *data,
> >      int ret;
> >  
> >      ret = qemu_co_send(sockfd, hdr, sizeof(*hdr));
> > -    if (ret < sizeof(*hdr)) {
> > +    if (ret != sizeof(*hdr)) {
> >          error_report("failed to send a req, %s", strerror(errno));
> >          return ret;
> >      }
> >  
> >      ret = qemu_co_send(sockfd, data, *wlen);
> > -    if (ret < *wlen) {
> > +    if (ret != *wlen) {
> >          error_report("failed to send a req, %s", strerror(errno));
> >      }
> >  
> > @@ -548,7 +548,7 @@ static coroutine_fn void do_co_req(void *opaque)
> >      qemu_aio_set_fd_handler(sockfd, restart_co_req, NULL, have_co_req, co);
> >  
> >      ret = qemu_co_recv(sockfd, hdr, sizeof(*hdr));
> > -    if (ret < sizeof(*hdr)) {
> > +    if (ret != sizeof(*hdr)) {
> >          error_report("failed to get a rsp, %s", strerror(errno));
> >          ret = -errno;
> >          goto out;
> > @@ -560,7 +560,7 @@ static coroutine_fn void do_co_req(void *opaque)
> >  
> >      if (*rlen) {
> >          ret = qemu_co_recv(sockfd, data, *rlen);
> > -        if (ret < *rlen) {
> > +        if (ret != *rlen) {
> >              error_report("failed to get the data, %s", strerror(errno));
> >              ret = -errno;
> >              goto out;
> > @@ -671,7 +671,7 @@ static void coroutine_fn aio_read_response(void *opaque)
> >  
> >      /* read a header */
> >      ret = qemu_co_recv(fd, &rsp, sizeof(rsp));
> > -    if (ret < 0) {
> > +    if (ret != sizeof(rsp)) {
> >          error_report("failed to get the header, %s", strerror(errno));
> >          goto out;
> >      }
> > @@ -722,7 +722,7 @@ static void coroutine_fn aio_read_response(void *opaque)
> >      case AIOCB_READ_UDATA:
> >          ret = qemu_co_recvv(fd, acb->qiov->iov, acb->qiov->niov,
> >                              aio_req->iov_offset, rsp.data_length);
> > -        if (ret < 0) {
> > +        if (ret != rsp.data_length) {
> >              error_report("failed to get the data, %s", strerror(errno));
> >              goto out;
> >          }
> > @@ -1075,7 +1075,7 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
> >  
> >      /* send a header */
> >      ret = qemu_co_send(s->fd, &hdr, sizeof(hdr));
> > -    if (ret < 0) {
> > +    if (ret != sizeof(hdr)) {
> >          qemu_co_mutex_unlock(&s->lock);
> >          error_report("failed to send a req, %s", strerror(errno));
> >          return -errno;
> > @@ -1083,7 +1083,7 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
> >  
> >      if (wlen) {
> >          ret = qemu_co_sendv(s->fd, iov, niov, aio_req->iov_offset, wlen);
> > -        if (ret < 0) {
> > +        if (ret != wlen) {
> >              qemu_co_mutex_unlock(&s->lock);
> >              error_report("failed to send a data, %s", strerror(errno));
> >              return -errno;
> 
> These checks are wrong because signed int will be converted to unsgned int. E.g,
> 
> ret = -1;
> (ret < sizeof(hdr)) will always be false, since -1 is converted to unsigned int.

Yes, that's the reason I replaced (ret < sizeof(hdr)) with (ret != sizeof(hdr)).

ret = -1;
(ret != sizeof(hdr)) will be true.

Thanks,

Kazutaka
Liu Yuan July 25, 2013, 8:56 a.m. UTC | #3 
On Thu, Jul 25, 2013 at 04:46:36PM +0800, Liu Yuan wrote:
> On Thu, Jul 25, 2013 at 05:31:58PM +0900, MORITA Kazutaka wrote:
> > If qemu_co_recv/send doesn't return the specified length, it means
> > that an error happened.
> > 
> > Signed-off-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
> > ---
> >  block/sheepdog.c | 16 ++++++++--------
> >  1 file changed, 8 insertions(+), 8 deletions(-)
> > 
> > diff --git a/block/sheepdog.c b/block/sheepdog.c
> > index 6a41ad9..c6e9b89 100644
> > --- a/block/sheepdog.c
> > +++ b/block/sheepdog.c
> > @@ -489,13 +489,13 @@ static coroutine_fn int send_co_req(int sockfd, SheepdogReq *hdr, void *data,
> >      int ret;
> >  
> >      ret = qemu_co_send(sockfd, hdr, sizeof(*hdr));
> > -    if (ret < sizeof(*hdr)) {
> > +    if (ret != sizeof(*hdr)) {
> >          error_report("failed to send a req, %s", strerror(errno));
> >          return ret;
> >      }
> >  
> >      ret = qemu_co_send(sockfd, data, *wlen);
> > -    if (ret < *wlen) {
> > +    if (ret != *wlen) {
> >          error_report("failed to send a req, %s", strerror(errno));
> >      }
> >  
> > @@ -548,7 +548,7 @@ static coroutine_fn void do_co_req(void *opaque)
> >      qemu_aio_set_fd_handler(sockfd, restart_co_req, NULL, have_co_req, co);
> >  
> >      ret = qemu_co_recv(sockfd, hdr, sizeof(*hdr));
> > -    if (ret < sizeof(*hdr)) {
> > +    if (ret != sizeof(*hdr)) {
> >          error_report("failed to get a rsp, %s", strerror(errno));
> >          ret = -errno;
> >          goto out;
> > @@ -560,7 +560,7 @@ static coroutine_fn void do_co_req(void *opaque)
> >  
> >      if (*rlen) {
> >          ret = qemu_co_recv(sockfd, data, *rlen);
> > -        if (ret < *rlen) {
> > +        if (ret != *rlen) {
> >              error_report("failed to get the data, %s", strerror(errno));
> >              ret = -errno;
> >              goto out;
> > @@ -671,7 +671,7 @@ static void coroutine_fn aio_read_response(void *opaque)
> >  
> >      /* read a header */
> >      ret = qemu_co_recv(fd, &rsp, sizeof(rsp));
> > -    if (ret < 0) {
> > +    if (ret != sizeof(rsp)) {
> >          error_report("failed to get the header, %s", strerror(errno));
> >          goto out;
> >      }
> > @@ -722,7 +722,7 @@ static void coroutine_fn aio_read_response(void *opaque)
> >      case AIOCB_READ_UDATA:
> >          ret = qemu_co_recvv(fd, acb->qiov->iov, acb->qiov->niov,
> >                              aio_req->iov_offset, rsp.data_length);
> > -        if (ret < 0) {
> > +        if (ret != rsp.data_length) {
> >              error_report("failed to get the data, %s", strerror(errno));
> >              goto out;
> >          }
> > @@ -1075,7 +1075,7 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
> >  
> >      /* send a header */
> >      ret = qemu_co_send(s->fd, &hdr, sizeof(hdr));
> > -    if (ret < 0) {
> > +    if (ret != sizeof(hdr)) {
> >          qemu_co_mutex_unlock(&s->lock);
> >          error_report("failed to send a req, %s", strerror(errno));
> >          return -errno;
> > @@ -1083,7 +1083,7 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
> >  
> >      if (wlen) {
> >          ret = qemu_co_sendv(s->fd, iov, niov, aio_req->iov_offset, wlen);
> > -        if (ret < 0) {
> > +        if (ret != wlen) {
> >              qemu_co_mutex_unlock(&s->lock);
> >              error_report("failed to send a data, %s", strerror(errno));
> >              return -errno;
> 
> These checks are wrong because signed int will be converted to unsgned int. E.g,
> 
> ret = -1;
> (ret < sizeof(hdr)) will always be false, since -1 is converted to unsigned int.

Sorry, please ingnore this comment

Thanks
Yuan
diff mbox

Patch

diff --git a/block/sheepdog.c b/block/sheepdog.c
index 6a41ad9..c6e9b89 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -489,13 +489,13 @@  static coroutine_fn int send_co_req(int sockfd, SheepdogReq *hdr, void *data,
     int ret;
 
     ret = qemu_co_send(sockfd, hdr, sizeof(*hdr));
-    if (ret < sizeof(*hdr)) {
+    if (ret != sizeof(*hdr)) {
         error_report("failed to send a req, %s", strerror(errno));
         return ret;
     }
 
     ret = qemu_co_send(sockfd, data, *wlen);
-    if (ret < *wlen) {
+    if (ret != *wlen) {
         error_report("failed to send a req, %s", strerror(errno));
     }
 
@@ -548,7 +548,7 @@  static coroutine_fn void do_co_req(void *opaque)
     qemu_aio_set_fd_handler(sockfd, restart_co_req, NULL, have_co_req, co);
 
     ret = qemu_co_recv(sockfd, hdr, sizeof(*hdr));
-    if (ret < sizeof(*hdr)) {
+    if (ret != sizeof(*hdr)) {
         error_report("failed to get a rsp, %s", strerror(errno));
         ret = -errno;
         goto out;
@@ -560,7 +560,7 @@  static coroutine_fn void do_co_req(void *opaque)
 
     if (*rlen) {
         ret = qemu_co_recv(sockfd, data, *rlen);
-        if (ret < *rlen) {
+        if (ret != *rlen) {
             error_report("failed to get the data, %s", strerror(errno));
             ret = -errno;
             goto out;
@@ -671,7 +671,7 @@  static void coroutine_fn aio_read_response(void *opaque)
 
     /* read a header */
     ret = qemu_co_recv(fd, &rsp, sizeof(rsp));
-    if (ret < 0) {
+    if (ret != sizeof(rsp)) {
         error_report("failed to get the header, %s", strerror(errno));
         goto out;
     }
@@ -722,7 +722,7 @@  static void coroutine_fn aio_read_response(void *opaque)
     case AIOCB_READ_UDATA:
         ret = qemu_co_recvv(fd, acb->qiov->iov, acb->qiov->niov,
                             aio_req->iov_offset, rsp.data_length);
-        if (ret < 0) {
+        if (ret != rsp.data_length) {
             error_report("failed to get the data, %s", strerror(errno));
             goto out;
         }
@@ -1075,7 +1075,7 @@  static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
 
     /* send a header */
     ret = qemu_co_send(s->fd, &hdr, sizeof(hdr));
-    if (ret < 0) {
+    if (ret != sizeof(hdr)) {
         qemu_co_mutex_unlock(&s->lock);
         error_report("failed to send a req, %s", strerror(errno));
         return -errno;
@@ -1083,7 +1083,7 @@  static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
 
     if (wlen) {
         ret = qemu_co_sendv(s->fd, iov, niov, aio_req->iov_offset, wlen);
-        if (ret < 0) {
+        if (ret != wlen) {
             qemu_co_mutex_unlock(&s->lock);
             error_report("failed to send a data, %s", strerror(errno));
             return -errno;