[stgt] [PATCH] fix check of sent len in mtask_recv_send_handler(), report exceptions in concat_write()

[FUJITA Tomonori]

CC'ed Brad,

Can you try the patch?

Thanks,

On Wed, 18 Apr 2012 01:17:35 +0300
nezhinsky at gmail.com wrote:

> From: Alexander Nezhinsky <alexandern at mellanox.com>
> 
> This patch hopefully fixes the problem reported by Brad. Please verify. 
> 
> 1) mtask->done is reset to 0 when transitioning to MTASK_STATE_PDU_SEND because it is used
>  as the offset into concat_buf, while rsp->len accounts for the total response length, incl. the header;
>  thus the check if the entire mtask resp is sent should subtract header len.
> 2) when writing concat_buf to a socket file, if the buffer offset is past the buffer end,
>  return -1 and set errno=EINVAL instead of just returning 0.
>  The problem is that concat_write() regularly returns the retcode from write(), which is either
>  the number of written bytes or -1 (and then errno describes the error). Thus when we avoid
>  calling write() the same convention should be simulated.
>  Similar fix in case of a previous error, then concat_buf->err stores the relevant errno,
>  so -1 is returned and errno is reset to the error code.
> 
> Signed-off-by: Alexander Nezhinsky <alexandern at mellanox.com>
> ---
>  usr/concat_buf.c |   12 ++++++++----
>  usr/mgmt.c       |    4 ++--
>  2 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/usr/concat_buf.c b/usr/concat_buf.c
> index 9395f4b..00661d4 100644
> --- a/usr/concat_buf.c
> +++ b/usr/concat_buf.c
> @@ -74,13 +74,17 @@ int concat_write(struct concat_buf *b, int fd, int offset)
>  {
>  	concat_buf_finish(b);
>  
> -	if (b->err)
> -		return b->err;
> +	if (b->err) {
> +		errno = b->err;
> +		return -1;
> +	}
>  
>  	if (b->size - offset > 0)
>  		return write(fd, b->buf + offset, b->size - offset);
> -	else
> -		return 0;
> +	else {
> +		errno = EINVAL;
> +		return -1;
> +	}
>  }
>  
>  void concat_buf_release(struct concat_buf *b)
> diff --git a/usr/mgmt.c b/usr/mgmt.c
> index 8b62d13..8915dea 100644
> --- a/usr/mgmt.c
> +++ b/usr/mgmt.c
> @@ -590,9 +590,9 @@ static void mtask_recv_send_handler(int fd, int events, void *data)
>  		break;
>  	case MTASK_STATE_PDU_SEND:
>  		err = concat_write(&mtask->rsp_concat, fd, mtask->done);
> -		if (err > 0) {
> +		if (err >= 0) {
>  			mtask->done += err;
> -			if (mtask->done == rsp->len) {
> +			if (mtask->done == (rsp->len - sizeof(*rsp))) {
>  				if (req->mode == MODE_SYSTEM &&
>  				    req->op == OP_DELETE &&
>  				    !rsp->err)
> -- 
> 1.7.9.6
> 
> --
> To unsubscribe from this list: send the line "unsubscribe stgt" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html