From mboxrd@z Thu Jan  1 00:00:00 1970
Content-Type: multipart/mixed; boundary="===============6186522051248968289=="
MIME-Version: 1.0
From: Mark Wielaard <mjw@redhat.com>
To: elfutils-devel@lists.fedorahosted.org
Subject: Re: [PATCH 1/2] Add is_executable to Dwfl_Module.
Date: Thu, 18 Sep 2014 16:56:59 +0200
Message-ID: <1411052219.27706.7.camel@bordewijk.wildebeest.org>
In-Reply-To: 20140911190049.GA1901@host2.jankratochvil.net

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

On Thu, 2014-09-11 at 21:00 +0200, Jan Kratochvil wrote:
> On Wed, 10 Sep 2014 22:15:42 +0200, Mark Wielaard wrote:
> > So I propose a cleanup like the attached first.
> =

> While I find that as an improvement in general IMO on top of your patch t=
he
> changes could be done a bit differently.

Clever. You create another struct in the union for just those fields
that do overlap. That does indeed solve the issue of documenting what
can/cannot be directly accessed. But I am afraid that might be a little
too subtle. You are kind of back to the original issue that you have to
check why this is allowed. I like the direct approach better, where the
union accesses are only done when the type is explicitly known.

Also we shouldn't add such a partial type struct to the public gelf.h
header since that is kind of a shared standard type, so if other
libelf/gelf implementations don't support it we shouldn't export it
IMHO.

Cheers,

Mark

--===============6186522051248968289==--