| Message ID | 20210226211656.46359-1-konrad.wilk@oracle.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] cxl: Make loop variable be 'i' instead of 'j' | expand |
From: Konrad Rzeszutek Wilk > Sent: 26 February 2021 21:17 > > .. otherwise people spend extra cycles looking for the > inner loop and wondering 'why j'? > > This was an oversight when initial work was rebased > so let's fix it here. > > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > --- > v1: Initial posting > v2: Fix per Dan's request > --- > drivers/cxl/mem.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c > index 244cb7d89678..d43197a193ce 100644 > --- a/drivers/cxl/mem.c > +++ b/drivers/cxl/mem.c > @@ -698,7 +698,6 @@ static int cxl_query_cmd(struct cxl_memdev *cxlmd, > struct device *dev = &cxlmd->dev; > struct cxl_mem_command *cmd; > u32 n_commands; > - int j = 0; > > dev_dbg(dev, "Query IOCTL\n"); > > @@ -715,11 +714,12 @@ static int cxl_query_cmd(struct cxl_memdev *cxlmd, > */ > cxl_for_each_cmd(cmd) { > const struct cxl_command_info *info = &cmd->info; > + int i = 0; > > - if (copy_to_user(&q->commands[j++], info, sizeof(*info))) > + if (copy_to_user(&q->commands[i++], info, sizeof(*info))) > return -EFAULT; > > - if (j == n_commands) > + if (i == n_commands) > break; Did you test this? Looks badly broken to me. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
..snip.. > > cxl_for_each_cmd(cmd) { > > const struct cxl_command_info *info = &cmd->info; > > + int i = 0; > > > > - if (copy_to_user(&q->commands[j++], info, sizeof(*info))) > > + if (copy_to_user(&q->commands[i++], info, sizeof(*info))) > > return -EFAULT; > > > > - if (j == n_commands) > > + if (i == n_commands) > > break; > > > Did you test this? > Looks badly broken to me. I sent out the v3 which had that fixed. See https://lore.kernel.org/linux-cxl/20210226222152.48467-1-konrad.wilk@oracle.com/T/#u
diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c index 244cb7d89678..d43197a193ce 100644 --- a/drivers/cxl/mem.c +++ b/drivers/cxl/mem.c @@ -698,7 +698,6 @@ static int cxl_query_cmd(struct cxl_memdev *cxlmd, struct device *dev = &cxlmd->dev; struct cxl_mem_command *cmd; u32 n_commands; - int j = 0; dev_dbg(dev, "Query IOCTL\n"); @@ -715,11 +714,12 @@ static int cxl_query_cmd(struct cxl_memdev *cxlmd, */ cxl_for_each_cmd(cmd) { const struct cxl_command_info *info = &cmd->info; + int i = 0; - if (copy_to_user(&q->commands[j++], info, sizeof(*info))) + if (copy_to_user(&q->commands[i++], info, sizeof(*info))) return -EFAULT; - if (j == n_commands) + if (i == n_commands) break; }
.. otherwise people spend extra cycles looking for the inner loop and wondering 'why j'? This was an oversight when initial work was rebased so let's fix it here. Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> --- v1: Initial posting v2: Fix per Dan's request --- drivers/cxl/mem.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)