Le 2016-12-20 12:24, Dan Carpenter a écrit :
This patch doesn't apply. Read Documentation/process/email-clients.rst
On Tue, Dec 20, 2016 at 11:49:41AM +0100, Francis Laniel wrote:
Hello.
As asked in the TODO file for this driver I added some goto statements
to
handle errors.
I used Linus Torvalds tree, I compiled it and tested it with a virtual
machine, here is the proof :
[ 42.394265] dgnc: module is from the staging directory, ...
[root@vm-nmv ~]# uname -r
4.9.0-11815-ge93b1cc-dirty
It is my first patch so I hope I did not break anything.
Good bye and thank you.
Don't put this stuff in the changelog text.
Why do I not put this stuff in the changelog ? Is it because it does not
give
information about the changes ?
Signed-off-by: Francis Laniel <laniel_francis@xxxxxxxxxxxxx>
---
drivers/staging/dgnc/dgnc_mgmt.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/dgnc/dgnc_mgmt.c b/drivers/staging/dgnc/
dgnc_mgmt.c
index 9d9b15d..6e41010 100644
--- a/drivers/staging/dgnc/dgnc_mgmt.c
+++ b/drivers/staging/dgnc/dgnc_mgmt.c
@@ -40,27 +40,34 @@ static int dgnc_mgmt_in_use[MAXMGMTDEVICES];
*/
int dgnc_mgmt_open(struct inode *inode, struct file *file)
{
+ int rc;
+
No blank line.
unsigned long flags;
unsigned int minor = iminor(inode);
spin_lock_irqsave(&dgnc_global_lock, flags);
+ rc = 0;
Just do that at the start.
+
/* mgmt device */
if (minor < MAXMGMTDEVICES) {
/* Only allow 1 open at a time on mgmt device */
if (dgnc_mgmt_in_use[minor]) {
- spin_unlock_irqrestore(&dgnc_global_lock,
flags);
- return -EBUSY;
+ rc = -EBUSY;
+
Don't add the extra blank line.
+ goto end;
}
dgnc_mgmt_in_use[minor]++;
} else {
- spin_unlock_irqrestore(&dgnc_global_lock, flags);
- return -ENXIO;
+ rc = -ENXIO;
+
+ goto end;
}
+end:
unlock: is a better name.
spin_unlock_irqrestore(&dgnc_global_lock, flags);
- return 0;
+ return rc;
}
I update my patch and send it as a v2.
/*
@@ -110,6 +117,8 @@ long dgnc_mgmt_ioctl(struct file *file, unsigned
int cmd,
unsigned long arg)
memset(&ddi, 0, sizeof(ddi));
ddi.dinfo_nboards = dgnc_num_boards;
+
+ /* Is it possible to use snprintf ? */
sprintf(ddi.dinfo_version, "%s", DG_PART);
This is not related, but yeah. You could use snprintf() if you want.
It's not super important because we know that the original code is
fine.
Ok I understand.
regards,
dan carpenter
Thank you for your advices.
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
