bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH][gnulib] Add the Sframe package


From: Bruno Haible
Subject: Re: [PATCH][gnulib] Add the Sframe package
Date: Wed, 16 Nov 2022 01:52:20 +0100

Hello Weimin,

> The main use-case for this format are
> "online" debugging tools like stack tracers

I'll appreciate anything that can help producing a universally working
backtrace for C, since experience (e.g. from Lisp, Java, Python) has
shown that such a backtrace function can tremendously improve the
developer productivity and the usefulness of bug reports from anonymous
users.

My experiments on this topic, two years ago, were not satisfactory:
  * The backtrace function from glibc
      - produces no file names, no line numbers,
      - produces function names only when the binary was linked with
        '-rdynamic',
      - is not portable.
  * The libbacktrace library produced hardly useful information as well, even
    in simple cases. See [1].

Can you describe how the SFrame format fits in, w.r.t. to
  - backtrace from glibc (based on libunwind AFAIK),
  - libbacktrace ?

> This patch adds two new modules to gnulib, related to the Simple Frame
> format (SFrame) recently introduced in binutils [1].

I'm pleased to see that it is extensively commented, includes sanity checks,
and is already in GNU coding style.

Since this is a significant contribution (regardless whether it finally
goes into Gnulib [2] or into Binutils [3]), we will need a copyright assignment
to the FSF for this code. Are you already aware how this works, in your context
as an Oracle employee? Maybe José Marchesi can share his experience on this
topic with you?

Once the big picture is clear and the copyright issue is handled, we'll
go into the detailed code review. For now, what I can see are only nits,
due to Gnulib conventions:
  - The header file module should better be named 'sframe-h' instead of
    'sframe-header'.
  - The *.c and *.h files should be untabified (run 'expand -t 8' on each).
  - tests/sframe-api.h should not be included since it's a copy of
    lib/sframe-api.h, right?
  - tests/DATA* should better be moved to a subdirectory:
    tests/sframe-data/DATA*, to make it clear to which part of Gnulib it
    belongs.

Bruno

[1] https://github.com/ianlancetaylor/libbacktrace/issues
[2] https://www.gnu.org/software/gnulib/manual/html_node/Copyright.html
[3] 
https://sourceware.org/gdb/wiki/ContributionChecklist#FSF_copyright_Assignment






reply via email to

[Prev in Thread] Current Thread [Next in Thread]