From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 42328 invoked by alias); 24 Jul 2015 11:26:49 -0000 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 Received: (qmail 42310 invoked by uid 89); 24 Jul 2015 11:26:48 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Fri, 24 Jul 2015 11:26:47 +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 (Postfix) with ESMTPS id 238FC9C0B6; Fri, 24 Jul 2015 11:26:46 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t6OBQix6031208; Fri, 24 Jul 2015 07:26:45 -0400 Message-ID: <55B220F4.60706@redhat.com> Date: Fri, 24 Jul 2015 11:26:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Simon Marchi , gdb-patches@sourceware.org Subject: Re: [PATCH 1/5] Update comment for struct type's length field, introduce type_length_units References: <1437072684-26565-1-git-send-email-simon.marchi@ericsson.com> In-Reply-To: <1437072684-26565-1-git-send-email-simon.marchi@ericsson.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2015-07/txt/msg00689.txt.bz2 On 07/16/2015 07:51 PM, Simon Marchi wrote: > This patch tries to clean up a bit the blur around the length field in > struct type, regarding its use with architectures with non-8-bits > addressable memory. It clarifies that the field is expressed in bytes, > which is what is the closest to the current reality. > > It also introduces a new function to get the length of the type in > addressable memory units. > LGTM, with: > gdb/ChangeLog: > > * gdbtypes.c (type_length_units): New function. > * gdbtypes.h (type_length_units): New declaration. > (struct type): Update LENGTH's comment. Write: (struct type) : Update comment. > + > /* Alloc a new type instance structure, fill it with some defaults, > and point it at OLDTYPE. Allocate the new type instance from the > same place as OLDTYPE. */ > diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h > index c166e48..83f85a6 100644 > --- a/gdb/gdbtypes.h > +++ b/gdb/gdbtypes.h > @@ -780,31 +780,23 @@ struct type > check_typedef. */ > int instance_flags; > > - /* * Length of storage for a value of this type. This is what > - sizeof(type) would return; use it for address arithmetic, memory > - reads and writes, etc. This size includes padding. For example, > - an i386 extended-precision floating point value really only > - occupies ten bytes, but most ABI's declare its size to be 12 > - bytes, to preserve alignment. A `struct type' representing such > - a floating-point type would have a `length' value of 12, even > - though the last two bytes are unused. > - > - There's a bit of a host/target mess here, if you're concerned > - about machines whose bytes aren't eight bits long, or who don't > - have byte-addressed memory. Various places pass this to memcpy > - and such, meaning it must be in units of host bytes. Various > - other places expect they can calculate addresses by adding it > - and such, meaning it must be in units of target bytes. For > - some DSP targets, in which HOST_CHAR_BIT will (presumably) be 8 > - and TARGET_CHAR_BIT will be (say) 32, this is a problem. > - > - One fix would be to make this field in bits (requiring that it > - always be a multiple of HOST_CHAR_BIT and TARGET_CHAR_BIT) --- > - the other choice would be to make it consistently in units of > - HOST_CHAR_BIT. However, this would still fail to address > - machines based on a ternary or decimal representation. */ > + /* * Length of storage for a value of this type. The value is the > + expression in bytes of of what sizeof(type) would return. This Double "of of". Please say "host bytes" to make this super clear. > + size includes padding. For example, an i386 extended-precision > + floating point value really only occupies ten bytes, but most > + ABI's declare its size to be 12 bytes, to preserve alignment. > + A `struct type' representing such a floating-point type would > + have a `length' value of 12, even though the last two bytes are > + unused. > + > + Since this field is expressed in bytes, its value is appropriate to Likewise, "host bytes". > + pass to memcpy and such (it is assumed that GDB itself always runs > + on an 8-bits addressable architecture). However, when using it for > + target address arithmetic (e.g. adding it to a target address), the > + type_length_units function should be used in order to get the length > + expressed in addressable memory units. */ "target addressable memory units" while at it. Likewise in the other patches. > > - unsigned length; > + unsigned int length; Thanks, Pedro Alves