Re: [PATCH 1/3] mtd: cmdlinepart: make the partitions rule more strict

[Artem Bityutskiy <dedekind1@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2390 bytes --]

[Dropped extra CCs - let's spam less]

On Mon, 2012-09-03 at 11:09 -0400, Huang Shijie wrote:
> On Mon, Sep 3, 2012 at 3:18 AM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> > On Sun, 2012-08-26 at 13:21 -0400, Huang Shijie wrote:
> >> + *
> >> + * Note:
> >> + * If you choose to set the @offset for the <partdef>, please set all
> >> + * the partitions with the same syntax, such as:
> >> + *     gpmi-nand:100m@0(boot),100m@100m(kernel),1g@200m(rootfs)
> >> + *
> >> + * Please do _NOT_ set the partitions like this:
> >> + *     gpmi-nand:100m@0(boot),100m(kernel),1g@200m(rootfs)
> >> + * The `kernel` partition does not set with the @offset, this is not permitted.
> >>   */
> >
> > I guess it is indeed OK to sort the partitions, just makes things a lot
> > simpler. But we probably then should also do the following:
> >
> > 1. Make sure there is only one partition without offset. If there are
> > several - error out.
> 
>  Why allow `only one partition without offset`?

E.g.,

gpmi-nand:1g@200m(rootfs),100m(boot),100m@100m(kernel),200m(ququ)

how would "boot" and "ququ" look like?

What I basically say is that we should refuse lines like this. Which
means checking that there is only one "OFFSET_CONTINOUS" partition.

> Take the following unsorted partitions as an example:
>      #gpmi-nand:1g@200m(rootfs),100m(boot),100m@100m(kernel)
> 
> The current code will parses out the following partitions:
>       rootfs: < size = 1g, offset=200m>
>       boot:   < size = 100m, offset= OFFSET_CONTINOUS>
>       kernel: <size = 100m, offset = 100m>
> 
> If we sort the partitions by the offset, we get the following result:
>      "kernel" , "rootfs", "boot"

Yeah, for some reason I assumed that if you do not specify offset then
you mean the partition must be the _last_ and span the rest of flash. I
assume other usage is bogus. So from this POW the example is bogus and
lines like this would be rejected.

I guess the criteria is that this sting contains a "gap" (0-100m).

With the logic I suggested, for this case what I said would basically:

1. Sort. This leads to

100m@100m(kernel)
1g@200m(rootfs)
100m(boot)

"OFFSET_CONTINOUS" is always the largest and will be at the end when
sorting.

2. Verification.

We verify for overlaps and gaps. And refuse this one.

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]