Re: [Qemu-devel] [PATCH v3 03/10] sheepdog: check return values of qemu_co_recv/send correctly

[Liu Yuan <namei.unix@gmail.com>]

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