RE: [PATCH 2/2] mtd: cmdlinepart: use cmdline partition parser lib

From: Caizhiyong
Date: Fri Nov 08 2013 - 02:14:36 EST


> Nobody has had time to test this on MTD, it seems, and as such, I
> strongly recommend you do not force it through -mm. We are perfectly
> capable of merging it through the MTD tree if it ever gets proper
> vetting by people in MTD (not just on block devices), and I am well
> aware of this patch set's existence.
>
> However, the patch really provides little to no benefit to the MTD
> subsystem at the moment, at the added cost of reviewing the small
> differences in parsing. For some reason, Cai decided to make small
> differences in the parser between drivers/mtd/cmdlinepart.c and
> block/cmdline-parser.c, and it is not obvious that these differences
> retain the same parsing. For instance, according to my code
> read-through, the block device parser seems to accept multiple
> partitions to be specified with "-" (meaning "fill the remaining
> device"). MTD already has code that rejects such a parser string
> outright.

The next '-' partition be ignore at function "cmdline_parts_set", and the client will get a right result.
Is there any other worry about '-' I don't know ?

>
> So, I'd recommend one of the following:
> (1) We (MTD users) make more time to properly test this patch and post
> relevant results (i.e., tested partition strings) or
> (2) Somebody (Cai?) spend time to actually make block/cmdline-parser.c
> fully equivalent to the parser in drivers/mtd/cmdlinepart.c. That
> means it should be trivial to compare the two and say "yes, these are
> equivalent". That is currently not the case, AFAICT.

I understand your worry about, we use cmdlinepart many years.
I will spend time to make block/cmdline-parser.c fully equivalent to the
parser in drivers/mtd/cmdlinepart.c.

>
> Without one of those two, I will give my NAK.
>
> Brian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/