linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Linus Walleij <linus.walleij@linaro.org>
To: David Woodhouse <dwmw2@infradead.org>,
	Brian Norris <computersforpeace@gmail.com>,
	Marek Vasut <marek.vasut@gmail.com>,
	Richard Weinberger <richard@nod.at>,
	Miquel Raynal <miquel.raynal@bootlin.com>,
	linux-mtd@lists.infradead.org
Cc: Linus Walleij <linus.walleij@linaro.org>,
	Liviu Dudau <liviu.dudau@arm.com>,
	Ryan Harkin <ryan.harkin@linaro.org>
Subject: [PATCH 4/9 RESEND 2] mtd: afs: simplify partition parsing
Date: Thu,  2 May 2019 16:30:29 +0200	[thread overview]
Message-ID: <20190502143034.16781-5-linus.walleij@linaro.org> (raw)
In-Reply-To: <20190502143034.16781-1-linus.walleij@linaro.org>

This simplifies the AFS partition parsing to make the code
more straight-forward and readable.

Before this patch the code tried to calculate the memory required
to hold the partition info by adding up the sizes of the strings
of the names and adding that to a single memory allocation,
indexing the name pointers in front of the struct mtd_partition
allocations so all allocated data was in one chunk.

This is overzealous. Instead use kstrdup and bail out,
kfree():ing the memory used for MTD partitions and names alike
on the errorpath.

In the process rename the index variable from idx to i.

Cc: Ryan Harkin <ryan.harkin@linaro.org>
Acked-by: Liviu Dudau <liviu.dudau@arm.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/mtd/parsers/afs.c | 67 +++++++++++++++++++--------------------
 1 file changed, 32 insertions(+), 35 deletions(-)

diff --git a/drivers/mtd/parsers/afs.c b/drivers/mtd/parsers/afs.c
index 3679e1d22595..c489938cd665 100644
--- a/drivers/mtd/parsers/afs.c
+++ b/drivers/mtd/parsers/afs.c
@@ -166,9 +166,9 @@ static int parse_afs_partitions(struct mtd_info *mtd,
 				struct mtd_part_parser_data *data)
 {
 	struct mtd_partition *parts;
-	u_int mask, off, idx, sz;
+	u_int mask, off, sz;
 	int ret = 0;
-	char *str;
+	int i;
 
 	/*
 	 * This is the address mask; we use this to mask off out of
@@ -181,78 +181,75 @@ static int parse_afs_partitions(struct mtd_info *mtd,
 	 * partition information.  We include in this the size of
 	 * the strings.
 	 */
-	for (idx = off = sz = 0; off < mtd->size; off += mtd->erasesize) {
-		struct image_info_v1 iis;
+	for (i = off = sz = 0; off < mtd->size; off += mtd->erasesize) {
 		u_int iis_ptr, img_ptr;
 
 		ret = afs_read_footer_v1(mtd, &img_ptr, &iis_ptr, off, mask);
 		if (ret < 0)
-			break;
+			return ret;
 		if (ret) {
-			ret = afs_read_iis_v1(mtd, &iis, iis_ptr);
-			if (ret < 0)
-				break;
-			if (ret == 0)
-				continue;
-
 			sz += sizeof(struct mtd_partition);
-			sz += strlen(iis.name) + 1;
-			idx += 1;
+			i += 1;
 		}
 	}
 
-	if (!sz)
-		return ret;
+	if (!i)
+		return 0;
 
 	parts = kzalloc(sz, GFP_KERNEL);
 	if (!parts)
 		return -ENOMEM;
 
-	str = (char *)(parts + idx);
-
 	/*
 	 * Identify the partitions
 	 */
-	for (idx = off = 0; off < mtd->size; off += mtd->erasesize) {
+	for (i = off = 0; off < mtd->size; off += mtd->erasesize) {
 		struct image_info_v1 iis;
 		u_int iis_ptr, img_ptr;
 
 		/* Read the footer. */
 		ret = afs_read_footer_v1(mtd, &img_ptr, &iis_ptr, off, mask);
 		if (ret < 0)
-			break;
+			goto out_free_parts;
 		if (ret == 0)
 			continue;
 
 		/* Read the image info block */
 		ret = afs_read_iis_v1(mtd, &iis, iis_ptr);
 		if (ret < 0)
-			break;
+			goto out_free_parts;
 		if (ret == 0)
 			continue;
 
-		strcpy(str, iis.name);
+		parts[i].name = kstrdup(iis.name, GFP_KERNEL);
+		if (!parts[i].name) {
+			ret = -ENOMEM;
+			goto out_free_parts;
+		}
 
-		parts[idx].name		= str;
-		parts[idx].size		= (iis.length + mtd->erasesize - 1) & ~(mtd->erasesize - 1);
-		parts[idx].offset	= img_ptr;
-		parts[idx].mask_flags	= 0;
+		parts[i].size		= (iis.length + mtd->erasesize - 1) & ~(mtd->erasesize - 1);
+		parts[i].offset	= img_ptr;
+		parts[i].mask_flags	= 0;
 
 		printk("  mtd%d: at 0x%08x, %5lluKiB, %8u, %s\n",
-			idx, img_ptr, parts[idx].size / 1024,
-			iis.imageNumber, str);
-
-		idx += 1;
-		str = str + strlen(iis.name) + 1;
-	}
+			i, img_ptr, parts[i].size / 1024,
+			iis.imageNumber, parts[i].name);
 
-	if (!idx) {
-		kfree(parts);
-		parts = NULL;
+		i += 1;
 	}
 
 	*pparts = parts;
-	return idx ? idx : ret;
+	return i;
+
+out_free_parts:
+	while (i >= 0) {
+		if (parts[i].name)
+			kfree(parts[i].name);
+		i--;
+	}
+	kfree(parts);
+	*pparts = NULL;
+	return ret;
 }
 
 static const struct of_device_id mtd_parser_afs_of_match_table[] = {
-- 
2.20.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

  parent reply	other threads:[~2019-05-02 14:33 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-02 14:30 [PATCH 0/9 RESEND 2] AFS patches resend 2 Linus Walleij
2019-05-02 14:30 ` [PATCH 1/9 RESEND 2] mtd: afs: Move AFS partition parser to parsers subdir Linus Walleij
2019-05-02 14:30 ` [PATCH 2/9 RESEND 2] mtd: partitions: Add AFS partitions DT bindings Linus Walleij
2019-05-02 14:30 ` [PATCH 3/9 RESEND 2] mtd: partitions: Add OF support to AFS partitions Linus Walleij
2019-05-02 14:30 ` Linus Walleij [this message]
2019-05-02 14:30 ` [PATCH 5/9 RESEND 2] mtd: afs: simplify partition detection Linus Walleij
2019-05-02 14:30 ` [PATCH 6/9 RESEND 2] mtd: factor out v1 partition parsing Linus Walleij
2019-05-02 14:30 ` [PATCH 7/9 RESEND 2] mtd: afs: factor footer parsing into the v1 part parsing Linus Walleij
2019-05-02 14:30 ` [PATCH 8/9 RESEND 2] mtd: afs: factor the IIS read into partition parser Linus Walleij
2019-05-02 14:30 ` [PATCH 9/9 RESEND 2] mtd: afs: add v2 partition parsing Linus Walleij

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=20190502143034.16781-5-linus.walleij@linaro.org \
    --to=linus.walleij@linaro.org \
    --cc=computersforpeace@gmail.com \
    --cc=dwmw2@infradead.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=liviu.dudau@arm.com \
    --cc=marek.vasut@gmail.com \
    --cc=miquel.raynal@bootlin.com \
    --cc=richard@nod.at \
    --cc=ryan.harkin@linaro.org \
    /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;
as well as URLs for NNTP newsgroup(s).