Re: [patchv3 1/8] Move utility functions to common/

[Pedro Alves <palves at redhat dot com>]

On 02/28/2014 08:10 PM, Jan Kratochvil wrote:
> On Fri, 28 Feb 2014 13:06:22 +0100, Pedro Alves wrote:
>> On 02/27/2014 09:32 PM, Jan Kratochvil wrote:
>>> 	(hex2bin, bin2hex): Move decls from remote.h.
>>
>> This one looks stale.
> 
> Yes, removed.
> 
> 
>>> --- a/gdb/common/common-utils.h
>>> +++ b/gdb/common/common-utils.h
>>> @@ -25,6 +25,25 @@
>>>  #include <stddef.h>
>>>  #include <stdarg.h>
>>>  
>>> +/* Static target-system-dependent parameters for GDB.  */
>>> +
>>> +/* * Number of bits in a char or unsigned char for the target machine.
>>> +   Just like CHAR_BIT in <limits.h> but describes the target machine.  */
>>> +#if !defined (TARGET_CHAR_BIT)
>>> +#define TARGET_CHAR_BIT 8
>>> +#endif
>>> +
>>> +/* * If we picked up a copy of CHAR_BIT from a configuration file
>>> +   (which may get it by including <limits.h>) then use it to set
>>
>> Sounds like common-utils.h should include limits.h?  defs.h does
>> include it.
> 
> Primarily TARGET_CHAR_BIT and HOST_CHAR_BIT are obsolete,

There are gdb ports out there with TARGET_CHAR_BIT != 8.  We don't
have any in the tree currently, but I believe most of the code
we have actually works.  I wouldn't have a problem with accepting
such a port.

See "Clarification on what a byte is in -data-*-memory-bytes ?"
(https://sourceware.org/ml/gdb/2014-01/msg00046.html)
question on gdb@ just recently, about a TARGET_CHAR_BIT == 16 port.
( guess somebody should respond :-P )

It's my belief that if we accept such a port in the tree, others would
follow suit more quickly.  It's not that uncommon to see DSPs with
that word size, and my guess is that GDB ports for those aren't
submitted because the first will need to do the actual leg work
of fixing the bit rot properly.

> already C90 defines CHAR_BIT as constant 8.

Sorry, but that's not true.  I'm looking at N1256 (C99/TC3 draft),
and in 5.2.4.2.1, it shows:

  #define CHAR_BIT 8

_but_, it also says just above:

 "The minimum magnitudes shown shall be replaced by implementation-defined
  magnitudes with the same sign."

So that means it must be _at least_ 8-bit, but it also be wider than 8.

Even if that were not in the standard, there are machines out there
(not talking about ancient 9-bit machines, I'm thinking of mostly
DSPs, with 16- or 32-bit chars).  Ignoring them because of the standard
would be just burying the head in the sand.

I could see deleting HOST_CHAR_BIT, that's a different story.  I believe
_POSIX_ requires 8-bit, but I haven't checked.  A general purpose CPU
with char bit != 8-bit is indeed unthinkable to me in this day and age.

>>> +   the number of bits in a host char.  If not, use the same size
>>> +   as the target.  */
>>> +
>>> +#if defined (CHAR_BIT)
>>> +#define HOST_CHAR_BIT CHAR_BIT
>>> +#else
>>> +#define HOST_CHAR_BIT TARGET_CHAR_BIT
>>> +#endif
>>
>> I think host-defs.h might be a better home for this.  (With the
>> target version placed in a new target-defs.h perhaps.  I won't
>> insist though.)
> 
> But I am not going to create new files because of an obsolete macro which
> should be in the first place deleted.

I don't agree with deleting TARGET_CHAR_BIT though.  (I do
think it should be a gdbarch hook in gdb instead.).  host-defs.h
already exists, and this being macro representing a fundamental
characteristic of the host, it just feels better than a header
for common string utilities and such.  But *schrug*.  I don't
intend to fight further over this.  :-)  Anywhere is fine, really.

-- 
Pedro Alves