From: Huang Shijie <shijie8@gmail.com>
To: Shmulik Ladkani <shmulik.ladkani@gmail.com>
Cc: dedekind1@gmail.com, dwmw2@infradead.org,
linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] mtd: cmdlinepart: fix the wrong check condition
Date: Sun, 26 Aug 2012 03:52:12 -0400 [thread overview]
Message-ID: <CAMiH66GXKPDsF0D5OOhdkBuSH-RhBUUQhM2B6bDqe2nc25YzOA@mail.gmail.com> (raw)
In-Reply-To: <20120825123149.3fafa4f4@halley>
On Sat, Aug 25, 2012 at 5:31 AM, Shmulik Ladkani
<shmulik.ladkani@gmail.com> wrote:
> Hi Huang, Artem,
>
> On Sat, 25 Aug 2012 16:06:50 -0400 Huang Shijie <shijie8@gmail.com> wrote:
>> diff --git a/drivers/mtd/cmdlinepart.c b/drivers/mtd/cmdlinepart.c
>> index fc960a3..216d751 100644
>> --- a/drivers/mtd/cmdlinepart.c
>> +++ b/drivers/mtd/cmdlinepart.c
>> @@ -322,13 +322,16 @@ static int parse_cmdline_partitions(struct mtd_info *master,
>> struct cmdline_mtd_partition *part;
>> const char *mtd_id = master->name;
>>
>> + if (!mtd_id)
>> + return 0;
>> +
>> /* parse command line */
>> if (!cmdline_parsed)
>> mtdpart_setup_real(cmdline);
>>
>> for(part = partitions; part; part = part->next)
>> {
>> - if ((!mtd_id) || (!strcmp(part->mtd_id, mtd_id)))
>> + if (!strcmp(part->mtd_id, mtd_id))
>> {
>> for(i = 0, offset = 0; i < part->num_parts; i++)
>> {
>
> This changes the behavior of cmdling parsing, which might affect users
> expecting the old behavior.
>
> According to the remark above 'parse_cmdline_partitions':
>
> * It returns partitions for the requested mtd device, or
> * the first one in the chain if a NULL mtd_id is passed in.
>
> I think the purpose of a NULL 'mtd_id' was to support simple systems
> where there's a single driver and a single chip.
> The driver could be dumb, not specifying its 'mtd_info->name'
> (thus, a NULL mtd_id is passed).
>
> In this case, since the system is simply configured (one driver, one
> chip), 'parse_cmdline_partitions' simply disregards the "mtd-id" name
> specified in the cmdline string, allowing the user to present some
> arbitrary string there.
>
> I quite remember seeing this pattern somewhere in the past, I don't know
> if it's still used, though.
>
thanks for the detail explanation.
> Obviously if you have many drivers (and many chips) in a system, that
> won't work; the drivers must initialize 'mtd_info->name' and the user
> should present a cmdline that has explicit 'mtd-id's.
>
> So question is, would we like to prohibit NULL mtd-id?
I prefer to prohibit the NULL mtd-id. The code looks strange enough.
>
> If so, we must make sure all drivers are properly assigning their
> 'mtd_info->name', and all users correctly specifying 'mtd-id' in their
> "mtdparts" cmdline strings.
yes. I agree that all the driver should set the mtd_info->name properly.
Best Regards
Huang Shijie
>
> Regards,
> Shmulik
next prev parent reply other threads:[~2012-08-26 7:52 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-08-25 20:06 [PATCH v2] mtd: cmdlinepart: fix the wrong check condition Huang Shijie
2012-08-25 9:31 ` Shmulik Ladkani
2012-08-26 7:52 ` Huang Shijie [this message]
2012-09-02 10:56 ` Artem Bityutskiy
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=CAMiH66GXKPDsF0D5OOhdkBuSH-RhBUUQhM2B6bDqe2nc25YzOA@mail.gmail.com \
--to=shijie8@gmail.com \
--cc=dedekind1@gmail.com \
--cc=dwmw2@infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mtd@lists.infradead.org \
--cc=shmulik.ladkani@gmail.com \
/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).