From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6993 invoked by alias); 27 Aug 2012 08:10:28 -0000 Received: (qmail 6972 invoked by uid 22791); 27 Aug 2012 08:10:25 -0000 X-SWARE-Spam-Status: No, hits=-7.6 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,MAY_BE_FORGED,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD,SPF_HELO_PASS 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; Mon, 27 Aug 2012 08:10:07 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q7R8A6RG008572 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 27 Aug 2012 04:10:06 -0400 Received: from spoyarek (dhcp223-8.pnq.redhat.com [10.65.223.8] (may be forged)) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q7R8A4vT027919; Mon, 27 Aug 2012 04:10:05 -0400 Date: Mon, 27 Aug 2012 08:10:00 -0000 From: Siddhesh Poyarekar To: Jan Kratochvil Cc: gdb-patches@sourceware.org Subject: Re: bitpos expansion patches summary Message-ID: <20120827133927.2356a430@spoyarek> In-Reply-To: <20120826174335.GA23917@host2.jankratochvil.net> References: <20120805005350.150e5b74@spoyarek> <20120826174335.GA23917@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/msg00790.txt.bz2 On Sun, 26 Aug 2012 19:43:35 +0200, Jan wrote: > (i386-nat.c:613): > FUNC(i386_length_and_rw_bits): (LONGEST to int) [len] > - To be re-checked with updated watchpoints patch. This is safe, since ok_for_watchpoint will restrict the incoming LEN in insert_watchpoint. > WPREVERTED(Likewise): (i386-nat.c:647): > FUNC(i386_length_and_rw_bits): (LONGEST to int) [len] > - To be re-checked with updated watchpoints patch. Likewise for remove_watchpoint. > SAFE: > (ia64-tdep.c:3299): VARINIT(n): (ULONGEST to > int) [(type)->length / (float_elt_type)->length] > - You did expand n in ia64_extract_return_value, it can be array of > floats. I did not investigate what happens if the array of floats is > too large and it does not fit into ia64 registers. But I find safe > to just expand it. OK, I misread the HFA definition as an array/struct that *only* has a float. I think I confused this with some other function that has this definition. > REVERTED(siginfo type should fit into > size_t): (infrun.c:6677): FUNC(xmalloc): (ULONGEST to > size_t) [len] > - I agree. So why have you changed 'size_t->ULONGEST len = > TYPE_LENGTH (type);' ? That change should be therefore reverted. Right, will revert. > FIXED(Expand param for get_frame_register_bytes): > (m68k-tdep.c:225): FUNC(get_frame_register_bytes): > (ULONGEST to int) [(type)->length] > - I am not against expanding LEN of get_frame_register_bytes, it has > no use but it is a good sanity check instead of checking all the > callers. But in this case BTW it is not needed, only TYPE_CODE_FLT > can be passed to get_frame_register_bytes, TYPE_CODE_FLT is always > small enough. Yes, you had suggested this expansion as a sanity check in an earlier review. > FIXED(Expand to size_t)|ENSURED_SIZET: (m88k-tdep.c:311): > VARINIT(len): (ULONGEST to int) [(type)->length] > - ENSURED_SIZET is redundant, there is above already: > const bfd_byte *valbuf = value_contents (args[i]); Yes, all of the ENSURED_SIZET for -tdep have been removed for similar reasons. > SAFE: > (mdebugread.c:1238): ASSIGN: (bfd_vma to > LONGEST) [ ((*f).loc.bitpos) = (sh->value)] > - Not so clear to me, --disable-64-bit-bfd cannot support >=2GB > inferiors but it should support >256MB structs. Still when it also > affects only STABS I do not think it needs a fix. Either ways, the point is that sizeof (bfd_vma) would always be less or equal to than sizeof (LONGEST) and hence this ought to be safe. Regards, Siddhesh