Re: [PATCH] efi/tpm.c: Add missing casts

[Daniel Kiper]

On Mon, Mar 25, 2019 at 03:27:11PM +0100, Vladimir 'phcoder' Serbinenko wrote:
> On Mon, Mar 25, 2019 at 1:09 PM Daniel Kiper <address@hidden> wrote:
> >
> > On Fri, Mar 22, 2019 at 04:11:10PM +0100, Vladimir 'phcoder' Serbinenko 
> > wrote:
> > > From 9443713c3d3403a21aac9b39de4bbd097a531dd4 Mon Sep 17 00:00:00 2001
> > > From: Vladimir Serbinenko <address@hidden>
> > > Date: Fri, 22 Mar 2019 16:07:47 +0100
> > > Subject: [PATCH 2/2] efi/tpm.c: Add missing casts
> >
> > Have you seen any warnings without this patch?
> >
> Yes, That we make either incompatible pointer type or integer from pointer.

Please say it in the commit message.

> > > ---
> > >  grub-core/commands/efi/tpm.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/grub-core/commands/efi/tpm.c b/grub-core/commands/efi/tpm.c
> > > index 7475fd87b..32909c192 100644
> > > --- a/grub-core/commands/efi/tpm.c
> > > +++ b/grub-core/commands/efi/tpm.c
> > > @@ -245,7 +245,7 @@ grub_tpm1_log_event (grub_efi_handle_t tpm_handle, 
> > > unsigned char *buf,
> > >    grub_memcpy (event->Event, description, event->EventSize);
> > >
> > >    algorithm = TCG_ALG_SHA;
> > > -  status = efi_call_7 (tpm->log_extend_event, tpm, buf, (grub_uint64_t) 
> > > size,
> > > +  status = efi_call_7 (tpm->log_extend_event, tpm, (grub_addr_t) buf, 
> > > (grub_uint64_t) size,
> > >                      algorithm, event, &eventnum, &lastevent);
> > >
> > >    switch (status)
> > > @@ -295,7 +295,7 @@ grub_tpm2_log_event (grub_efi_handle_t tpm_handle, 
> > > unsigned char *buf,
> > >      sizeof (*event) - sizeof (event->Event) + grub_strlen (description) 
> > > + 1;
> > >    grub_memcpy (event->Event, description, grub_strlen (description) + 1);
> > >
> > > -  status = efi_call_5 (tpm->hash_log_extend_event, tpm, 0, buf,
> > > +  status = efi_call_5 (tpm->hash_log_extend_event, tpm, 0, (grub_addr_t) 
> > > buf,
> >
> > Should not you use grub_efi_physical_address_t instead of grub_addr_t?
> Nope. On 32-bit system we can cast the pointer only to 32-bit but
> grub_efi_physical_address_t is 64-bit. Casting pointer directly to
> 64-bit create a warning. This way we have explicit cast to addr_t and
> then implicit cat to efi_physical_address_t.

OK, if you add why to the commit message then you can add
  Reviewed-by: Daniel Kiper <address@hidden>

Daniel