Re: [PATCH v4 08/13] error: Use %p format code for pointer types

[Daniel Kiper]

On Sun, Feb 28, 2021 at 02:02:20PM -0600, Glenn Washburn wrote:
> On Sat, 27 Feb 2021 12:43:06 +0100
> Daniel Kiper <dkiper@net-space.pl> wrote:
>
> > On Thu, Feb 18, 2021 at 08:47:09PM -0600, Glenn Washburn wrote:
> > > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > > ---
> > >  grub-core/loader/i386/pc/linux.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/grub-core/loader/i386/pc/linux.c
> > > b/grub-core/loader/i386/pc/linux.c index 814988ab9..0bc5d6807 100644
> > > --- a/grub-core/loader/i386/pc/linux.c
> > > +++ b/grub-core/loader/i386/pc/linux.c
> > > @@ -230,9 +230,9 @@ grub_cmd_linux (grub_command_t cmd
> > > __attribute__ ((unused)), && GRUB_LINUX_ZIMAGE_ADDR +
> > > grub_linux16_prot_size
> > >        > grub_linux_real_target)
> > >      {
> > > -      grub_error (GRUB_ERR_BAD_OS, "too big zImage (0x%x > 0x%x),
> > > use bzImage instead",
> > > +      grub_error (GRUB_ERR_BAD_OS, "too big zImage (%p > %p), use
> > > bzImage instead", (char *) GRUB_LINUX_ZIMAGE_ADDR +
> > > grub_linux16_prot_size,
> > > -           (grub_size_t) grub_linux_real_target);
> > > +           (void *) grub_linux_real_target);
> >
> > I would use PRIxGRUB_SIZE and PRIxGRUB_ADDR respectively.
>
> IIRC, The reason I chose the %p format string was because the compiler
> considers it an error to use a pointer for an integer format string

If you look a bit above you can see:
  static grub_size_t grub_linux16_prot_size;
  static grub_addr_t grub_linux_real_target;

So, they are not pointers.

> code. Does it make more sense to have both be PRIxGRUB_ADDR and for
> PRIxGRUB_ADDR to be defined as "p"?

Nope.

> Since they are both pointers, I'm not following why the code should be

No, they are not.

> semantically different (ie PRIxGRUB_SIZE and PRIxGRUB_ADDR, instead of
> just one of those). If you do want to use a "*x" format code, I think
> we'd need to cast those arguments.

If you drop (void/char *) casting the you should be able to use
PRIxGRUB_SIZE and PRIxGRUB_ADDR without any issues.

Daniel