Hi Aaron,

On Fri, 2019-11-15 at 12:03 -0500, Aaron Merey wrote:
> On Fri, Nov 15, 2019 at 11:16 AM Mark Wielaard <m...@klomp.org> wrote:
> > On Mon, 2019-11-04 at 16:48 -0500, Frank Ch. Eigler wrote:
> > > At the wise counsel of gdb folks such as <tromey> and <simark>:
> > > 
> > >     debuginfod 3/3: client interruptability
> > > 
> > >     For interactive clients such as gdb, interruptibility is important for
> > >     usability during longer downloads.  This patchset adds a
> > >     download-progress callback function to the debuginfod client library,
> > >     with which a caller app can interrupt a download as well as be
> > >     notified of its quantitative progress.
> > 
> > I have some concerns about this. It cannot be associated with a
> > particular debuginfod-find request.
> 
> Good point, maybe we should add a progress function to debuginfo-find,
> or include a default progress function with libdebuginfod. Maybe one
> that prints the url where the target was found plus the download
> completion percentage every few seconds.
> 
> > IMHO it should have an extra argument to indicate which build-id
> > and request type exec, debug or source (+path) the progress/callback
> > was for.
> 
> I think we could add this information to the progress function output
> without changing the API. But if users would like more control over
> how this information gets used in the progress func maybe it's
> worth considering.
> 
> > Also this one global progress/interruptable hook. Which means it
> > cannot be used by e.g. libdw and an application that uses both
> > libdw and find-debuginfo directly.
> 
> I wonder how often this kind of situation will come up. If it's very
> infrequent maybe it's enough to simply flag this issue in the docs
> so users know to avoid it.

I think it is pretty common these days. People reuse libraries, which
use libraries themselves and more and more programs will use multiple
threads of execution.

IMHO it would be best to avoid any global state from the start. Since
we haven't released this api yet we can make it so that it is easy to
have state per request object. In the gdb thread 
https://sourceware.org/ml/gdb-patches/2019-11/msg00118.html Simon
Marchi suggested something like:

    debuginfod_client *client = debuginfod_create_client ();
    debuginfod_set_progressfn (client, my_progress_cb);

With debuginfod_client * being an opaque pointer. I think that is a
good idea. It also matches what libelf (with Elf *) and libdw (with
Dwarf *) do. It also makes it easy to add other kinds of state to
client requests later.

Cheers,

Mark

Reply via email to