Patchwork [v2,3/7] UBI: accept empty string for vid_hdr_offs parameter

login
register
mail settings
Submitter Richard Genoud
Date Aug. 20, 2012, 12:09 p.m.
Message ID <1345464561-24464-4-git-send-email-richard.genoud@gmail.com>
Download mbox | patch
Permalink /patch/178755/
State New
Headers show

Comments

Richard Genoud - Aug. 20, 2012, 12:09 p.m.
as a new parameter (max_beb_per1024) will come in next patches, it's
better to accept empty string value so that we can do:
mtd.ubi=2,,25

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
 drivers/mtd/ubi/build.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)
Shmulik Ladkani - Aug. 20, 2012, 12:52 p.m.
Hi Richard,

On Mon, 20 Aug 2012 14:09:17 +0200 Richard Genoud <richard.genoud@gmail.com> wrote:
> @@ -1292,7 +1292,11 @@ static int __init bytes_str_to_int(const char *str)
>  	unsigned long result;
>  
>  	result = simple_strtoul(str, &endp, 0);
> -	if (str == endp || result >= INT_MAX) {
> +	if (str == endp)
> +		/* empty string, assume it's 0 */
> +		return 0;

This will allow "ubi=2,#" (or any non numeric character after the
comma), setting a zero vid_hdr_offs instead of erroring.

How about adding:

+	if (!*str)
+		/* empty string, assume it's 0 */
+		return 0;

Before the original (keep it):

 	result = simple_strtoul(str, &endp, 0);
	if (str == endp || result >= INT_MAX) {

Can you test this works in all cases?

Regards,
Shmulik
Artem Bityutskiy - Aug. 20, 2012, 1:04 p.m.
On Mon, 2012-08-20 at 15:52 +0300, Shmulik Ladkani wrote:
> Hi Richard,
> 
> On Mon, 20 Aug 2012 14:09:17 +0200 Richard Genoud <richard.genoud@gmail.com> wrote:
> > @@ -1292,7 +1292,11 @@ static int __init bytes_str_to_int(const char *str)
> >  	unsigned long result;
> >  
> >  	result = simple_strtoul(str, &endp, 0);
> > -	if (str == endp || result >= INT_MAX) {
> > +	if (str == endp)
> > +		/* empty string, assume it's 0 */
> > +		return 0;

Please, let's not over-engineer this, do not bother with ",,", use ",0,"
instead. Just do amend the documentation properly.
Richard Genoud - Aug. 20, 2012, 1:06 p.m.
2012/8/20 Artem Bityutskiy <dedekind1@gmail.com>:
> On Mon, 2012-08-20 at 15:52 +0300, Shmulik Ladkani wrote:
>> Hi Richard,
>>
>> On Mon, 20 Aug 2012 14:09:17 +0200 Richard Genoud <richard.genoud@gmail.com> wrote:
>> > @@ -1292,7 +1292,11 @@ static int __init bytes_str_to_int(const char *str)
>> >     unsigned long result;
>> >
>> >     result = simple_strtoul(str, &endp, 0);
>> > -   if (str == endp || result >= INT_MAX) {
>> > +   if (str == endp)
>> > +           /* empty string, assume it's 0 */
>> > +           return 0;
>
> Please, let's not over-engineer this, do not bother with ",,", use ",0,"
> instead. Just do amend the documentation properly.

ok, I'll drop the patch and change the doc.

Patch

diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index 3c0b91f..cc0a4fa 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -1292,7 +1292,11 @@  static int __init bytes_str_to_int(const char *str)
 	unsigned long result;
 
 	result = simple_strtoul(str, &endp, 0);
-	if (str == endp || result >= INT_MAX) {
+	if (str == endp)
+		/* empty string, assume it's 0 */
+		return 0;
+
+	if (result >= INT_MAX) {
 		printk(KERN_ERR "UBI error: incorrect bytes count: \"%s\"\n",
 		       str);
 		return -EINVAL;