| Message ID | aZg8HnSkj3p2lhkY@rocket |
|---|---|
| State | New |
| Delegated to: | Stefano Babic |
| Headers | show |
| Series | sbabic/libubootenv][PATCH v2] Non-existing devices results in parsing error (Issue #33) | expand |
Hi Harald, On 2/20/26 11:49, Harald Brinkmann wrote: > @LeSpocky requested changes on this pull request: >> Other part of the code initialize the buffer with all zeros to avoid >> truncation issues before calling strncpy(), example: memset(normalized, >> 0, PATH_MAX); > Fixed. > >> How does this affect the call of check_env_device() in function >> libuboot_read_config_ext() (file src/uboot_env.c line 913). > It does not fail if one of the env devices is not readable. > Is there an issue down the line if there is a non-existing env device? > > This might be a quck and dirty hack. It sure was quick. > > On Feb 18, 2026, 10:39:25, Stefano Babic wrote: >> The right solution is to rearrange code and call check_env() when this >> is used. This must be caleld just once, that is the reason it is now >> done as extension of the configuration > If you clarify this, I have a go at a clean solution. It is a change in the supported feature. Currently, it requires that the configured file (fw_env.config) points to existing device. And the current use case, if the device are not existing (but still static), is to adjust fw_env.config at runtime. Changes here are not acceptable because they simply skip the check and solves only when fw_setenv / fw_printev are called as tools. However, this project is thought to be a library (with yes, replacement for old tools, too), and a user application can link to the library and use it, mainly calling libuboot_set_env() and libuboot_get_env(), so without reinitializing again. That means, specially if the environment is not on a device (MTD or whatever, but just on a file, this file could maybe been created to a later time, and then the check should be done when devic eis really used. Note also that your patch is skipping the error and it is just storing that the configuration is ok, but it is not. I can then call libuboot_get_namespace() for a bad configured environment without notice, or worse I get an error much later. So what I would like to see: - a namespace can get an additional atribute, "optional" - if optional, the check can be skipped, but it must be marked in the structure as "non valid" - if a call tries to access a namespace that is marked "non valid", it must be checked again because run time conditions can change. Best regards, Stefano Babic > > > Ignore devices which are not existing. This might be partitions on > unplugged SD cards. > > Signed-off-by: Harald Brinkmann <brinkmann@stulz-digital-solutions.com> > --- > src/common.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/src/common.c b/src/common.c > index 2cbde93..9e9f037 100644 > --- a/src/common.c > +++ b/src/common.c > @@ -65,8 +65,10 @@ int normalize_device_path(char *path, struct uboot_flash_env *dev) > > if ((normalized = realpath(path, NULL)) == NULL) > { > - /* device file didn't exist */ > - return -EINVAL; > + /* device file does not exist */ > + normalized = malloc(PATH_MAX); > + memset(normalized, 0, PATH_MAX); > + strncpy(normalized, path, PATH_MAX - 1); > } > > normalized_len = strlen(normalized); > @@ -184,8 +186,10 @@ int check_env_device(struct uboot_flash_env *dev) > } > > ret = stat(dev->devname, &st); > - if (ret < 0) > - return -EBADF; > + if (ret < 0) { > + /* device is not readable, no further checks possible */ > + return 0; > + } > fd = open(dev->devname, O_RDONLY); > if (fd < 0) > return -EBADF; > -- > 2.52.0 > > Sitz der Gesellschaft: Hamburg Amtsgericht Hamburg | HRB 172947 Geschäftsführer: Markus Trautwein, Thorsten Weiß >
diff --git a/src/common.c b/src/common.c index 2cbde93..9e9f037 100644 --- a/src/common.c +++ b/src/common.c @@ -65,8 +65,10 @@ int normalize_device_path(char *path, struct uboot_flash_env *dev) if ((normalized = realpath(path, NULL)) == NULL) { - /* device file didn't exist */ - return -EINVAL; + /* device file does not exist */ + normalized = malloc(PATH_MAX); + memset(normalized, 0, PATH_MAX); + strncpy(normalized, path, PATH_MAX - 1); } normalized_len = strlen(normalized); @@ -184,8 +186,10 @@ int check_env_device(struct uboot_flash_env *dev) } ret = stat(dev->devname, &st); - if (ret < 0) - return -EBADF; + if (ret < 0) { + /* device is not readable, no further checks possible */ + return 0; + } fd = open(dev->devname, O_RDONLY); if (fd < 0) return -EBADF;