[PATCH] RDMA: usnic: Fix misuse of sysfs_emit_at

[PATCH] RDMA: usnic: Fix misuse of sysfs_emit_at

[Joe Perches]

In commit e28bf1f03b01 ("RDMA: Convert various random sprintf sysfs _show
uses to sysfs_emit") I mistakenly used len = sysfs_emit_at to overwrite
the last trailing space of potentially multiple entry output.

The length of the last sysfs_emit_at call is 1 and it should instead be
ignored.  Do so.

Fixes: e28bf1f03b01 ("RDMA: Convert various random sprintf sysfs _show uses to sysfs_emit")

Reported-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/infiniband/hw/usnic/usnic_ib_sysfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/usnic/usnic_ib_sysfs.c b/drivers/infiniband/hw/usnic/usnic_ib_sysfs.c
index e59615a4c9d9..fc077855b46c 100644
--- a/drivers/infiniband/hw/usnic/usnic_ib_sysfs.c
+++ b/drivers/infiniband/hw/usnic/usnic_ib_sysfs.c
@@ -231,7 +231,7 @@ static ssize_t summary_show(struct usnic_ib_qp_grp *qp_grp, char *buf)
 		}
 	}
 
-	len = sysfs_emit_at(buf, len, "\n");
+	sysfs_emit_at(buf, len, "\n");	/* Overwrite the last trailing space */
 
 	return len;
 }

Re: [PATCH] RDMA: usnic: Fix misuse of sysfs_emit_at

[James Bottomley]

On Fri, 2021-01-15 at 13:23 -0800, Joe Perches wrote:
> In commit e28bf1f03b01 ("RDMA: Convert various random sprintf sysfs
> _show
> uses to sysfs_emit") I mistakenly used len = sysfs_emit_at to
> overwrite
> the last trailing space of potentially multiple entry output.
> 
> The length of the last sysfs_emit_at call is 1 and it should instead
> be
> ignored.  Do so.
> 
> Fixes: e28bf1f03b01 ("RDMA: Convert various random sprintf sysfs
> _show uses to sysfs_emit")
> 
> Reported-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  drivers/infiniband/hw/usnic/usnic_ib_sysfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/hw/usnic/usnic_ib_sysfs.c
> b/drivers/infiniband/hw/usnic/usnic_ib_sysfs.c
> index e59615a4c9d9..fc077855b46c 100644
> --- a/drivers/infiniband/hw/usnic/usnic_ib_sysfs.c
> +++ b/drivers/infiniband/hw/usnic/usnic_ib_sysfs.c
> @@ -231,7 +231,7 @@ static ssize_t summary_show(struct
> usnic_ib_qp_grp *qp_grp, char *buf)
>  		}
>  	}
>  
> -	len = sysfs_emit_at(buf, len, "\n");
> +	sysfs_emit_at(buf, len, "\n");	/* Overwrite the last
> trailing space */

len is the offset of where the next character gets written, isn't it?
so if you're overwriting the last character emitted into buf, shouldn't
the offset point at that character rather than one beyond it?  So

sysfs_emit_at(buf, len - 1, "\n");	/* Overwrite the last trailing
space */

?

James

Re: [PATCH] RDMA: usnic: Fix misuse of sysfs_emit_at

[Joe Perches]

On Fri, 2021-01-15 at 14:15 -0800, James Bottomley wrote:
> On Fri, 2021-01-15 at 13:23 -0800, Joe Perches wrote:
> > In commit e28bf1f03b01 ("RDMA: Convert various random sprintf sysfs
> > _show
> > uses to sysfs_emit") I mistakenly used len = sysfs_emit_at to
> > overwrite
> > the last trailing space of potentially multiple entry output.
> > 
> > The length of the last sysfs_emit_at call is 1 and it should instead
> > be
> > ignored.  Do so.
> > 
> > Fixes: e28bf1f03b01 ("RDMA: Convert various random sprintf sysfs
> > _show uses to sysfs_emit")
> > 
> > Reported-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> > Signed-off-by: Joe Perches <joe@perches.com>
> > ---
> >  drivers/infiniband/hw/usnic/usnic_ib_sysfs.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/infiniband/hw/usnic/usnic_ib_sysfs.c
> > b/drivers/infiniband/hw/usnic/usnic_ib_sysfs.c
> > index e59615a4c9d9..fc077855b46c 100644
> > --- a/drivers/infiniband/hw/usnic/usnic_ib_sysfs.c
> > +++ b/drivers/infiniband/hw/usnic/usnic_ib_sysfs.c
> > @@ -231,7 +231,7 @@ static ssize_t summary_show(struct
> > usnic_ib_qp_grp *qp_grp, char *buf)
> >  		}
> >  	}
> >  
> > 
> > -	len = sysfs_emit_at(buf, len, "\n");
> > +	sysfs_emit_at(buf, len, "\n");	/* Overwrite the last
> > trailing space */
> 
> len is the offset of where the next character gets written, isn't it?
> so if you're overwriting the last character emitted into buf, shouldn't
> the offset point at that character rather than one beyond it?  So
> 
> sysfs_emit_at(buf, len - 1, "\n");	/* Overwrite the last trailing
> space */

<sigh> quite right, thanks for catching yet another braino from me today.

I'll step away from the keyboard and go back to doing other things today
after submitting a V2 with a more typical style without the silly
backspace/newline by trimming the trailing space from the formats...

[PATCH V2] RDMA: usnic: Fix misuse of sysfs_emit_at

[Joe Perches]

In commit e28bf1f03b01 ("RDMA: Convert various random sprintf sysfs _show
uses to sysfs_emit") I mistakenly used len = sysfs_emit_at to overwrite
the last trailing space of potentially multiple entry output.

Instead use a more common style by removing the trailing space from the
output formats and adding a prefixing space to the contination formats and
converting the final terminating output newline from the defective
	len = sysfs_emit_at(buf, len, "\n");
to the now appropriate and typical
	len += sysfs_emit_at(buf, len, "\n");

Fixes: e28bf1f03b01 ("RDMA: Convert various random sprintf sysfs _show uses to sysfs_emit")

Reported-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/infiniband/hw/usnic/usnic_ib_sysfs.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/hw/usnic/usnic_ib_sysfs.c b/drivers/infiniband/hw/usnic/usnic_ib_sysfs.c
index e59615a4c9d9..586b0e52ba7f 100644
--- a/drivers/infiniband/hw/usnic/usnic_ib_sysfs.c
+++ b/drivers/infiniband/hw/usnic/usnic_ib_sysfs.c
@@ -214,7 +214,7 @@ static ssize_t summary_show(struct usnic_ib_qp_grp *qp_grp, char *buf)
 	struct usnic_vnic_res *vnic_res;
 	int len;
 
-	len = sysfs_emit(buf, "QPN: %d State: (%s) PID: %u VF Idx: %hu ",
+	len = sysfs_emit(buf, "QPN: %d State: (%s) PID: %u VF Idx: %hu",
 			 qp_grp->ibqp.qp_num,
 			 usnic_ib_qp_grp_state_to_string(qp_grp->state),
 			 qp_grp->owner_pid,
@@ -224,14 +224,13 @@ static ssize_t summary_show(struct usnic_ib_qp_grp *qp_grp, char *buf)
 		res_chunk = qp_grp->res_chunk_list[i];
 		for (j = 0; j < res_chunk->cnt; j++) {
 			vnic_res = res_chunk->res[j];
-			len += sysfs_emit_at(
-				buf, len, "%s[%d] ",
+			len += sysfs_emit_at(buf, len, " %s[%d]",
 				usnic_vnic_res_type_to_str(vnic_res->type),
 				vnic_res->vnic_idx);
 		}
 	}
 
-	len = sysfs_emit_at(buf, len, "\n");
+	len += sysfs_emit_at(buf, len, "\n");
 
 	return len;
 }

Re: [PATCH V2] RDMA: usnic: Fix misuse of sysfs_emit_at

[Jason Gunthorpe]

On Fri, Jan 15, 2021 at 04:36:50PM -0800, Joe Perches wrote:
> In commit e28bf1f03b01 ("RDMA: Convert various random sprintf sysfs _show
> uses to sysfs_emit") I mistakenly used len = sysfs_emit_at to overwrite
> the last trailing space of potentially multiple entry output.
> 
> Instead use a more common style by removing the trailing space from the
> output formats and adding a prefixing space to the contination formats and
> converting the final terminating output newline from the defective
> 	len = sysfs_emit_at(buf, len, "\n");
> to the now appropriate and typical
> 	len += sysfs_emit_at(buf, len, "\n");
> 
> Fixes: e28bf1f03b01 ("RDMA: Convert various random sprintf sysfs _show uses to sysfs_emit")
> 
> Reported-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  drivers/infiniband/hw/usnic/usnic_ib_sysfs.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)

Applied to for-rc, thanks

Jason