From: Mike Frysinger <vapier@gentoo.org>
To: Weimin Pan <weimin.pan@oracle.com>
Cc: binutils@sourceware.org
Subject: Re: [PATCH, V3 08/15] unwinder: generate backtrace using SFrame format
Date: Wed, 2 Nov 2022 20:45:59 +0545 [thread overview]
Message-ID: <Y2KGK4hkIofjNeIf@vapier> (raw)
In-Reply-To: <34f06d43-3db9-b16d-95e4-805c24ad258a@oracle.com>
[-- Attachment #1: Type: text/plain, Size: 1557 bytes --]
On 01 Nov 2022 15:36, Weimin Pan via Binutils wrote:
> On 10/30/2022 7:03 AM, Mike Frysinger via Binutils wrote:
> > On 30 Oct 2022 00:44, Indu Bhagat via Binutils wrote:
> >> +static void
> >> +sframe_unwind (struct sframe_unwind_info *sf, void **ra_lst,
> >> + int *ra_size, int *errp)
> >> +{
> >> ...
> >> +#ifdef __x86_64__
> > this really needs better delegation for porting than inlined in the middle
> > of a large portable file. can you at least factor it out into a header ?
>
> Sorry, not sure I follow this comment. Would you please elaborate on
> "factoring it out into a header"?
common code like this should not be stuffed with arch-specific checks and
implementations. it makes it hard for more ports to happen, and for code
to be checked/validated.
further, as-written, this code can only be run on a system with a matching
runtime architecture. normally a library that is meant for processing of
files is host-agnostic. that's like the whole design of libbfd and other
tools in the binutils project. i don't know how important that use case
is to this particular bit of code though.
to at least address the code health, create an API between the common code
and the arch-specific code, and then add arch-specific files that implement
that API. it could be a header of small inline functions.
for the host-agnostic part, i haven't really read the library API to make
suggestions for how to address it. libbfd takes objects/arches as arguments
and then operates according to those.
-mike
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2022-11-02 16:15 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-30 7:44 [PATCH,V3 00/15] Definition and support for SFrame unwind format Indu Bhagat
2022-10-30 7:44 ` [PATCH,V3 01/15] sframe.h: Add SFrame format definition Indu Bhagat
2022-10-30 7:44 ` [PATCH,V3 02/15] gas: add new command line option --gsframe Indu Bhagat
2022-10-30 7:44 ` [PATCH,V3 03/15] gas: generate .sframe from CFI directives Indu Bhagat
2022-10-30 8:03 ` Mike Frysinger
2022-11-06 5:30 ` Indu Bhagat
2022-11-06 5:36 ` [PATCH,V3.1 " Indu Bhagat
2022-10-30 7:44 ` [PATCH,V3 04/15] gas: testsuite: add new tests for SFrame unwind info Indu Bhagat
2022-10-30 7:44 ` [PATCH,V3 05/15] libsframe: add the SFrame library Indu Bhagat
2022-10-30 7:44 ` [PATCH,V3 06/15] bfd: linker: merge .sframe sections Indu Bhagat
2022-10-30 13:33 ` Mike Frysinger
2022-10-30 7:44 ` [PATCH,V3 07/15] readelf/objdump: support for SFrame section Indu Bhagat
2022-10-30 7:44 ` [PATCH,V3 08/15] unwinder: generate backtrace using SFrame format Indu Bhagat
2022-10-30 14:03 ` Mike Frysinger
2022-11-01 22:36 ` [PATCH, V3 " Weimin Pan
2022-11-02 15:00 ` Mike Frysinger [this message]
2022-11-02 6:23 ` [PATCH,V3 " Indu Bhagat
2022-10-30 7:44 ` [PATCH,V3 09/15] unwinder: Add SFrame unwinder tests Indu Bhagat
2022-10-30 14:14 ` Mike Frysinger
2022-10-30 7:44 ` [PATCH,V3 10/15] gdb: sim: buildsystem changes to accommodate libsframe Indu Bhagat
2022-10-30 14:15 ` [PATCH, V3 " Mike Frysinger
2022-10-31 20:39 ` Indu Bhagat
2022-11-02 15:02 ` Mike Frysinger
2022-11-02 19:11 ` Jose E. Marchesi
2022-11-03 15:27 ` Mike Frysinger
2022-11-04 12:14 ` Jose E. Marchesi
2022-10-30 7:44 ` [PATCH,V3 11/15] libctf: add libsframe to LDFLAGS and LIBS Indu Bhagat
2022-10-30 13:27 ` Mike Frysinger
2022-10-31 20:06 ` Indu Bhagat
2022-11-01 18:57 ` Nick Alcock
2022-11-01 21:42 ` Andreas Schwab
2022-11-02 13:16 ` Nick Alcock
2022-11-04 19:02 ` [PATCH,V3.1,11/15] " Indu Bhagat
2022-11-04 21:01 ` [PATCH,V3.2,11/15] " Indu Bhagat
2022-11-05 9:21 ` Andreas Schwab
2022-11-07 22:28 ` [PATCH,V3.3 11/15] " Indu Bhagat
2022-11-08 3:26 ` Mike Frysinger
2022-11-08 19:26 ` Indu Bhagat
2022-10-30 7:44 ` [PATCH,V3 12/15] src-release.sh: Add libsframe Indu Bhagat
2022-10-30 7:44 ` [PATCH,V3 13/15] binutils/NEWS: add text for SFrame support Indu Bhagat
2022-10-30 7:44 ` [PATCH,V3 14/15] gas/NEWS: add text about new command line option and " Indu Bhagat
2022-10-30 8:05 ` [PATCH, V3 " Mike Frysinger
2022-10-31 23:17 ` Indu Bhagat
2022-10-30 7:44 ` [PATCH,V3 15/15] doc: add SFrame spec file Indu Bhagat
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=Y2KGK4hkIofjNeIf@vapier \
--to=vapier@gentoo.org \
--cc=binutils@sourceware.org \
--cc=weimin.pan@oracle.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).