| Message ID | 20210927043054.32727-1-takahiro.akashi@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | efi_driver: fix a wrong cast | expand |
On 9/27/21 6:30 AM, AKASHI Takahiro wrote: > struct efi_driver_binding_protocol, i.e. bp, is not the first element > in struct efi_driver_binding_extended_protocol. > So the casting: > struct efi_driver_binding_extended_protocol *bp = > (struct efi_driver_binding_extended_protocol *)this; > is simply wrong. The definition in include/efi_driver.h is: /* * This structure adds internal fields to the driver binding protocol. */ struct efi_driver_binding_extended_protocol { struct efi_driver_binding_protocol bp; const struct efi_driver_ops *ops; }; Why do you claim "bp is not the first element"? The whole efi_driver code relies on the fact that the efi_driver_binding_extended_protocol can be interpreted as efi_driver_binding_protocol. Best regards Heinrich > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > --- > lib/efi_driver/efi_uclass.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/lib/efi_driver/efi_uclass.c b/lib/efi_driver/efi_uclass.c > index 382c2b477f4d..9e784f9c237f 100644 > --- a/lib/efi_driver/efi_uclass.c > +++ b/lib/efi_driver/efi_uclass.c > @@ -65,8 +65,8 @@ static efi_status_t EFIAPI efi_uc_supported( > { > efi_status_t r, ret; > void *interface; > - struct efi_driver_binding_extended_protocol *bp = > - (struct efi_driver_binding_extended_protocol *)this; > + struct efi_driver_binding_extended_protocol *bp = container_of(this, > + struct efi_driver_binding_extended_protocol, bp); > > EFI_ENTRY("%p, %p, %ls", this, controller_handle, > efi_dp_str(remaining_device_path)); > @@ -113,8 +113,8 @@ static efi_status_t EFIAPI efi_uc_start( > { > efi_status_t r, ret; > void *interface = NULL; > - struct efi_driver_binding_extended_protocol *bp = > - (struct efi_driver_binding_extended_protocol *)this; > + struct efi_driver_binding_extended_protocol *bp = container_of(this, > + struct efi_driver_binding_extended_protocol, bp); > > EFI_ENTRY("%p, %p, %ls", this, controller_handle, > efi_dp_str(remaining_device_path)); > @@ -201,8 +201,8 @@ static efi_status_t EFIAPI efi_uc_stop( > efi_status_t ret; > efi_uintn_t count; > struct efi_open_protocol_info_entry *entry_buffer; > - struct efi_driver_binding_extended_protocol *bp = > - (struct efi_driver_binding_extended_protocol *)this; > + struct efi_driver_binding_extended_protocol *bp = container_of(this, > + struct efi_driver_binding_extended_protocol, bp); > > EFI_ENTRY("%p, %p, %zu, %p", this, controller_handle, > number_of_children, child_handle_buffer); > @@ -283,7 +283,7 @@ static efi_status_t efi_add_driver(struct driver *drv) > } > bp->bp.image_handle = bp->bp.driver_binding_handle; > ret = efi_add_protocol(bp->bp.driver_binding_handle, > - &efi_guid_driver_binding_protocol, bp); > + &efi_guid_driver_binding_protocol, &bp->bp); > if (ret != EFI_SUCCESS) { > efi_delete_handle(bp->bp.driver_binding_handle); > free(bp); >
On Mon, Sep 27, 2021 at 09:48:54AM +0200, Heinrich Schuchardt wrote: > > > On 9/27/21 6:30 AM, AKASHI Takahiro wrote: > > struct efi_driver_binding_protocol, i.e. bp, is not the first element > > in struct efi_driver_binding_extended_protocol. > > So the casting: > > struct efi_driver_binding_extended_protocol *bp = > > (struct efi_driver_binding_extended_protocol *)this; > > is simply wrong. > > The definition in include/efi_driver.h is: > > /* > * This structure adds internal fields to the driver binding protocol. > */ > struct efi_driver_binding_extended_protocol { > struct efi_driver_binding_protocol bp; > const struct efi_driver_ops *ops; > }; > > Why do you claim "bp is not the first element"? > > The whole efi_driver code relies on the fact that the > efi_driver_binding_extended_protocol can be interpreted as > efi_driver_binding_protocol. Oops, *I* have added some member elements in another patch. (I'm trying to make efi_driver objects show up in DM tree.) Please forget this patch for a while. -Takahiro Akashi > Best regards > > Heinrich > > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > --- > > lib/efi_driver/efi_uclass.c | 14 +++++++------- > > 1 file changed, 7 insertions(+), 7 deletions(-) > > > > diff --git a/lib/efi_driver/efi_uclass.c b/lib/efi_driver/efi_uclass.c > > index 382c2b477f4d..9e784f9c237f 100644 > > --- a/lib/efi_driver/efi_uclass.c > > +++ b/lib/efi_driver/efi_uclass.c > > @@ -65,8 +65,8 @@ static efi_status_t EFIAPI efi_uc_supported( > > { > > efi_status_t r, ret; > > void *interface; > > - struct efi_driver_binding_extended_protocol *bp = > > - (struct efi_driver_binding_extended_protocol *)this; > > + struct efi_driver_binding_extended_protocol *bp = container_of(this, > > + struct efi_driver_binding_extended_protocol, bp); > > > > EFI_ENTRY("%p, %p, %ls", this, controller_handle, > > efi_dp_str(remaining_device_path)); > > @@ -113,8 +113,8 @@ static efi_status_t EFIAPI efi_uc_start( > > { > > efi_status_t r, ret; > > void *interface = NULL; > > - struct efi_driver_binding_extended_protocol *bp = > > - (struct efi_driver_binding_extended_protocol *)this; > > + struct efi_driver_binding_extended_protocol *bp = container_of(this, > > + struct efi_driver_binding_extended_protocol, bp); > > > > EFI_ENTRY("%p, %p, %ls", this, controller_handle, > > efi_dp_str(remaining_device_path)); > > @@ -201,8 +201,8 @@ static efi_status_t EFIAPI efi_uc_stop( > > efi_status_t ret; > > efi_uintn_t count; > > struct efi_open_protocol_info_entry *entry_buffer; > > - struct efi_driver_binding_extended_protocol *bp = > > - (struct efi_driver_binding_extended_protocol *)this; > > + struct efi_driver_binding_extended_protocol *bp = container_of(this, > > + struct efi_driver_binding_extended_protocol, bp); > > > > EFI_ENTRY("%p, %p, %zu, %p", this, controller_handle, > > number_of_children, child_handle_buffer); > > @@ -283,7 +283,7 @@ static efi_status_t efi_add_driver(struct driver *drv) > > } > > bp->bp.image_handle = bp->bp.driver_binding_handle; > > ret = efi_add_protocol(bp->bp.driver_binding_handle, > > - &efi_guid_driver_binding_protocol, bp); > > + &efi_guid_driver_binding_protocol, &bp->bp); > > if (ret != EFI_SUCCESS) { > > efi_delete_handle(bp->bp.driver_binding_handle); > > free(bp); > >
diff --git a/lib/efi_driver/efi_uclass.c b/lib/efi_driver/efi_uclass.c index 382c2b477f4d..9e784f9c237f 100644 --- a/lib/efi_driver/efi_uclass.c +++ b/lib/efi_driver/efi_uclass.c @@ -65,8 +65,8 @@ static efi_status_t EFIAPI efi_uc_supported( { efi_status_t r, ret; void *interface; - struct efi_driver_binding_extended_protocol *bp = - (struct efi_driver_binding_extended_protocol *)this; + struct efi_driver_binding_extended_protocol *bp = container_of(this, + struct efi_driver_binding_extended_protocol, bp); EFI_ENTRY("%p, %p, %ls", this, controller_handle, efi_dp_str(remaining_device_path)); @@ -113,8 +113,8 @@ static efi_status_t EFIAPI efi_uc_start( { efi_status_t r, ret; void *interface = NULL; - struct efi_driver_binding_extended_protocol *bp = - (struct efi_driver_binding_extended_protocol *)this; + struct efi_driver_binding_extended_protocol *bp = container_of(this, + struct efi_driver_binding_extended_protocol, bp); EFI_ENTRY("%p, %p, %ls", this, controller_handle, efi_dp_str(remaining_device_path)); @@ -201,8 +201,8 @@ static efi_status_t EFIAPI efi_uc_stop( efi_status_t ret; efi_uintn_t count; struct efi_open_protocol_info_entry *entry_buffer; - struct efi_driver_binding_extended_protocol *bp = - (struct efi_driver_binding_extended_protocol *)this; + struct efi_driver_binding_extended_protocol *bp = container_of(this, + struct efi_driver_binding_extended_protocol, bp); EFI_ENTRY("%p, %p, %zu, %p", this, controller_handle, number_of_children, child_handle_buffer); @@ -283,7 +283,7 @@ static efi_status_t efi_add_driver(struct driver *drv) } bp->bp.image_handle = bp->bp.driver_binding_handle; ret = efi_add_protocol(bp->bp.driver_binding_handle, - &efi_guid_driver_binding_protocol, bp); + &efi_guid_driver_binding_protocol, &bp->bp); if (ret != EFI_SUCCESS) { efi_delete_handle(bp->bp.driver_binding_handle); free(bp);
struct efi_driver_binding_protocol, i.e. bp, is not the first element in struct efi_driver_binding_extended_protocol. So the casting: struct efi_driver_binding_extended_protocol *bp = (struct efi_driver_binding_extended_protocol *)this; is simply wrong. Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> --- lib/efi_driver/efi_uclass.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) -- 2.33.0