From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754104AbbDGQMT (ORCPT ); Tue, 7 Apr 2015 12:12:19 -0400 Received: from mail-wi0-f173.google.com ([209.85.212.173]:35089 "EHLO mail-wi0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752923AbbDGQMP (ORCPT ); Tue, 7 Apr 2015 12:12:15 -0400 From: =?UTF-8?q?Giedrius=20Statkevi=C4=8Dius?= 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, =?UTF-8?q?Giedrius=20Statkevi=C4=8Dius?= Subject: [PATCH v4 1/3] staging: dgnc: clean up allocation of ->channels[i] Date: Tue, 7 Apr 2015 19:11:40 +0300 Message-Id: <1428423102-29138-1-git-send-email-giedrius.statkevicius@gmail.com> X-Mailer: git-send-email 2.3.5 In-Reply-To: <1428415875-23797-1-git-send-email-giedrius.statkevicius@gmail.com> References: <1428415875-23797-1-git-send-email-giedrius.statkevicius@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Check if kzalloc fails in dgnc_tty_init() and if it does then free all previously allocated ->channels[i] and set them to NULL. This makes the code less error/bug prone because instead of needing programmers attention to add checks everywhere we do that in one place. Also, remove a bogus comment and check in the same loop because ->channels[i] isn't allocated anywhere else. Finally, remove a unnecessary check if ->channels[i] is NULL in the next loop because it can't be. Signed-off-by: Giedrius Statkevičius --- v4: Make this patch only for dgnc_tty.c and only for this thing. Split into 3 patches (1 patch fixes a bug reported by Dan Carpenter) 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_tty.c | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/drivers/staging/dgnc/dgnc_tty.c b/drivers/staging/dgnc/dgnc_tty.c index ce4187f..0e48f95 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,13 @@ int dgnc_tty_init(struct dgnc_board *brd) } return 0; + +err_free_channels: + for (i = i - 1; i >= 0; --i) { + kfree(brd->channels[i]); + brd->channels[i] = NULL; + } + return -ENOMEM; } -- 2.3.5 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from fraxinus.osuosl.org (fraxinus.osuosl.org [140.211.166.137]) by ash.osuosl.org (Postfix) with ESMTP id 3E1571BF978 for ; Tue, 7 Apr 2015 16:12:16 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 39608A200A for ; Tue, 7 Apr 2015 16:12:16 +0000 (UTC) Received: from fraxinus.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id BVcTr58qzu_H for ; Tue, 7 Apr 2015 16:12:15 +0000 (UTC) Received: from mail-wg0-f52.google.com (mail-wg0-f52.google.com [74.125.82.52]) by fraxinus.osuosl.org (Postfix) with ESMTPS id 724A5A1E8E for ; Tue, 7 Apr 2015 16:12:15 +0000 (UTC) Received: by wgbdm7 with SMTP id dm7so61401250wgb.1 for ; Tue, 07 Apr 2015 09:12:14 -0700 (PDT) From: =?UTF-8?q?Giedrius=20Statkevi=C4=8Dius?= Subject: [PATCH v4 1/3] staging: dgnc: clean up allocation of ->channels[i] Date: Tue, 7 Apr 2015 19:11:40 +0300 Message-Id: <1428423102-29138-1-git-send-email-giedrius.statkevicius@gmail.com> In-Reply-To: <1428415875-23797-1-git-send-email-giedrius.statkevicius@gmail.com> References: <1428415875-23797-1-git-send-email-giedrius.statkevicius@gmail.com> MIME-Version: 1.0 List-Id: Linux Driver Project Developer List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Errors-To: driverdev-devel-bounces@linuxdriverproject.org Sender: "devel" To: lidza.louina@gmail.com, markh@compro.net Cc: devel@driverdev.osuosl.org, gregkh@linuxfoundation.org, driverdev-devel@linuxdriverproject.org, linux-kernel@vger.kernel.org, sudipm.mukherjee@gmail.com, dan.carpenter@oracle.com Q2hlY2sgaWYga3phbGxvYyBmYWlscyBpbiBkZ25jX3R0eV9pbml0KCkgYW5kIGlmIGl0IGRvZXMg dGhlbiBmcmVlIGFsbApwcmV2aW91c2x5IGFsbG9jYXRlZCAtPmNoYW5uZWxzW2ldIGFuZCBzZXQg dGhlbSB0byBOVUxMLiBUaGlzIG1ha2VzIHRoZSBjb2RlCmxlc3MgZXJyb3IvYnVnIHByb25lIGJl Y2F1c2UgaW5zdGVhZCBvZiBuZWVkaW5nIHByb2dyYW1tZXJzIGF0dGVudGlvbiB0byBhZGQKY2hl Y2tzIGV2ZXJ5d2hlcmUgd2UgZG8gdGhhdCBpbiBvbmUgcGxhY2UuIEFsc28sIHJlbW92ZSBhIGJv Z3VzIGNvbW1lbnQgYW5kCmNoZWNrIGluIHRoZSBzYW1lIGxvb3AgYmVjYXVzZSAtPmNoYW5uZWxz W2ldIGlzbid0IGFsbG9jYXRlZCBhbnl3aGVyZSBlbHNlLgpGaW5hbGx5LCByZW1vdmUgYSB1bm5l Y2Vzc2FyeSBjaGVjayBpZiAtPmNoYW5uZWxzW2ldIGlzIE5VTEwgaW4gdGhlIG5leHQgbG9vcApi ZWNhdXNlIGl0IGNhbid0IGJlLgoKU2lnbmVkLW9mZi1ieTogR2llZHJpdXMgU3RhdGtldmnEjWl1 cyA8Z2llZHJpdXMuc3RhdGtldmljaXVzQGdtYWlsLmNvbT4KLS0tCnY0OiBNYWtlIHRoaXMgcGF0 Y2ggb25seSBmb3IgZGduY190dHkuYyBhbmQgb25seSBmb3IgdGhpcyB0aGluZy4gU3BsaXQgaW50 byAzCnBhdGNoZXMgKDEgcGF0Y2ggZml4ZXMgYSBidWcgcmVwb3J0ZWQgYnkgRGFuIENhcnBlbnRl cikKCnYzOiBSZW1vdmUgdGhlIHdyb25nIGNvbW1lbnQgYXQgZGduY190dHlfaW5pdCgpIHRoYXQg c2F5cyB0aGUgYWxsb2NhdGlvbiBjb3VsZApoYXBwZW4gc29tZXdoZXJlIGVsc2UgYmVmb3JlIHRo aXMgYW5kIHJlbW92ZSB0aGUgY2hlY2sgaWYgKCFicmQtPmNoYW5uZWxzW2ldKS4KQWxzbywgcmVt b3ZlIGFsbCBjaGVja3Mgd2hlcmUgLT5jaGFubmVsc1tpXSBpcyBjaGVja2VkIGlmIGl0J3MgTlVM TCBvciBub3QKYmVjYXVzZSBpZiBwcm9iZSBmYWlscyB0aGVuIHRoYXQgbWVhbnMgdGhhdCB0aGlz IHNjZW5hcmlvIGlzbid0IHBvc3NpYmxlIChleGNlcHQKaW4gdGhlIGlvY3RsKS4KCnYyOiBPbmx5 IHJldHVybmluZyAtRU5PTUVNIGlmIGFuIGFsbG9jYXRpb24gZmFpbGVkIGlzbid0IGVub3VnaCBh cyBpdCB3YXMKc3BvdHRlZCBieSBTdWRpcCBzbyBjcmVhdGUgYSBuZXcgbGFiZWwgdGhhdCBmcmVl cyBhbGwgc3VjY2Vzc2Z1bGx5IGFsbG9jYXRlZApzdHVmZiBhbmQgb25seSB0aGVuIHJldHVybnMg LUVOT01FTS4gQWxzbywgcmVtb3ZlIGEgdW5uZWNlc3NhcnkgY2hlY2sgaW4gdGhlCm5leHQgbG9v cC4KCiBkcml2ZXJzL3N0YWdpbmcvZGduYy9kZ25jX3R0eS5jIHwgMzEgKysrKysrKysrKysrKysr LS0tLS0tLS0tLS0tLS0tLQogMSBmaWxlIGNoYW5nZWQsIDE1IGluc2VydGlvbnMoKyksIDE2IGRl bGV0aW9ucygtKQoKZGlmZiAtLWdpdCBhL2RyaXZlcnMvc3RhZ2luZy9kZ25jL2RnbmNfdHR5LmMg Yi9kcml2ZXJzL3N0YWdpbmcvZGduYy9kZ25jX3R0eS5jCmluZGV4IGNlNDE4N2YuLjBlNDhmOTUg MTAwNjQ0Ci0tLSBhL2RyaXZlcnMvc3RhZ2luZy9kZ25jL2RnbmNfdHR5LmMKKysrIGIvZHJpdmVy cy9zdGFnaW5nL2RnbmMvZGduY190dHkuYwpAQCAtMzA0LDE5ICszMDQsMTUgQEAgaW50IGRnbmNf dHR5X2luaXQoc3RydWN0IGRnbmNfYm9hcmQgKmJyZCkKIAogCWJyZC0+bmFzeW5jID0gYnJkLT5t YXhwb3J0czsKIAotCS8qCi0JICogQWxsb2NhdGUgY2hhbm5lbCBtZW1vcnkgdGhhdCBtaWdodCBu b3QgaGF2ZSBiZWVuIGFsbG9jYXRlZAotCSAqIHdoZW4gdGhlIGRyaXZlciB3YXMgZmlyc3QgbG9h ZGVkLgotCSAqLwogCWZvciAoaSA9IDA7IGkgPCBicmQtPm5hc3luYzsgaSsrKSB7Ci0JCWlmICgh YnJkLT5jaGFubmVsc1tpXSkgewotCi0JCQkvKgotCQkJICogT2theSB0byBtYWxsb2Mgd2l0aCBH RlBfS0VSTkVMLCB3ZSBhcmUgbm90IGF0Ci0JCQkgKiBpbnRlcnJ1cHQgY29udGV4dCwgYW5kIHRo ZXJlIGFyZSBubyBsb2NrcyBoZWxkLgotCQkJICovCi0JCQlicmQtPmNoYW5uZWxzW2ldID0ga3ph bGxvYyhzaXplb2YoKmJyZC0+Y2hhbm5lbHNbaV0pLCBHRlBfS0VSTkVMKTsKLQkJfQorCQkvKgor CQkgKiBPa2F5IHRvIG1hbGxvYyB3aXRoIEdGUF9LRVJORUwsIHdlIGFyZSBub3QgYXQKKwkJICog aW50ZXJydXB0IGNvbnRleHQsIGFuZCB0aGVyZSBhcmUgbm8gbG9ja3MgaGVsZC4KKwkJICovCisJ CWJyZC0+Y2hhbm5lbHNbaV0gPSBremFsbG9jKHNpemVvZigqYnJkLT5jaGFubmVsc1tpXSksCisJ CQkJCSAgIEdGUF9LRVJORUwpOworCQlpZiAoIWJyZC0+Y2hhbm5lbHNbaV0pCisJCQlnb3RvIGVy cl9mcmVlX2NoYW5uZWxzOwogCX0KIAogCWNoID0gYnJkLT5jaGFubmVsc1swXTsKQEAgLTMyNCwx MCArMzIwLDYgQEAgaW50IGRnbmNfdHR5X2luaXQoc3RydWN0IGRnbmNfYm9hcmQgKmJyZCkKIAog CS8qIFNldCB1cCBjaGFubmVsIHZhcmlhYmxlcyAqLwogCWZvciAoaSA9IDA7IGkgPCBicmQtPm5h c3luYzsgaSsrLCBjaCA9IGJyZC0+Y2hhbm5lbHNbaV0pIHsKLQotCQlpZiAoIWJyZC0+Y2hhbm5l bHNbaV0pCi0JCQljb250aW51ZTsKLQogCQlzcGluX2xvY2tfaW5pdCgmY2gtPmNoX2xvY2spOwog CiAJCS8qIFN0b3JlIGFsbCBvdXIgbWFnaWMgbnVtYmVycyAqLwpAQCAtMzc1LDYgKzM2NywxMyBA QCBpbnQgZGduY190dHlfaW5pdChzdHJ1Y3QgZGduY19ib2FyZCAqYnJkKQogCX0KIAogCXJldHVy biAwOworCitlcnJfZnJlZV9jaGFubmVsczoKKwlmb3IgKGkgPSBpIC0gMTsgaSA+PSAwOyAtLWkp IHsKKwkJa2ZyZWUoYnJkLT5jaGFubmVsc1tpXSk7CisJCWJyZC0+Y2hhbm5lbHNbaV0gPSBOVUxM OworCX0KKwlyZXR1cm4gLUVOT01FTTsKIH0KIAogCi0tIAoyLjMuNQoKX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZGV2ZWwgbWFpbGluZyBsaXN0CmRldmVs QGxpbnV4ZHJpdmVycHJvamVjdC5vcmcKaHR0cDovL2RyaXZlcmRldi5saW51eGRyaXZlcnByb2pl Y3Qub3JnL21haWxtYW4vbGlzdGluZm8vZHJpdmVyZGV2LWRldmVsCg==