[...]
> >
> > +/**
> > + * efi_handle_cleanup() - Clean the deleted handle from the various lists
> > + * @handle: handle to remove
> > + * @force: force removal even if protocols are still installed on the
> > handle
> > + *
> > + * Return: status code
> > + */
> > +static void efi_handle_cleanup(efi_handle_t handle, bool force)
> > +{
> > + struct efi_register_notify_event *item, *next;
> > +
> > + if (!list_empty(&handle->protocols) || force)
>
> I guess the parameter force can be dropped from the interface.
>
'force' is here for a reason(see below), although the if is wrong.
The correct one is obviously
if (!list_empty(&handle->protocols) && !force)
....
> > + return;
>
> Please, return a failure code.
I am not sure why you want that, but I'll have a closer look
>
> > + /* The handle is about to be freed. Remove it from events */
> > + list_for_each_entry_safe(item, next, &efi_register_notify_events, link)
> > {
> > + struct efi_protocol_notification *hitem, *hnext;
> > +
> > + list_for_each_entry_safe(hitem, hnext, &item->handles, link) {
> > + if (handle == hitem->handle) {
> > + list_del(&hitem->link);
> > + free(hitem);
> > + }
> > + }
> > + }
> > + /* The last protocol has been removed, delete the handle. */
> > + list_del(&handle->link);
> > + free(handle);
> > +}
> > +
> > /**
> > * efi_process_event_queue() - process event queue
> > */
> > @@ -627,8 +656,7 @@ void efi_delete_handle(efi_handle_t handle)
> > return;
> > }
>
> It would be safer to move the checks if a protocol interface can be
> removed from efi_uninstall_protocol() to efi_remove_protocol(). That way
> we are sure that no driver has attached to the protocols that we try to
> remove.
>
Yes it would, in fact I tried this on my initial patches and I mention it
on the patch description . However the selftests started failing. The reason
is that efi_reinstall_protocol_interface() does not remove the handle and we
have
specific selftests for that. I don't really know or remember why that was
done like that, does the EFI spec describe this? I thought
UninstallProtocolInterface was always supposed to delete the handle if it
had no more protocols
> >
> > - list_del(&handle->link);
> > - free(handle);
> > + efi_handle_cleanup(handle, true);
>
> I don't think we should use a special treatment here. If the protocols
> cannot be deleted, because a driver has attached we should not delete
> the handle.
>
> The function should return a status code. Then efi_disk_delete_raw() can
> return an error for EVT_DM_PRE_REMOVE and stop the device from being
> deleted.
>
We agree again, but I am not trying to fix that atm. The reason for the
special check and the force flag is that efi_delete_handle() is already
doing something similar, which is cwnot optimal, but that's the current
state. It bails out only if the *handle* doesn't exist and doesn't care
if the protocols got removed. If you prefer having a bigger cleanup + fixes
let me know.
Thanks
/Ilias
> Best regards
>
> Heinrich
>
> > }
> >
> > /**
> > @@ -1396,11 +1424,8 @@ static efi_status_t EFIAPI
> > efi_uninstall_protocol_interface
> > if (ret != EFI_SUCCESS)
> > goto out;
> >
> > - /* If the last protocol has been removed, delete the handle. */
> > - if (list_empty(&handle->protocols)) {
> > - list_del(&handle->link);
> > - free(handle);
> > - }
> > + /* Check if we have to purge the handle from various lists */
> > + efi_handle_cleanup(handle, false);
> > out:
> > return EFI_EXIT(ret);
> > }
> > @@ -2777,11 +2802,7 @@
> > efi_uninstall_multiple_protocol_interfaces_int(efi_handle_t handle,
> > i++;
> > }
> > if (ret == EFI_SUCCESS) {
> > - /* If the last protocol has been removed, delete the handle. */
> > - if (list_empty(&handle->protocols)) {
> > - list_del(&handle->link);
> > - free(handle);
> > - }
> > + efi_handle_cleanup(handle, false);
> > goto out;
> > }
> >
>
