On Tue, 2017-07-11 at 15:37 +0200, Reichel Andreas wrote: > If the user manually creates a loopback device node > in /dev, for example /dev/NAME and uses it to setup a > loopback device, the resulting base name of /sys/block/loop* > does not match that of /dev/NAME. On some systems, > /dev/loop* gets created automatically by using losetup, on > others not. > The solution is to read the major and minor revision > out of /sys/block/NAME/dev and look for the same > in /dev. Thus, the correct block device node in /dev > can be found. > > Signed-off-by: Andreas Reichel <[email protected]> > --- > tools/ebgpart.c | 97 > ++++++++++++++++++++++++++++++++++++++++++++++++--------- > tools/ebgpart.h | 3 +- > 2 files changed, 85 insertions(+), 15 deletions(-) > > diff --git a/tools/ebgpart.c b/tools/ebgpart.c > index 07edb05..5375fbb 100644 > --- a/tools/ebgpart.c > +++ b/tools/ebgpart.c > @@ -353,27 +353,96 @@ bool check_partition_table(PedDevice *dev) > return true; > } > > -void ped_device_probe_all() > +void scan_devdir(unsigned int fmajor, unsigned int fminor, char > *fullname, > + unsigned int maxlen)
scan_devdir should have an explicit return value about the success of
it. Don't do it over the fullname length.
> {
> + DIR *devdir = opendir(DEVDIR);
> + if (!devdir) {
> + VERBOSE(stderr, "Failed to open %s\n", DEVDIR);
Shouldn't that also set `fullname[0] = 0`? If that is the case, then
this shows why a return value is better for this stuff.
> + return;
> + }
> struct dirent *devfile;
> + do {
> + devfile = readdir(devdir);
> + if (!devfile) {
> + break;
> + }
> + snprintf(fullname, maxlen, "%s/%s", DEVDIR, devfile-
> >d_name);
> + struct stat fstat;
> + if (stat(fullname, &fstat) == -1) {
> + VERBOSE(stderr, "stat failed on %s\n",
> fullname);
> + break;
> + }
> + if (major(fstat.st_rdev) == fmajor &&
> + minor(fstat.st_rdev) == fminor) {
> + VERBOSE(stdout, "Node found: %s\n",
> fullname);
> + break;
> + }
> + fullname[0] = 0;
Hmm, you might have to check if maxlen >= 1 before doing that ;)
Go with a decent return value instead, please.
> + } while (devfile);
> + closedir(devdir);
> +}
> +
> +int get_major_minor(char *filename, unsigned int *major, unsigned
> int *minor)
> +{
> + if (!filename || !major || !minor) {
> + return -1;
> + }
> + FILE *fh = fopen(filename, "r");
> + if (fh == 0) {
> + VERBOSE(stderr, "Error opening %s for read",
> filename);
> + return -1;
> + }
> + int res = fscanf(fh, "%u:%u", major, minor);
> + fclose(fh);
> + if (res < 2) {
> + VERBOSE(stderr,
> + "Error reading major/minor of device entry.
> (%s)\n",
> + strerror(errno));
> + return -1;
> + };
> + return 0;
> +}
> +
> +void ped_device_probe_all()
> +{
> + struct dirent *sysblockfile;
> char fullname[256];
>
> - DIR *devdir = opendir(DEVDIRNAME);
> - if (!devdir) {
> - VERBOSE(stderr, "Could not open %s\n", DEVDIRNAME);
> + DIR *sysblockdir = opendir(SYSBLOCKDIR);
> + if (!sysblockdir) {
> + VERBOSE(stderr, "Could not open %s\n", SYSBLOCKDIR);
> return;
> }
>
> - /* get all files from devdir */
> + /* get all files from sysblockdir */
> do {
> - devfile = readdir(devdir);
> - if (!devfile)
> - break;
> - if (strcmp(devfile->d_name, ".") == 0 ||
> - strcmp(devfile->d_name, "..") == 0)
> + sysblockfile = readdir(sysblockdir);
> + if (!sysblockfile) break;
> + if (strcmp(sysblockfile->d_name, ".") == 0 ||
> + strcmp(sysblockfile->d_name, "..") == 0)
> continue;
> -
> - snprintf(fullname, 255, "/dev/%s", devfile->d_name);
> + snprintf(fullname, 255, "/sys/block/%s/dev",
255/256 is a constant, give it a name. define or just constant, for me
thats not that important. But maybe define in the header might be the
right way to do.
Also PRINTF(3):
The functions snprintf() and vsnprintf() write at most size bytes
(including the terminating null byte ('\0')) to str.
So correct me if I am wrong, but shouldn't there be 256 instead of 255?
Maybe just use sizeof(fullname)?
> + sysblockfile->d_name);
> + /* Get major and minor revision from
> /sys/block/sdX/dev */
> + unsigned int fmajor, fminor;
> + if (get_major_minor(fullname, &fmajor, &fminor) ==
> -1) {
Minor issue: I would prefer < 0 in these cases. If a function is
expanded later to cover multiple different error values, you don't need
to fix this.
> + continue;
> + }
> + VERBOSE(stdout,
> + "Trying device with: Major = %d, Minor = %d,
> (%s)\n",
> + fmajor, fminor, fullname);
> + /* Check if this file is really in the dev directory
> */
> + snprintf(fullname, 255, "%s/%s", DEVDIR,
> sysblockfile->d_name);
> + struct stat fstat;
> + if (stat(fullname, &fstat) == -1) {
> + /* Node with same name not found in /dev,
> thus search
> + * for node with identical Major and Minor
> revision */
> + scan_devdir(fmajor, fminor, fullname, 255);
> + }
If you have a scan_devdir return value, you can easily (and cleanly)
add additional ways to get to the block device path later. mknod if
that is enabled via command line switch/enabled on compile time.
> + if (strlen(fullname) == 0) {
> + continue;
> + }
> /* This is a block device, so add it to the list*/
> PedDevice *dev = calloc(sizeof(PedDevice), 1);
> asprintf(&dev->model, "N/A");
> @@ -385,9 +454,9 @@ void ped_device_probe_all()
> free(dev->path);
> free(dev);
> }
> - } while (devfile);
> + } while (sysblockfile);
>
> - closedir(devdir);
> + closedir(sysblockdir);
> }
>
> void ped_partition_destroy(PedPartition *p)
> diff --git a/tools/ebgpart.h b/tools/ebgpart.h
> index 76fee9d..679a1d7 100644
> --- a/tools/ebgpart.h
> +++ b/tools/ebgpart.h
> @@ -41,7 +41,8 @@
> #include <string.h>
> #include <stdlib.h>
>
> -#define DEVDIRNAME "/sys/block"
> +#define SYSBLOCKDIR "/sys/block"
> +#define DEVDIR "/dev"
>
> #define LB_SIZE 512
>
> --
> 2.11.0
>
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-54 Fax: (+49)-8142-66989-80 Email: [email protected]
PGP key: 6FF2 E59F 00C6 BC28 31D8 64C1 1173 CB19 9808 B153
Keyserver: hkp://pool.sks-keyservers.net
--
You received this message because you are subscribed to the Google Groups "EFI
Boot Guard" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/efibootguard-dev/1499798198.3494.3.camel%40denx.de.
For more options, visit https://groups.google.com/d/optout.
signature.asc
Description: This is a digitally signed message part
