This is the mail archive of the elfutils-devel@sourceware.org mailing list for the elfutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 1/2] Add is_executable to Dwfl_Module.


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 the
> 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

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]