Re: [patch 1/4] libdecnumber support

[Daniel Jacobowitz <drow at false dot org>]

I'll review all these now.  Before you check them in, though, give
me a chance to get libdecnumber merged into the src repository.  It
shouldn't take long.

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.

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

> +/* 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.  Also we never use malloc, only xmalloc.  Or you could avoid
the allocation by using gdb_byte dec[16].

> +        break;
> +      default:
> +	free (dec);

Spaces and tabs mixed up?  xfree, like xmalloc.

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

Space before parentheses please.

> +/* 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.

> +#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.

-- 
Daniel Jacobowitz
CodeSourcery