From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4462 invoked by alias); 10 Aug 2012 01:44:24 -0000 Received: (qmail 4448 invoked by uid 22791); 10 Aug 2012 01:44:23 -0000 X-SWARE-Spam-Status: No, hits=-7.6 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,SPF_HELO_PASS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 10 Aug 2012 01:44:07 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q7A1i7iq025454 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 9 Aug 2012 21:44:07 -0400 Received: from spoyarek (vpn-238-210.phx2.redhat.com [10.3.238.210]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q7A1i4LO012049; Thu, 9 Aug 2012 21:44:06 -0400 Date: Fri, 10 Aug 2012 01:44:00 -0000 From: Siddhesh Poyarekar To: Jan Kratochvil Cc: gdb-patches@sourceware.org Subject: Re: bitpos expansion patches summary Message-ID: <20120810071338.15d68ab0@spoyarek> In-Reply-To: <20120809200356.GA11680@host2.jankratochvil.net> References: <20120805005350.150e5b74@spoyarek> <20120809200356.GA11680@host2.jankratochvil.net> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit 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: 2012-08/txt/msg00300.txt.bz2 On Thu, 9 Aug 2012 22:03:56 +0200, Jan wrote: > +void > +ensure_type_fits_sizet (const struct type *type) > + > +Make it ensure_type_fits_size_t. And there should be 'error' in its > name. +For example 'type_fits_size_t_or_error' or something like that. OK, will make this type_fits_size_t_or_error. > +{ > + if (!TYPE_LENGTH_FITS_SIZET (type)) > + error (_("Target object too large for host GDB memory.")); > + > +Please print the sizes. Also this message is present at multiple > places so +maybe rather make a function for unconditionally printing > the error? + Could you please give an example of this? I didn't think that there are any such checks in the source yet. > +/* Make sure that TYPE_LENGTH fits into a size_t. */ > +#define TYPE_LENGTH_FITS_SIZET(thistype) ((size_t) TYPE_LENGTH > (thistype) \ > + == TYPE_LENGTH (thistype)) > + > +Make it TYPE_LENGTH_FITS_SIZE_T, please. And I think this macro is > not needed, +inline it. (It does not access internal fields of the > type structures.) + I had kept it for possible future need if someone wants to only check if a type fits in and not throw an error. I will inline it. > +And (a) check it already for ssize_t. Because the code is not safe > enough to +properly handle unsigned sizes everywhere without > overflows. (b) Make there +some reserve, anything close to ssize_t > will never get successfully xmalloc-ed +anyway. Some maximum size > could be: ((size_t) -1) / 4. Depending on SSIZE_MAX +may not be > compatible enough I guess. + I had thought of that, but I figured that instead of guessing a value, I would be better off only doing the size_t check (i.e. code sanity) and leave the question of whether a type gets successfully malloc'd or not to the OS. For ssize_t, I could add an extra boolean argument to type_fits_size_t_or_error that indicates whether the type is signed or not. Regards, Siddhesh