Re: [patch 2/2] Wrap-up expression support for DFP.

[Eli Zaretskii <eliz at gnu dot org>]

> From: Thiago Jung Bauermann <bauerman@br.ibm.com>
> Cc: gdb-patches@sourceware.org
> Date: Wed, 02 Jan 2008 18:00:46 -0200
> 
> Fine by me. Thanks for your review, Eli. The attached patch has the
> following changes from the one I originally posted:
> 
> - includes documentation update to GDB manual, under the "C and C++"
> subsection;
> - uses asprintf instead of sprintf, with "%.30Lg" in
> decimal_from_floating;
> - uses strtod instead of atof in decimal_to_double;
> - decimal/binary FP conversion tests now accept some rounding variation:
> 
> gdb_test "p (_Decimal64) 3.1" " = 3.(0999.*|1000.*)"
> 
> 
> Is this version ok?

Yes, but.  I took a look on the error messages you added, and found a
few that need some fixing:

> +  ret = asprintf (&buffer, "%.30Lg", value_as_double (from));
> +  if (ret == -1)
> +    error (_("Error in conversion to decimal float."));

The documentation of asprintf says that -1 is returned if it fails to
allocate memory for the buffer.  So I think our error message should
say the same.  In any case, "error in conversion" is too vague to be
useful.

> +	error (_("Unknown decimal floating point operation."));

Shouldn't this be internal_error?  I mean, there couldn't be any valid
op at this point, so this is a kind-of "can't happen" situation, isn't
it?

> +    error (_("Don't know how to convert to decimal floating type."));

Wouldn't it be better to state the source type (from which we tried to
convert) here as well?

> +	  error (_("Integer-only operation on floating point number."));

Did you mean to say "Integer-only operation on floating point number
is not allowed."?  Or something else?  As a GDB user, I'd be quite
confused about the actual problem if I were to see this message.

> Index: src-git/gdb/doc/gdb.texinfo
> ===================================================================
> --- src-git.orig/gdb/doc/gdb.texinfo	2007-12-28 01:11:37.000000000 -0200
> +++ src-git/gdb/doc/gdb.texinfo	2008-01-02 14:14:48.000000000 -0200

This part of your patch is approved.

Thanks.