[RFC] mount: Fix the printing of device name

[RFC] mount: Fix the printing of device name

[franck.jullien]

From: Franck Jullien <franck.jullien@gmail.com>

Mount without argument always print a "none" as device name mounted
because entry->parent_device is always NULL.

The problem is the mount function in fs/fs.c. parent_device is
initialized to NULL and never updated. With this patch, parent_device
is set with the mounted device name.

Moreover, the mount function has been modified to print the device name
plus device id using the dev_name function.

Signed-off-by: Franck Jullien <franck.jullien@gmail.com>
---
 commands/mount.c |    2 +-
 fs/fs.c          |   11 +++++++++++
 2 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/commands/mount.c b/commands/mount.c
index 52d1700..7cefdbe 100644
--- a/commands/mount.c
+++ b/commands/mount.c
@@ -40,7 +40,7 @@ static int do_mount(struct command *cmdtp, int argc, char *argv[])
 			entry = mtab_next_entry(entry);
 			if (entry) {
 				printf("%s on %s type %s\n",
-					entry->parent_device ? entry->parent_device->name : "none",
+					entry->parent_device ? dev_name(entry->parent_device) : "none",
 					entry->path,
 					entry->dev->name);
 			}
diff --git a/fs/fs.c b/fs/fs.c
index 51a7411..529ff53 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -31,6 +31,7 @@
 #include <init.h>
 #include <module.h>
 #include <libbb.h>
+#include <libgen.h>
 
 void *read_file(const char *filename, size_t *size)
 {
@@ -741,6 +742,7 @@ int mount(const char *device, const char *fsname, const char *_path)
 	struct device_d *dev, *parent_device = NULL;
 	int ret;
 	char *path = normalise_path(_path);
+	char *devname, *par;
 
 	errno = 0;
 
@@ -804,6 +806,15 @@ int mount(const char *device, const char *fsname, const char *_path)
 		goto out2;
 	}
 
+	devname = basename((char *)device);
+
+	if (strchr(devname, '.')) {
+		par = strchr(devname, '.');
+		*par = 0;
+	}
+
+	parent_device = get_device_by_name(devname);
+
 	if (parent_device)
 		dev_add_child(parent_device, &fsdev->dev);
 
-- 
1.7.7


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [RFC] mount: Fix the printing of device name

[Sascha Hauer]

Hi Franck,

On Sun, Nov 06, 2011 at 08:20:48AM +0100, franck.jullien@gmail.com wrote:
> From: Franck Jullien <franck.jullien@gmail.com>
> 
> Mount without argument always print a "none" as device name mounted
> because entry->parent_device is always NULL.
> 
> The problem is the mount function in fs/fs.c. parent_device is
> initialized to NULL and never updated. With this patch, parent_device
> is set with the mounted device name.
> 
> Moreover, the mount function has been modified to print the device name
> plus device id using the dev_name function.
> 
> Signed-off-by: Franck Jullien <franck.jullien@gmail.com>
> ---
>  commands/mount.c |    2 +-
>  fs/fs.c          |   11 +++++++++++
>  2 files changed, 12 insertions(+), 1 deletions(-)
> 
> diff --git a/commands/mount.c b/commands/mount.c
> index 52d1700..7cefdbe 100644
> --- a/commands/mount.c
> +++ b/commands/mount.c
> @@ -40,7 +40,7 @@ static int do_mount(struct command *cmdtp, int argc, char *argv[])
>  			entry = mtab_next_entry(entry);
>  			if (entry) {
>  				printf("%s on %s type %s\n",
> -					entry->parent_device ? entry->parent_device->name : "none",
> +					entry->parent_device ? dev_name(entry->parent_device) : "none",
>  					entry->path,
>  					entry->dev->name);
>  			}
> diff --git a/fs/fs.c b/fs/fs.c
> index 51a7411..529ff53 100644
> --- a/fs/fs.c
> +++ b/fs/fs.c
> @@ -31,6 +31,7 @@
>  #include <init.h>
>  #include <module.h>
>  #include <libbb.h>
> +#include <libgen.h>
>  
>  void *read_file(const char *filename, size_t *size)
>  {
> @@ -741,6 +742,7 @@ int mount(const char *device, const char *fsname, const char *_path)
>  	struct device_d *dev, *parent_device = NULL;
>  	int ret;
>  	char *path = normalise_path(_path);
> +	char *devname, *par;
>  
>  	errno = 0;
>  
> @@ -804,6 +806,15 @@ int mount(const char *device, const char *fsname, const char *_path)
>  		goto out2;
>  	}
>  
> +	devname = basename((char *)device);
> +
> +	if (strchr(devname, '.')) {
> +		par = strchr(devname, '.');
> +		*par = 0;
> +	}
> +
> +	parent_device = get_device_by_name(devname);

This does not look good. First of all basename returns (a part of) the
same string which is passed to it, so you modify a const char * here.
Then you try to find the device name by using the first part before the
'.', but the name of the file under /dev/ does not necessarily match the
device name.

I think what you should do here is to see whether the first part of
*device matches "/dev/" and then use cdev_open on the second part. The
struct cdev from cdev_open contains the device pointer you are looking
for.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [RFC] mount: Fix the printing of device name

[Franck JULLIEN]

2011/11/7 Sascha Hauer <s.hauer@pengutronix.de>:
> Hi Franck,
>
> On Sun, Nov 06, 2011 at 08:20:48AM +0100, franck.jullien@gmail.com wrote:
>> From: Franck Jullien <franck.jullien@gmail.com>
>>
>> Mount without argument always print a "none" as device name mounted
>> because entry->parent_device is always NULL.
>>
>> The problem is the mount function in fs/fs.c. parent_device is
>> initialized to NULL and never updated. With this patch, parent_device
>> is set with the mounted device name.
>>
>> Moreover, the mount function has been modified to print the device name
>> plus device id using the dev_name function.
>>
>> Signed-off-by: Franck Jullien <franck.jullien@gmail.com>
>> ---
>>  commands/mount.c |    2 +-
>>  fs/fs.c          |   11 +++++++++++
>>  2 files changed, 12 insertions(+), 1 deletions(-)
>>
>> diff --git a/commands/mount.c b/commands/mount.c
>> index 52d1700..7cefdbe 100644
>> --- a/commands/mount.c
>> +++ b/commands/mount.c
>> @@ -40,7 +40,7 @@ static int do_mount(struct command *cmdtp, int argc, char *argv[])
>>                       entry = mtab_next_entry(entry);
>>                       if (entry) {
>>                               printf("%s on %s type %s\n",
>> -                                     entry->parent_device ? entry->parent_device->name : "none",
>> +                                     entry->parent_device ? dev_name(entry->parent_device) : "none",
>>                                       entry->path,
>>                                       entry->dev->name);
>>                       }
>> diff --git a/fs/fs.c b/fs/fs.c
>> index 51a7411..529ff53 100644
>> --- a/fs/fs.c
>> +++ b/fs/fs.c
>> @@ -31,6 +31,7 @@
>>  #include <init.h>
>>  #include <module.h>
>>  #include <libbb.h>
>> +#include <libgen.h>
>>
>>  void *read_file(const char *filename, size_t *size)
>>  {
>> @@ -741,6 +742,7 @@ int mount(const char *device, const char *fsname, const char *_path)
>>       struct device_d *dev, *parent_device = NULL;
>>       int ret;
>>       char *path = normalise_path(_path);
>> +     char *devname, *par;
>>
>>       errno = 0;
>>
>> @@ -804,6 +806,15 @@ int mount(const char *device, const char *fsname, const char *_path)
>>               goto out2;
>>       }
>>
>> +     devname = basename((char *)device);
>> +
>> +     if (strchr(devname, '.')) {
>> +             par = strchr(devname, '.');
>> +             *par = 0;
>> +     }
>> +
>> +     parent_device = get_device_by_name(devname);
>
> This does not look good. First of all basename returns (a part of) the
> same string which is passed to it, so you modify a const char * here.
> Then you try to find the device name by using the first part before the
> '.', but the name of the file under /dev/ does not necessarily match the
> device name.
>
> I think what you should do here is to see whether the first part of
> *device matches "/dev/" and then use cdev_open on the second part. The
> struct cdev from cdev_open contains the device pointer you are looking
> for.
>
> Sascha
>
> --
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>

Ok Sasha, thanks for the tips. You are saying I was lucky it worked on
my board :)
I'll rework on this.

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox