Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Mark Kettenis <mark.kettenis@xs4all.nl>
To: brobecker@adacore.com
Cc: gdb-patches@sourceware.org
Subject: Re: [RFC] convert a host address to a string
Date: Thu, 08 Jan 2009 13:26:00 -0000	[thread overview]
Message-ID: <200901081326.n08DQEgY002357@brahms.sibelius.xs4all.nl> (raw)
In-Reply-To: <20090108101911.GQ3664@adacore.com> (message from Joel Brobecker 	on Thu, 8 Jan 2009 14:19:11 +0400)

> Date: Thu, 8 Jan 2009 14:19:11 +0400
> From: Joel Brobecker <brobecker@adacore.com>
> 
> > An option would be to use the strategy used by phex_nz() to print host
> > addresses.  Or we could use PRINTF_HAS_LONG_LONG, and always use %llx
> > if it's available.
> 
> Unfortunately, I don't know how this could be made to work.
> The problem is that GCC insists that the integer type that we
> use to cast the host address to must have the same size.
> At one point, hoping that GCC would kill the wrong branch,
> I even tried:
> 
>   if (sizeof (void *) == sizeof (long))
>     printf ("0x%lx", (long) address);
>   else
>     printf ("0x%llx", (long long) address);
> 
> But this didn't work, because GCC complained about the cast
> in the "if" branch.

Ah, GCC is being a bit too helpful here.  Damn!

> Actually, it's only after writing the entire email that I realized
> that we have another option. See option (3) below.
> 
> > I'd really like to avoid introducing another macro dealing with
> > type-size issues if possible.  I especially dislike HOST_IS_LLP64
> > since I fear its existence encourages people to write unportable code.
> 
> I can see several solutions:
> 
>   1. Use %p. To overcome the problem with 0x, we could use
>      two alternatives:
> 
>        a. Import printf from gnulib. I looked at this a while ago,
>           for some other issue, and I immediately stopped, as it
>           looked like it might be a lot of work to do so (printf
>           doesn't come alone, there's a bunch of other routines
>           that printf uses which we probably want).

I'm not very excited about this option.  And if the gnulib printf
doesn't actually implement the Microsoft-invented non-standard format
specifiers it may even cause us more grief.

>        b. Strip the leading "0x" if %p already provides it. In other
>           words:
> 
>             fprintf (buf, "0x%p", address);
>             if (buf[2] == '0' && buf[3] == 'x')
>               buf = buf + 2;
>             return buf;
> 
>           There is no memory management issue in this case, because
>           the buffer we return is more or less static. It's part
>           of a bunch of buffers we cycle through each time we call
>           this routine.  The caller never frees the memory we return.

Ugh, this is a bit ugly.  And we can't even be sure that there are
even more variations on the format that %p generates.  I wouldn't be
surprised at all if some platforms would use upper case for the hex
digits for example.

>   2. Avoid the HOST_IS_LLP64 macro, but still do something similar
>      inside host_address_to_string. Something like:
> 
>        #if defined(WIN64_)
>            fprintf (buf, "0x%llx", (unsigned long long) address);
>        #else
>            fprintf (buf, "0x%lx", (unsigned long) address);
>        #endif
> 
>      This eliminates the likeliness of re-using the HOST_IS_LLP64
>      macro to write non-portable code.

Not really excited about this one either.

>   3. Work through uintptr_t.
> 
>        #ifdef PRINTF_HAS_LONG_LONG
>          fprintf (buf, "0x%llx", (unsigned long long) (uintptr_t) address);
>        #else
>          fprintf (buf, "0x%lx", (unsigned long) (uintptr_t) address);
>        #endif

This wouldn't be the first place where we'd use a double cast in
connection with intptr_t/uintptr_t.  So I'd say that while this is a
bit ugly, it's certainly acceptable.  It's by far the simplest way to
fix things.

> I kinda like option 1b as being simple and avoiding the need to
> cast the address to an integer.  Option (3) is my next favorite,
> but I don't like the fact that we end up doing an unnecessary
> integer promotion on the 32bit targets.

I'm not really worried about the integer promotion.  Printing host
addresses is a fairly rare operation, and certainly not time critical.


  parent reply	other threads:[~2009-01-08 13:26 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-07 12:19 Joel Brobecker
2009-01-07 16:17 ` Mark Kettenis
2009-01-08 10:19   ` Joel Brobecker
2009-01-08 10:25     ` Kai Tietz
2009-01-08 10:48       ` Joel Brobecker
2009-01-08 11:02         ` Kai Tietz
2009-01-08 11:25           ` Joel Brobecker
2009-01-08 11:31             ` Kai Tietz
2009-01-08 12:49           ` Mark Kettenis
2009-01-08 12:54             ` Joel Brobecker
2009-01-08 13:04               ` Kai Tietz
2009-01-08 13:12               ` Mark Kettenis
2009-01-08 13:26     ` Mark Kettenis [this message]
2009-01-08 13:35       ` Kai Tietz
2009-01-08 13:42         ` Joel Brobecker
2009-01-08 14:04           ` Kai Tietz
2009-01-08 16:18         ` Mark Kettenis
2009-01-08 16:23           ` Kai Tietz
2009-01-09  9:57             ` Joel Brobecker
2009-01-09 10:05               ` Kai Tietz
2009-01-09 13:12       ` Joel Brobecker
2009-01-09 14:28         ` Kai Tietz
2009-01-10  7:12           ` Joel Brobecker
2009-01-10 13:31             ` Kai Tietz
2009-01-10 13:34               ` Kai Tietz
2009-01-10 13:58               ` Mark Kettenis
2009-01-10 14:04               ` Mark Kettenis
2009-01-10 14:15                 ` Kai Tietz
2009-01-10 14:22                   ` Mark Kettenis
2009-01-10 14:25                     ` Kai Tietz
2009-01-11 13:31                     ` Joel Brobecker
2009-01-11 13:53                       ` Mark Kettenis
2009-01-13 12:09                     ` Joel Brobecker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200901081326.n08DQEgY002357@brahms.sibelius.xs4all.nl \
    --to=mark.kettenis@xs4all.nl \
    --cc=brobecker@adacore.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox