[PATCH] discover/udev.c: Added warning in system status log
Jeremy Kerr
jk at ozlabs.org
Tue Sep 28 13:35:35 AEST 2021
Hi Lulu,
> When the same iso files are installed at the same time,
> the petitboot menu will only display the installed device first,
> and will not notify the user that the same iso file has been
> installed,
> so an alert is added to the system status log to remind the user that
> they have mounted the same iso file.
>
> Signed-off-by: LuluTHSu <Lulu_Su at wistron.com>
If you're sending a second version of the patch, please include a short
description of the change. It's OK for now, but can be handy for future
revisions.
It looks like this version just has the discover_device_create()
removed, which is better. However, I still think it needs
simplification:
> @@ -184,6 +185,13 @@ static int udev_handle_block_add(struct pb_udev *udev, struct udev_device *dev,
> if (ddev) {
> pb_log("SKIP: %s UUID [%s] already present (as %s)\n",
> name, uuid, ddev->device->id);
> + if (strncmp(name, ddev->device->id, strlen(ddev->device->id)) && (strlen(name) > strlen(ddev->device->id))) {
> + char temp[] = "sdx";
> + strncpy(temp, name, strlen(ddev->device->id));
This introduces a potential stack overflow - we may attempt to write
more data into 'temp' than it can store.
But still, I don't know why you're making the log message conditional
on not being a prefix of the other device. From your log in the previous
discussion, you end up with four devices with the same UUID; surely you
want to inform the user about all of them, rather than a subset? This
check makes a lot of assumptions about when a duplicate device is worth
logging or not.
In that case, this patch would be *really* simple: just unconditionally
call device_handler_status_info() with the log message. You probably
want to make it shorter so it can fit in the status line of a default
80-column petitboot UI.
Regards,
Jeremy
More information about the Petitboot
mailing list