On Thu, 2007-10-11 at 11:54 -0400, Daniel Jacobowitz wrote: 
> I'll review all these now.

Thank you very much.

> Before you check them in, though, give
> me a chance to get libdecnumber merged into the src repository.  It
> shouldn't take long.

Thanks for taking care of this.

> On Thu, Sep 20, 2007 at 06:54:11PM -0300, Thiago Jung Bauermann wrote:
> > +#include <ctype.h>
> > +#include <endian.h>
> > +#include "defs.h"
> > +#include "dfp.h"
> 
> What do you need ctype.h for?  That will tell me whether you really
> want ctype.h or libiberty's locale-independent safe-ctype.h.
> 
> defs.h should always be first.

It turned out that ctype.h was not needed at all. I just removed it and
it made no difference. Probably a left-over from an older version of the
code.

> endian.h is not portable.  You should probably use the autoconf macro
> AC_C_BIGENDIAN.

Ok, I'm using it now. When I run autoreconf inside the src/gdb
directory, I get the following:

$ autoreconf
autoreconf: cannot create `cd $srcdir;pwd`/../../../..: No such file or
directory
autoreconf: cannot create `cd $srcdir;pwd`/../../..: No such file or
directory
autoreconf: cannot create `cd $srcdir;pwd`/..: No such file or directory
$ echo $?
0

Is this a problem? Things still seem to work fine. I used autoconf
version 2.61.

> > +/* Convert decimal type to its string representation.  LEN is the length
> > +   of the decimal type, 4 bytes for decimal32, 8 bytes for decimal64 and
> > +   16 bytes for decimal128.  */
> > +void
> > +decimal_to_string (const uint8_t *decbytes, int len, char *s)
> > +{
> > +  uint8_t *dec = (uint8_t *)malloc (len);
> 
> You didn't include anything that would give you uint8_t; use
> gdb_byte.

I included dfp.h, which included stdint.h. I see now that GDB doesn't
use C99 yet, so I changed to gdb_byte.

> Also we never use malloc, only xmalloc.  Or you could avoid
> the allocation by using gdb_byte dec[16].

Good idea. Just changed to use temporary array as suggested.

> > +        break;
> > +      default:
> > +	free (dec);
> 
> Spaces and tabs mixed up?  xfree, like xmalloc.

Fixed. No '\t' in the file now. By the way, what's the policy in GDB
w.r.t. tabs and spaces? Never use tabs?

> > +	error(_("Unknown decimal floating point type.\n"));
> 
> Space before parentheses please.

Fixed.

> > +/* There is a project intended to add DFP support into GCC, described in
> > +   http://gcc.gnu.org/wiki/Decimal%20Floating-Point.  This file is intended
> > +   to add DFP support into GDB.  */
> 
> This comment doesn't belong here.  The file is part of GDB once the
> patch is applied.

Ok, removed comment.

> > +#ifndef DFP_H
> > +#define DFP_H
> > +#include <string.h>
> > +#include <stdio.h>
> > +#include <stdint.h>
> 
> Don't include standard headers here; stick to what defs.h provides.
> Just as well, since neither string.h nor stdint.h is portable.

Ok, removed.
-- 
[]'s
Thiago Jung Bauermann
Software Engineer
IBM Linux Technology Center