mtd: remove driver-core BUS_ID_SIZE

mtd: remove driver-core BUS_ID_SIZE

[Kay Sievers]

From: Kay Sievers <kay.sievers@vrfy.org>
Subject: mtd: remove driver-core BUS_ID_SIZE

The name size limit is gone from the driver-core, the BUS_ID_SIZE
value will be removed.

Cc: dwmw2@infradead.org
Cc: linux-mtd@lists.infradead.org
Acked-by: Greg Kroah-Hartman <gregkh@suse.de>
Signed-off-by: Kay Sievers <kay.sievers@vrfy.org>
---
 drivers/mtd/nand/txx9ndfmc.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/mtd/nand/txx9ndfmc.c
+++ b/drivers/mtd/nand/txx9ndfmc.c
@@ -64,7 +64,7 @@ struct txx9ndfmc_priv {
 	struct nand_chip chip;
 	struct mtd_info mtd;
 	int cs;
-	char mtdname[BUS_ID_SIZE + 2];
+	char mtdname[20 + 2];
 };
 
 #define MAX_TXX9NDFMC_DEV	4

Re: mtd: remove driver-core BUS_ID_SIZE

[Artem Bityutskiy]

On Thu, 2009-04-16 at 18:26 +0200, Kay Sievers wrote:
> From: Kay Sievers <kay.sievers@vrfy.org>
> Subject: mtd: remove driver-core BUS_ID_SIZE
> 
> The name size limit is gone from the driver-core, the BUS_ID_SIZE
> value will be removed.
> 
> Cc: dwmw2@infradead.org
> Cc: linux-mtd@lists.infradead.org
> Acked-by: Greg Kroah-Hartman <gregkh@suse.de>
> Signed-off-by: Kay Sievers <kay.sievers@vrfy.org>
> ---
>  drivers/mtd/nand/txx9ndfmc.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- a/drivers/mtd/nand/txx9ndfmc.c
> +++ b/drivers/mtd/nand/txx9ndfmc.c
> @@ -64,7 +64,7 @@ struct txx9ndfmc_priv {
>  	struct nand_chip chip;
>  	struct mtd_info mtd;
>  	int cs;
> -	char mtdname[BUS_ID_SIZE + 2];
> +	char mtdname[20 + 2];
>  };
>  
>  #define MAX_TXX9NDFMC_DEV	4

Pushed, to l2-mtd-2.6.git. This does not guarantee anything,
it just mean I'll ping dwmw2 to pull from that tree.

-- 
Best regards,
Artem Bityutskiy (Битюцкий Артём)

Re: mtd: remove driver-core BUS_ID_SIZE

[David Woodhouse]

On Thu, 2009-04-16 at 18:26 +0200, Kay Sievers wrote:
> From: Kay Sievers <kay.sievers@vrfy.org>
> Subject: mtd: remove driver-core BUS_ID_SIZE
> 
> The name size limit is gone from the driver-core, the BUS_ID_SIZE
> value will be removed.
> 
> Cc: dwmw2@infradead.org
> Cc: linux-mtd@lists.infradead.org
> Acked-by: Greg Kroah-Hartman <gregkh@suse.de>
> Signed-off-by: Kay Sievers <kay.sievers@vrfy.org>
> ---
>  drivers/mtd/nand/txx9ndfmc.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- a/drivers/mtd/nand/txx9ndfmc.c
> +++ b/drivers/mtd/nand/txx9ndfmc.c
> @@ -64,7 +64,7 @@ struct txx9ndfmc_priv {
>  	struct nand_chip chip;
>  	struct mtd_info mtd;
>  	int cs;
> -	char mtdname[BUS_ID_SIZE + 2];
> +	char mtdname[20 + 2];
>  };
>  
>  #define MAX_TXX9NDFMC_DEV	4

Not so cunning. You're explicitly noting that the limit on the input
string size is gone, and yet you do nothing about the code which uses
it...

		if (plat->ch_mask != 1) {
			txx9_priv->cs = i;
			sprintf(txx9_priv->mtdname, "%s.%u",
				dev_name(&dev->dev), i);
		} else {
			txx9_priv->cs = -1;
			strcpy(txx9_priv->mtdname, dev_name(&dev->dev));
		}

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: mtd: remove driver-core BUS_ID_SIZE

[David Woodhouse]

On Fri, 2009-05-29 at 14:11 +0100, David Woodhouse wrote:
> On Thu, 2009-04-16 at 18:26 +0200, Kay Sievers wrote:
> > From: Kay Sievers <kay.sievers@vrfy.org>
> > Subject: mtd: remove driver-core BUS_ID_SIZE
> > 
> > The name size limit is gone from the driver-core, the BUS_ID_SIZE
> > value will be removed.
> > 
> > Cc: dwmw2@infradead.org
> > Cc: linux-mtd@lists.infradead.org
> > Acked-by: Greg Kroah-Hartman <gregkh@suse.de>
> > Signed-off-by: Kay Sievers <kay.sievers@vrfy.org>
> > ---
> >  drivers/mtd/nand/txx9ndfmc.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > --- a/drivers/mtd/nand/txx9ndfmc.c
> > +++ b/drivers/mtd/nand/txx9ndfmc.c
> > @@ -64,7 +64,7 @@ struct txx9ndfmc_priv {
> >  	struct nand_chip chip;
> >  	struct mtd_info mtd;
> >  	int cs;
> > -	char mtdname[BUS_ID_SIZE + 2];
> > +	char mtdname[20 + 2];
> >  };
> >  
> >  #define MAX_TXX9NDFMC_DEV	4
> 
> Not so cunning. You're explicitly noting that the limit on the input
> string size is gone, and yet you do nothing about the code which uses
> it...
> 
> 		if (plat->ch_mask != 1) {
> 			txx9_priv->cs = i;
> 			sprintf(txx9_priv->mtdname, "%s.%u",
> 				dev_name(&dev->dev), i);
> 		} else {
> 			txx9_priv->cs = -1;
> 			strcpy(txx9_priv->mtdname, dev_name(&dev->dev));
> 		}

This might work better. Nemoto-san, can you test and confirm?

diff --git a/drivers/mtd/nand/txx9ndfmc.c b/drivers/mtd/nand/txx9ndfmc.c
index 8124792..6329382 100644
--- a/drivers/mtd/nand/txx9ndfmc.c
+++ b/drivers/mtd/nand/txx9ndfmc.c
@@ -64,7 +64,7 @@ struct txx9ndfmc_priv {
 	struct nand_chip chip;
 	struct mtd_info mtd;
 	int cs;
-	char mtdname[BUS_ID_SIZE + 2];
+	char *mtdname;
 };
 
 #define MAX_TXX9NDFMC_DEV	4
@@ -333,12 +333,21 @@ static int __init txx9ndfmc_probe(struct platform_device *dev)
 		txx9_priv->dev = dev;
 
 		if (plat->ch_mask != 1) {
+			char *devname = dev_name(&dev->dev);
 			txx9_priv->cs = i;
+			txx9_priv->mtdname = kmalloc(strlen(devname) + 3,
+						     GFP_KERNEL);
+			if (!txx9_mtdname) {
+				kfree(txx9_priv);
+				dev_err(&dev->dev,
+					"Unable to allocate TXx9 NDFMC MTD device name.\n");
+				continue;
+			}
 			sprintf(txx9_priv->mtdname, "%s.%u",
-				dev_name(&dev->dev), i);
+				devname, i);
 		} else {
 			txx9_priv->cs = -1;
-			strcpy(txx9_priv->mtdname, dev_name(&dev->dev));
+			txx9_priv->mtdname = dev_name(&dev->dev);
 		}
 		if (plat->wide_mask & (1 << i))
 			chip->options |= NAND_BUSWIDTH_16;
@@ -385,6 +394,8 @@ static int __exit txx9ndfmc_remove(struct platform_device *dev)
 		kfree(drvdata->parts[i]);
 #endif
 		del_mtd_device(mtd);
+		if (txx9_priv->mtdname != dev_name(&dev->dev))
+			kfree(txx9_priv->mtdname);
 		kfree(txx9_priv);
 	}
 	return 0;

-- 
dwmw2

Re: mtd: remove driver-core BUS_ID_SIZE

[Kay Sievers]

On Fri, May 29, 2009 at 15:21, David Woodhouse <dwmw2@infradead.org> wrote:
> This might work better. Nemoto-san, can you test and confirm?

Looks good to me.

> diff --git a/drivers/mtd/nand/txx9ndfmc.c b/drivers/mtd/nand/txx9ndfmc.c

>                if (plat->ch_mask != 1) {
> +                       char *devname = dev_name(&dev->dev);
>                        txx9_priv->cs = i;
> +                       txx9_priv->mtdname = kmalloc(strlen(devname) + 3,
> +                                                    GFP_KERNEL);

kasprintf() might be nicer here?

Thanks,
Kay

Re: mtd: remove driver-core BUS_ID_SIZE

[David Woodhouse]

On Fri, 2009-05-29 at 15:25 +0200, Kay Sievers wrote:
> On Fri, May 29, 2009 at 15:21, David Woodhouse <dwmw2@infradead.org> wrote:
> > This might work better. Nemoto-san, can you test and confirm?
> 
> Looks good to me.
> 
> > diff --git a/drivers/mtd/nand/txx9ndfmc.c b/drivers/mtd/nand/txx9ndfmc.c
> 
> >                if (plat->ch_mask != 1) {
> > +                       char *devname = dev_name(&dev->dev);
> >                        txx9_priv->cs = i;
> > +                       txx9_priv->mtdname = kmalloc(strlen(devname) + 3,
> > +                                                    GFP_KERNEL);
> 
> kasprintf() might be nicer here?

True; thanks. (Er, how have I managed not to know that asprintf()
exists, for the last 20-odd years?)

diff --git a/drivers/mtd/nand/txx9ndfmc.c b/drivers/mtd/nand/txx9ndfmc.c
index 8124792..5f919e6 100644
--- a/drivers/mtd/nand/txx9ndfmc.c
+++ b/drivers/mtd/nand/txx9ndfmc.c
@@ -64,7 +64,7 @@ struct txx9ndfmc_priv {
 	struct nand_chip chip;
 	struct mtd_info mtd;
 	int cs;
-	char mtdname[BUS_ID_SIZE + 2];
+	const char *mtdname;
 };
 
 #define MAX_TXX9NDFMC_DEV	4
@@ -334,11 +334,17 @@ static int __init txx9ndfmc_probe(struct platform_device *dev)
 
 		if (plat->ch_mask != 1) {
 			txx9_priv->cs = i;
-			sprintf(txx9_priv->mtdname, "%s.%u",
-				dev_name(&dev->dev), i);
+			txx9_priv->mtdname = kasprintf(GFP_KERNEL, "%s.%u",
+						       dev_name(&dev->dev), i);
+			if (!txx9_priv->mtdname) {
+				kfree(txx9_priv);
+				dev_err(&dev->dev,
+					"Unable to allocate TXx9 NDFMC MTD device name.\n");
+				continue;
+			}
 		} else {
 			txx9_priv->cs = -1;
-			strcpy(txx9_priv->mtdname, dev_name(&dev->dev));
+			txx9_priv->mtdname = dev_name(&dev->dev);
 		}
 		if (plat->wide_mask & (1 << i))
 			chip->options |= NAND_BUSWIDTH_16;
@@ -385,6 +391,8 @@ static int __exit txx9ndfmc_remove(struct platform_device *dev)
 		kfree(drvdata->parts[i]);
 #endif
 		del_mtd_device(mtd);
+		if (txx9_priv->mtdname != dev_name(&dev->dev))
+			kfree(txx9_priv->mtdname);
 		kfree(txx9_priv);
 	}
 	return 0;


-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: mtd: remove driver-core BUS_ID_SIZE

[Atsushi Nemoto]

On Fri, 29 May 2009 14:33:41 +0100, David Woodhouse <dwmw2@infradead.org> wrote:
> > > This might work better. Nemoto-san, can you test and confirm?

Sure, I will test it on next week.

> > > diff --git a/drivers/mtd/nand/txx9ndfmc.c b/drivers/mtd/nand/txx9ndfmc.c
> > 
> > >                if (plat->ch_mask != 1) {
> > > +                       char *devname = dev_name(&dev->dev);
> > >                        txx9_priv->cs = i;
> > > +                       txx9_priv->mtdname = kmalloc(strlen(devname) + 3,
> > > +                                                    GFP_KERNEL);
> > 
> > kasprintf() might be nicer here?
> 
> True; thanks. (Er, how have I managed not to know that asprintf()
> exists, for the last 20-odd years?)
> 
> diff --git a/drivers/mtd/nand/txx9ndfmc.c b/drivers/mtd/nand/txx9ndfmc.c
...
>  		} else {
>  			txx9_priv->cs = -1;
> -			strcpy(txx9_priv->mtdname, dev_name(&dev->dev));
> +			txx9_priv->mtdname = dev_name(&dev->dev);

Use kstrdup() here, otherwise nand_scan() failure will cause memory
leak.

Or you can add conditional kfree into failure path of nand_scan(), but
I think saving few bytes is not worth to complicate an error (and
removal) path.

>  		}
>  		if (plat->wide_mask & (1 << i))
>  			chip->options |= NAND_BUSWIDTH_16;
> @@ -385,6 +391,8 @@ static int __exit txx9ndfmc_remove(struct platform_device *dev)
>  		kfree(drvdata->parts[i]);
>  #endif
>  		del_mtd_device(mtd);
> +		if (txx9_priv->mtdname != dev_name(&dev->dev))
> +			kfree(txx9_priv->mtdname);

If you did kstrdup() as above, you can just call kfree unconditionally.

---
Atsushi Nemoto

Re: mtd: remove driver-core BUS_ID_SIZE

[Atsushi Nemoto]

On Fri, 29 May 2009 14:33:41 +0100, David Woodhouse <dwmw2@infradead.org> wrote:
> > > This might work better. Nemoto-san, can you test and confirm?
...
> > kasprintf() might be nicer here?
> 
> True; thanks. (Er, how have I managed not to know that asprintf()
> exists, for the last 20-odd years?)
> 
> diff --git a/drivers/mtd/nand/txx9ndfmc.c b/drivers/mtd/nand/txx9ndfmc.c
> index 8124792..5f919e6 100644

Your latest patch with slight fix works fine.

Here is diff against your patch.  Please fold into it.  Thanks.

diff --git a/drivers/mtd/nand/txx9ndfmc.c b/drivers/mtd/nand/txx9ndfmc.c
index 5f919e6..fa0458e 100644
--- a/drivers/mtd/nand/txx9ndfmc.c
+++ b/drivers/mtd/nand/txx9ndfmc.c
@@ -344,12 +344,13 @@ static int __init txx9ndfmc_probe(struct platform_device *dev)
 			}
 		} else {
 			txx9_priv->cs = -1;
-			txx9_priv->mtdname = dev_name(&dev->dev);
+			txx9_priv->mtdname = kstrdup(dev_name(&dev->dev));
 		}
 		if (plat->wide_mask & (1 << i))
 			chip->options |= NAND_BUSWIDTH_16;
 
 		if (nand_scan(mtd, 1)) {
+			kfree(txx9_priv->mtdname)
 			kfree(txx9_priv);
 			continue;
 		}
@@ -391,8 +392,7 @@ static int __exit txx9ndfmc_remove(struct platform_device *dev)
 		kfree(drvdata->parts[i]);
 #endif
 		del_mtd_device(mtd);
-		if (txx9_priv->mtdname != dev_name(&dev->dev))
-			kfree(txx9_priv->mtdname);
+		kfree(txx9_priv->mtdname);
 		kfree(txx9_priv);
 	}
 	return 0;

---
Atsushi Nemoto