Patchwork [2/3] mtd: cmdlinepart: sort the unsorted partitions

login
register
mail settings
Submitter Huang Shijie
Date Aug. 26, 2012, 5:21 p.m.
Message ID <1346001700-26895-2-git-send-email-shijie8@gmail.com>
Download mbox | patch
Permalink /patch/180025/
State New
Headers show

Comments

Huang Shijie - Aug. 26, 2012, 5:21 p.m.
Assume we have a 1GB(8Gb) nand chip.
It is legit if we set the partitions as the following:
    gpmi-nand:1g@200m(rootfs),100m@0(boot),100m@100m(kernel)

But the current code can not parse out any partition with this
cmdline.

This patch sorts the unsorted partitions by the @offset.
For there are maybe only several partitions, i use the simple
Bubble sort algorithm.

Signed-off-by: Huang Shijie <shijie8@gmail.com>
---
 drivers/mtd/cmdlinepart.c |   29 +++++++++++++++++++++++++++++
 1 files changed, 29 insertions(+), 0 deletions(-)
Artem Bityutskiy - Aug. 31, 2012, 1:59 p.m.
On Sun, 2012-08-26 at 13:21 -0400, Huang Shijie wrote:
> Assume we have a 1GB(8Gb) nand chip.
> It is legit if we set the partitions as the following:
>     gpmi-nand:1g@200m(rootfs),100m@0(boot),100m@100m(kernel)
> 
> But the current code can not parse out any partition with this
> cmdline.
> 
> This patch sorts the unsorted partitions by the @offset.
> For there are maybe only several partitions, i use the simple
> Bubble sort algorithm.
> 
> Signed-off-by: Huang Shijie <shijie8@gmail.com>

I still cannot find time to actually think about this carefully, but the
commit message does not sound convincing, it does not explain why
sorting is the right way to fix the issue, and what would be the
alternatives. It actually also does not explain why exactly we currently
cannot parse the example string.
Huang Shijie - Aug. 31, 2012, 2:29 p.m.
On Fri, Aug 31, 2012 at 9:59 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> On Sun, 2012-08-26 at 13:21 -0400, Huang Shijie wrote:
>> Assume we have a 1GB(8Gb) nand chip.
>> It is legit if we set the partitions as the following:
>>     gpmi-nand:1g@200m(rootfs),100m@0(boot),100m@100m(kernel)
>>
>> But the current code can not parse out any partition with this
>> cmdline.
>>
>> This patch sorts the unsorted partitions by the @offset.
>> For there are maybe only several partitions, i use the simple
>> Bubble sort algorithm.
>>
>> Signed-off-by: Huang Shijie <shijie8@gmail.com>
>
> I still cannot find time to actually think about this carefully, but the
> commit message does not sound convincing, it does not explain why
> sorting is the right way to fix the issue, and what would be the
> alternatives. It actually also does not explain why exactly we currently
> cannot parse the example string.
>
thanks .

I will add more comment in the next version.

Best Regards
Huang Shijie
Artem Bityutskiy - Sept. 3, 2012, 7:21 a.m.
On Sun, 2012-08-26 at 13:21 -0400, Huang Shijie wrote:
> +/* There are only several partitions, so the Bubble sort is enough. */
> +static inline void sort_partitons(struct mtd_partition *parts, int num_parts)
> +{
> +	int i, j;
> +
> +	if (num_parts < 2)
> +		return;

Not necessary, the for loop should work for this case.
> +
> +	if (parts[0].offset == OFFSET_CONTINUOUS)
> +		return;

Hmm, I guess the sorting function should not have this check. You
probably can just sort normally these ones, they will be the last ones.

And then we can do a separate pass where we check for overlapping
partitions and multiple OFFSET_CONTINUOUS.

> +
> +	/* sort by the offset */
> +	for (i = 0; i < num_parts - 1; i++) {
> +		for (j = 1; j < num_parts - i; j++) {
> +			if (parts[j - 1].offset > parts[j].offset) {
> +				struct mtd_partition tmp;
> +
> +				tmp = parts[j - 1];
> +				parts[j - 1] = parts[j];
> +				parts[j] = tmp;
> +			}
> +		}
> +	}
> +	return;
> +}

Patch

diff --git a/drivers/mtd/cmdlinepart.c b/drivers/mtd/cmdlinepart.c
index 0b7b2ad..f40d390 100644
--- a/drivers/mtd/cmdlinepart.c
+++ b/drivers/mtd/cmdlinepart.c
@@ -234,6 +234,32 @@  static struct mtd_partition * newpart(char *s,
 	return parts;
 }
 
+/* There are only several partitions, so the Bubble sort is enough. */
+static inline void sort_partitons(struct mtd_partition *parts, int num_parts)
+{
+	int i, j;
+
+	if (num_parts < 2)
+		return;
+
+	if (parts[0].offset == OFFSET_CONTINUOUS)
+		return;
+
+	/* sort by the offset */
+	for (i = 0; i < num_parts - 1; i++) {
+		for (j = 1; j < num_parts - i; j++) {
+			if (parts[j - 1].offset > parts[j].offset) {
+				struct mtd_partition tmp;
+
+				tmp = parts[j - 1];
+				parts[j - 1] = parts[j];
+				parts[j] = tmp;
+			}
+		}
+	}
+	return;
+}
+
 /*
  * Parse the command line.
  */
@@ -292,6 +318,9 @@  static int mtdpart_setup_real(char *s)
 		this_mtd->mtd_id = (char*)(this_mtd + 1);
 		strlcpy(this_mtd->mtd_id, mtd_id, mtd_id_len + 1);
 
+		/* sort the partitions */
+		sort_partitons(parts, num_parts);
+
 		/* link into chain */
 		this_mtd->next = partitions;
 		partitions = this_mtd;