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

[Franck JULLIEN <franck.jullien@gmail.com>]

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