Re: [PATCH v2] cxl: Make loop variable be 'i' instead of 'j'
..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.w...@oracle.com/T/#u
RE: [PATCH v2] cxl: Make loop variable be 'i' instead of 'j'
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 > --- > 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)
[PATCH v2] cxl: Make loop variable be 'i' instead of 'j'
.. 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 --- 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; } -- 2.13.6