From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27445 invoked by alias); 7 Jan 2008 16:20:34 -0000 Received: (qmail 27432 invoked by uid 22791); 7 Jan 2008 16:20:33 -0000 X-Spam-Check-By: sourceware.org Received: from igw2.br.ibm.com (HELO igw2.br.ibm.com) (32.104.18.25) by sourceware.org (qpsmtpd/0.31) with ESMTP; Mon, 07 Jan 2008 16:20:08 +0000 Received: from mailhub1.br.ibm.com (mailhub1 [9.18.232.109]) by igw2.br.ibm.com (Postfix) with ESMTP id B470D17F49D for ; Mon, 7 Jan 2008 14:14:30 -0200 (BRDT) Received: from d24av01.br.ibm.com (d24av01.br.ibm.com [9.18.232.46]) by mailhub1.br.ibm.com (8.13.8/8.13.8/NCO v8.7) with ESMTP id m07GK1fp4178028 for ; Mon, 7 Jan 2008 14:20:01 -0200 Received: from d24av01.br.ibm.com (loopback [127.0.0.1]) by d24av01.br.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id m07GK1NR029316 for ; Mon, 7 Jan 2008 14:20:01 -0200 Received: from [9.18.238.251] (dyn532128.br.ibm.com [9.18.238.251]) by d24av01.br.ibm.com (8.12.11.20060308/8.12.11) with ESMTP id m07GJx06029256; Mon, 7 Jan 2008 14:19:59 -0200 Subject: Re: [patch 2/2] Wrap-up expression support for DFP. From: Thiago Jung Bauermann To: Eli Zaretskii Cc: gdb-patches@sourceware.org In-Reply-To: References: <20071220054926.148275471@br.ibm.com> <20071220055107.194393592@br.ibm.com> <1198705277.12907.39.camel@localhost.localdomain> <1199304046.12907.77.camel@localhost.localdomain> Content-Type: text/plain; charset=utf-8 Date: Mon, 07 Jan 2008 16:20:00 -0000 Message-Id: <1199722798.5586.16.camel@localhost.localdomain> Mime-Version: 1.0 X-Mailer: Evolution 2.12.2 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-01/txt/msg00112.txt.bz2 On Sat, 2008-01-05 at 13:30 +0200, Eli Zaretskii wrote: > > Is this version ok? > > Yes, but. I took a look on the error messages you added, and found a > few that need some fixing: Ok. > > + 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. Yes, that's what the libiberty implementation says. The Linux manpage is somewhat vague, though: "If memory allocation wasn’t possible, or some other error occurs, these functions will return -1". The glibc implementation returns -1 for memory errors, or the return value of vfprintf, the manpage of which says that "If an output error is encountered, a negative value is returned.". So it seems that these functions don't let the caller know exactly what led them to failure. But it seems unlikely that an output error would happen when writing to a string, so I guess we could assume that negative return values are memory allocation errors. Then the error message could be changed to: error (_("Error in memory allocation for conversion to decimal float.")); > > > + 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? Actually, it turns out this else block is unreachable in the current code. All callers of value_args_as_decimal check if the types are either decfloat or integral before calling it. I'm not sure if it's better to leave the else part just in case the code changes, or remove it. If the message is left there, it can be changed to: error (_("Don't know how to convert from %s to %s."), TYPE_NAME (type1), TYPE_NAME (type2)); > > + 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. I thought "Integer-only operation" already told the user that it's not allowed on floating point number. But I changed it to: error (_("Operation not valid for decimal floating point number.")); What do you think of the above? > > > 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. -- []'s Thiago Jung Bauermann Software Engineer IBM Linux Technology Center