From mboxrd@z Thu Jan  1 00:00:00 1970
Content-Type: multipart/mixed; boundary="===============4631187996243371387=="
MIME-Version: 1.0
From: Roland McGrath <roland@hack.frob.com>
To: elfutils-devel@lists.fedorahosted.org
Subject: Re: [PATCH] libdw: Add dwarf_peel_type. Use it in
 dwarf_aggregate_size.
Date: Fri, 10 Oct 2014 11:42:50 -0700
Message-ID: <20141010184250.21AB62C3AE0@topped-with-meat.com>
In-Reply-To: 1412627360-24629-1-git-send-email-mjw@redhat.com

--===============4631187996243371387==
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable

The description in your prologue and the libdw.h comment do not match what
the code actually does.  You describe it as going until it hits one of the
known categories of actual type.  But what it actually does is go only as
long as it is seeing one of the known categories of wrapper type.  I'm not
really sure which it should do (or if it should instead do something
different from either), but the description should match the behavior.

As described, it would fall down if a new tag were introduced for a new
category of actual type.  As implemented, it would fall down if a new tag
were introduced for a new category of wrapper type.

A third potential approach would be to attempt future-proofing for those
cases.  That is, just keep going as long as there is a DW_AT_type
attribute.  But that would need a special case for DW_TAG_pointer_type and
DW_TAG_reference_type, where you want to stop even though it has a
DW_AT_type referring to another type.  I can't think of any other cases
where an "actual" type has a DW_AT_type, but there might well be some.  If
any new tag were like DW_TAG_pointer_type rather than like
DW_TAG_const_type et al, then this approach would fall down there.

So firstly we need to decide which kinds of future addition we expect and
thus how to handle the future-proofing.  Then we need to have descriptions
and implementation that match.


Thanks,
Roland

--===============4631187996243371387==--