From: "Giedrius Statkevičius" <giedrius.statkevicius@gmail.com>
To: lidza.louina@gmail.com, markh@compro.net
Cc: gregkh@linuxfoundation.org,
driverdev-devel@linuxdriverproject.org,
devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org,
dan.carpenter@oracle.com, sudipm.mukherjee@gmail.com,
"Giedrius Statkevičius" <giedrius.statkevicius@gmail.com>
Subject: [PATCH v3] staging: dgnc: check if kzalloc fails in dgnc_tty_init()
Date: Tue, 7 Apr 2015 17:11:15 +0300 [thread overview]
Message-ID: <1428415875-23797-1-git-send-email-giedrius.statkevicius@gmail.com> (raw)
In-Reply-To: <1428410417-18524-1-git-send-email-giedrius.statkevicius@gmail.com>
If one of the allocations of memory for storing a channel information struct
fails then free all the successful allocations and return -ENOMEM that gets
propogated to the pci layer. Also, remove a bogus skipping in the next part of
the initiation if a previous memory allocation failed because we won't execute
that if any of the allocations failed. Next, remove the misleading comment that
allocation could happen elsewhere. Finally, remove all (except in the ioctl
which can try to get information about a board that failed to probe) checks if
->channels[foo] is NULL or not because probe failing if we can't allocate enough
memory means that this scenario isn't possible.
Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@gmail.com>
---
v3: Remove the wrong comment at dgnc_tty_init() that says the allocation could
happen somewhere else before this and remove the check if (!brd->channels[i]).
Also, remove all checks where ->channels[i] is checked if it's NULL or not
because if probe fails then that means that this scenario isn't possible (except
in the ioctl).
v2: Only returning -ENOMEM if an allocation failed isn't enough as it was
spotted by Sudip so create a new label that frees all successfully allocated
stuff and only then returns -ENOMEM. Also, remove a unnecessary check in the
next loop.
drivers/staging/dgnc/dgnc_cls.c | 4 +---
drivers/staging/dgnc/dgnc_neo.c | 2 +-
drivers/staging/dgnc/dgnc_tty.c | 29 +++++++++++++----------------
3 files changed, 15 insertions(+), 20 deletions(-)
diff --git a/drivers/staging/dgnc/dgnc_cls.c b/drivers/staging/dgnc/dgnc_cls.c
index e3564d2..a629a78 100644
--- a/drivers/staging/dgnc/dgnc_cls.c
+++ b/drivers/staging/dgnc/dgnc_cls.c
@@ -379,7 +379,7 @@ static inline void cls_parse_isr(struct dgnc_board *brd, uint port)
return;
ch = brd->channels[port];
- if (!ch || ch->magic != DGNC_CHANNEL_MAGIC)
+ if (ch->magic != DGNC_CHANNEL_MAGIC)
return;
/* Here we try to figure out what caused the interrupt to happen */
@@ -714,8 +714,6 @@ static void cls_tasklet(unsigned long data)
/* Loop on each port */
for (i = 0; i < ports; i++) {
ch = bd->channels[i];
- if (!ch)
- continue;
/*
* NOTE: Remember you CANNOT hold any channel
diff --git a/drivers/staging/dgnc/dgnc_neo.c b/drivers/staging/dgnc/dgnc_neo.c
index f5a4d36..1e583c2 100644
--- a/drivers/staging/dgnc/dgnc_neo.c
+++ b/drivers/staging/dgnc/dgnc_neo.c
@@ -395,7 +395,7 @@ static inline void neo_parse_isr(struct dgnc_board *brd, uint port)
return;
ch = brd->channels[port];
- if (!ch || ch->magic != DGNC_CHANNEL_MAGIC)
+ if (ch->magic != DGNC_CHANNEL_MAGIC)
return;
/* Here we try to figure out what caused the interrupt to happen */
diff --git a/drivers/staging/dgnc/dgnc_tty.c b/drivers/staging/dgnc/dgnc_tty.c
index ce4187f..d1a2c0f 100644
--- a/drivers/staging/dgnc/dgnc_tty.c
+++ b/drivers/staging/dgnc/dgnc_tty.c
@@ -304,19 +304,15 @@ int dgnc_tty_init(struct dgnc_board *brd)
brd->nasync = brd->maxports;
- /*
- * Allocate channel memory that might not have been allocated
- * when the driver was first loaded.
- */
for (i = 0; i < brd->nasync; i++) {
- if (!brd->channels[i]) {
-
- /*
- * Okay to malloc with GFP_KERNEL, we are not at
- * interrupt context, and there are no locks held.
- */
- brd->channels[i] = kzalloc(sizeof(*brd->channels[i]), GFP_KERNEL);
- }
+ /*
+ * Okay to malloc with GFP_KERNEL, we are not at
+ * interrupt context, and there are no locks held.
+ */
+ brd->channels[i] = kzalloc(sizeof(*brd->channels[i]),
+ GFP_KERNEL);
+ if (!brd->channels[i])
+ goto err_free_channels;
}
ch = brd->channels[0];
@@ -324,10 +320,6 @@ int dgnc_tty_init(struct dgnc_board *brd)
/* Set up channel variables */
for (i = 0; i < brd->nasync; i++, ch = brd->channels[i]) {
-
- if (!brd->channels[i])
- continue;
-
spin_lock_init(&ch->ch_lock);
/* Store all our magic numbers */
@@ -375,6 +367,11 @@ int dgnc_tty_init(struct dgnc_board *brd)
}
return 0;
+
+err_free_channels:
+ for (i = i - 1; i >= 0; --i)
+ kfree(brd->channels[i]);
+ return -ENOMEM;
}
--
2.3.5
next prev parent reply other threads:[~2015-04-07 14:11 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-04-07 10:26 [PATCH] " Giedrius Statkevičius
2015-04-07 10:41 ` Sudip Mukherjee
2015-04-07 11:53 ` Giedrius Statkevičius
2015-04-07 12:40 ` [PATCH v2] " Giedrius Statkevičius
2015-04-07 13:01 ` Dan Carpenter
2015-04-07 13:25 ` Giedrius Statkevičius
2015-04-07 14:11 ` Giedrius Statkevičius [this message]
2015-04-07 14:24 ` [PATCH v3] " Sudip Mukherjee
2015-04-07 14:37 ` Giedrius Statkevičius
2015-04-07 14:54 ` Dan Carpenter
2015-04-07 14:48 ` Dan Carpenter
2015-04-07 15:35 ` Giedrius Statkevičius
2015-04-07 15:46 ` Dan Carpenter
2015-04-07 16:11 ` [PATCH v4 1/3] staging: dgnc: clean up allocation of ->channels[i] Giedrius Statkevičius
2015-04-09 23:42 ` [PATCH v5 " Giedrius Statkevičius
2015-04-09 23:42 ` [PATCH v5 2/3] staging: dgnc: don't forget to check if ->channels[i] is NULL in dgnc_tty_uninit() Giedrius Statkevičius
2015-04-09 23:42 ` [PATCH v5 3/3] staging: dgnc: remove redundant !ch checks Giedrius Statkevičius
2015-04-07 16:11 ` [PATCH v4 2/3] staging: dgnc: don't forget to check if ->channels[i] is NULL in dgnc_tty_uninit() Giedrius Statkevičius
2015-04-07 16:11 ` [PATCH v4 3/3] staging: dgnc: remove redundant !ch checks Giedrius Statkevičius
2015-04-10 0:37 ` Dan Carpenter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1428415875-23797-1-git-send-email-giedrius.statkevicius@gmail.com \
--to=giedrius.statkevicius@gmail.com \
--cc=dan.carpenter@oracle.com \
--cc=devel@driverdev.osuosl.org \
--cc=driverdev-devel@linuxdriverproject.org \
--cc=gregkh@linuxfoundation.org \
--cc=lidza.louina@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=markh@compro.net \
--cc=sudipm.mukherjee@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome