Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1466075 > unrolled thread
| Started by | SF Markus Elfring <elfring@users.sourceforge.net> |
|---|---|
| First post | 2016-08-19 09:10 +0200 |
| Last post | 2016-08-19 17:40 +0200 |
| Articles | 7 — 4 participants |
Back to article view | Back to linux.kernel
[PATCH] IB/qib: Use memdup_user() rather than duplicating its implementation SF Markus Elfring <elfring@users.sourceforge.net> - 2016-08-19 09:10 +0200
RE: [PATCH] IB/qib: Use memdup_user() rather than duplicating its implementation "Marciniszyn, Mike" <mike.marciniszyn@intel.com> - 2016-08-19 17:40 +0200
RE: [PATCH] IB/qib: Use memdup_user() rather than duplicating its implementation "Marciniszyn, Mike" <mike.marciniszyn@intel.com> - 2016-08-19 17:50 +0200
Re: [PATCH] IB/qib: Use memdup_user() rather than duplicating its implementation Leon Romanovsky <leon@kernel.org> - 2016-08-19 17:50 +0200
Re: [PATCH] IB/qib: Use memdup_user() rather than duplicating its implementation Leon Romanovsky <leon@kernel.org> - 2016-08-19 18:00 +0200
Re: [PATCH] IB/qib: Use memdup_user() rather than duplicating its implementation Doug Ledford <dledford@redhat.com> - 2016-08-23 18:50 +0200
Re: [PATCH] IB/qib: Use memdup_user() rather than duplicating its implementation Leon Romanovsky <leon@kernel.org> - 2016-08-19 17:40 +0200
| From | SF Markus Elfring <elfring@users.sourceforge.net> |
|---|---|
| Date | 2016-08-19 09:10 +0200 |
| Subject | [PATCH] IB/qib: Use memdup_user() rather than duplicating its implementation |
| Message-ID | <s7M6e-2to-11@gated-at.bofh.it> |
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 19 Aug 2016 08:50:23 +0200
Reuse existing functionality from memdup_user() instead of keeping
duplicate source code.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/infiniband/hw/qib/qib_fs.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/infiniband/hw/qib/qib_fs.c b/drivers/infiniband/hw/qib/qib_fs.c
index fcdf3791..910f0d9 100644
--- a/drivers/infiniband/hw/qib/qib_fs.c
+++ b/drivers/infiniband/hw/qib/qib_fs.c
@@ -338,17 +338,12 @@ static ssize_t flash_write(struct file *file, const char __user *buf,
goto bail;
}
- tmp = kmalloc(count, GFP_KERNEL);
- if (!tmp) {
- ret = -ENOMEM;
+ tmp = memdup_user(buf, count);
+ if (IS_ERR(tmp)) {
+ ret = PTR_ERR(tmp);
goto bail;
}
- if (copy_from_user(tmp, buf, count)) {
- ret = -EFAULT;
- goto bail_tmp;
- }
-
dd = private2dd(file);
if (qib_eeprom_write(dd, pos, tmp, count)) {
ret = -ENXIO;
--
2.9.3
[toc] | [next] | [standalone]
| From | "Marciniszyn, Mike" <mike.marciniszyn@intel.com> |
|---|---|
| Date | 2016-08-19 17:40 +0200 |
| Message-ID | <s7U3L-7oh-1@gated-at.bofh.it> |
| In reply to | #1466075 |
> Subject: [PATCH] IB/qib: Use memdup_user() rather than duplicating its
> diff --git a/drivers/infiniband/hw/qib/qib_fs.c
I would be even more aggressive at reducing lines of code.
For example do direct returns when ok to do:
if (pos != 0 || count != sizeof(struct qib_flash))
return -EINVAL;
tmp = memdup_user(buf, count);
if (IS_ERR(tmp))
return PTR_ERR(tmp);
The bail_tmp: label is then not needed.
Mike
[toc] | [prev] | [next] | [standalone]
| From | "Marciniszyn, Mike" <mike.marciniszyn@intel.com> |
|---|---|
| Date | 2016-08-19 17:50 +0200 |
| Message-ID | <s7Uds-7rO-17@gated-at.bofh.it> |
| In reply to | #1466496 |
> > > > The bail_tmp: label is then not needed. > > You still need to free tmp allocation if qib_eeprom_write failed and this is > your bail_tmp. > Typo. The bail: label is not needed. Mike
[toc] | [prev] | [next] | [standalone]
| From | Leon Romanovsky <leon@kernel.org> |
|---|---|
| Date | 2016-08-19 17:50 +0200 |
| Message-ID | <s7Udt-7rO-41@gated-at.bofh.it> |
| In reply to | #1466510 |
[Multipart message — attachments visible in raw view] — view raw
On Fri, Aug 19, 2016 at 03:42:29PM +0000, Marciniszyn, Mike wrote: > > > > > > The bail_tmp: label is then not needed. > > > > You still need to free tmp allocation if qib_eeprom_write failed and this is > > your bail_tmp. > > > > Typo. The bail: label is not needed. Yeah, it makes sense. Thanks > > Mike
[toc] | [prev] | [next] | [standalone]
| From | Leon Romanovsky <leon@kernel.org> |
|---|---|
| Date | 2016-08-19 18:00 +0200 |
| Message-ID | <s7Uds-7rO-19@gated-at.bofh.it> |
| In reply to | #1466496 |
[Multipart message — attachments visible in raw view] — view raw
On Fri, Aug 19, 2016 at 03:27:20PM +0000, Marciniszyn, Mike wrote:
> > Subject: [PATCH] IB/qib: Use memdup_user() rather than duplicating its
> > diff --git a/drivers/infiniband/hw/qib/qib_fs.c
>
> I would be even more aggressive at reducing lines of code.
>
> For example do direct returns when ok to do:
> if (pos != 0 || count != sizeof(struct qib_flash))
> return -EINVAL;
>
> tmp = memdup_user(buf, count);
> if (IS_ERR(tmp))
> return PTR_ERR(tmp);
>
> The bail_tmp: label is then not needed.
You still need to free tmp allocation if qib_eeprom_write failed and
this is your bail_tmp.
341 tmp = kmalloc(count, GFP_KERNEL);
342 if (!tmp) {
343 ret = -ENOMEM;
344 goto bail;
345 }
346
347 if (copy_from_user(tmp, buf, count)) {
348 ret = -EFAULT;
349 goto bail_tmp;
350 }
351
352 dd = private2dd(file);
353 if (qib_eeprom_write(dd, pos, tmp, count)) {
354 ret = -ENXIO;
355 qib_dev_err(dd, "failed to write to flash\n");
356 goto bail_tmp;
357 }
>
> Mike
>
[toc] | [prev] | [next] | [standalone]
| From | Doug Ledford <dledford@redhat.com> |
|---|---|
| Date | 2016-08-23 18:50 +0200 |
| Message-ID | <s9n3I-6xg-15@gated-at.bofh.it> |
| In reply to | #1466496 |
[Multipart message — attachments visible in raw view] — view raw
On 8/19/2016 11:27 AM, Marciniszyn, Mike wrote:
>> Subject: [PATCH] IB/qib: Use memdup_user() rather than duplicating its
>> diff --git a/drivers/infiniband/hw/qib/qib_fs.c
>
> I would be even more aggressive at reducing lines of code.
>
> For example do direct returns when ok to do:
> if (pos != 0 || count != sizeof(struct qib_flash))
> return -EINVAL;
>
> tmp = memdup_user(buf, count);
> if (IS_ERR(tmp))
> return PTR_ERR(tmp);
>
> The bail_tmp: label is then not needed.
>
> Mike
>
With Mike's additional cleanups in place, patch applied.
--
Doug Ledford <dledford@redhat.com>
GPG Key ID: 0E572FDD
[toc] | [prev] | [next] | [standalone]
| From | Leon Romanovsky <leon@kernel.org> |
|---|---|
| Date | 2016-08-19 17:40 +0200 |
| Message-ID | <s7U3L-7oh-3@gated-at.bofh.it> |
| In reply to | #1466075 |
[Multipart message — attachments visible in raw view] — view raw
On Fri, Aug 19, 2016 at 09:06:20AM +0200, SF Markus Elfring wrote: > From: Markus Elfring <elfring@users.sourceforge.net> > Date: Fri, 19 Aug 2016 08:50:23 +0200 > > Reuse existing functionality from memdup_user() instead of keeping > duplicate source code. > > This issue was detected by using the Coccinelle software. > > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web