From: Mark Wielaard <mark@klomp.org>
To: tbaeder@redhat.com, elfutils-devel@sourceware.org
Subject: Re: Remove nested functions from readelf.c
Date: Mon, 01 Feb 2021 15:21:44 +0100 [thread overview]
Message-ID: <c4c83db4dba3d489fdb748465ecbddae65c19a32.camel@klomp.org> (raw)
In-Reply-To: <20210108081633.2202592-1-tbaeder@redhat.com>
On Fri, 2021-01-08 at 09:16 +0100, Timm Bäder via Elfutils-devel wrote:
> here another round for src/readelf.c. I think they are simple, but I'm
> not happy with the advance_pc() commit. I'm open for suggestions here
> but I can't come up with something better right now. I'll keep looking
> in to that in the meantime.
Yeah, the advance_pc function is really why you want nested functions
in the first place IMHO. Passing around the captured state as 6 extra
arguments seems to not really make the code much clearer. Especially
because on its own it isn't immediately clear what the code is about or
that it is to be used as part of the special opcode or
DW_LNS_advance_pc opcode (where DW_LNS_const_add_pc acts like special
opcode 255) handling.
It might be helpful to spell out in a comment to the function which
part of the DWARF4 6.2.5.1 Special Opcodes handling the advance_pc
function is responsible for.
But honestly in this case I think just keeping the nested function is
the cleanest way to handle this.
Cheers,
Mark
next prev parent reply other threads:[~2021-02-01 14:21 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-08 8:16 tbaeder
2021-01-08 8:16 ` [PATCH 1/5] readelf: Pull add_dump_section() into file scope tbaeder
2021-01-30 19:55 ` Mark Wielaard
2021-01-08 8:16 ` [PATCH 2/5] readelf: Pull same_set() info " tbaeder
2021-01-30 20:14 ` Mark Wielaard
2021-01-08 8:16 ` [PATCH 3/5] readelf: Pull left() " tbaeder
2021-01-30 20:29 ` Mark Wielaard
2021-01-08 8:16 ` [PATCH 4/5] readelf: Pull regname() into " tbaeder
2021-01-30 20:47 ` Mark Wielaard
2021-01-08 8:16 ` [PATCH 5/5] readelf: Pull advance_pc() " tbaeder
2021-02-01 14:21 ` Mark Wielaard [this message]
2021-02-02 10:07 ` Remove nested functions from readelf.c Timm Bäder
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c4c83db4dba3d489fdb748465ecbddae65c19a32.camel@klomp.org \
--to=mark@klomp.org \
--cc=elfutils-devel@sourceware.org \
--cc=tbaeder@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).