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

[Brian Norris]

On Tue, Nov 5, 2013 at 2:43 PM, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
> On Tue, 22 Oct 2013 13:14:17 +0000 Caizhiyong <caizhiyong@xxxxxxxxxxxxx> wrote:
>
>> In the previous version, adjust the cmdline parser code to library-style
>> code, and move it to a separate file "block/cmdline-parser.c", we can use
>> it in some client code. there is no any functionality change in the adjusting.
>>
>> this patch use cmdline parser lib.
>>
>> For further information, see "https://lkml.org/lkml/2013/8/6/550";
>
> Thanks for doing this.  Could we please get some acked-by's or,
> preferably, tested-by's from the MTD people?

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.

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.

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/