Re: gpxlogger (x: ✘3.23 is near. Please test!)

[Gary E. Miller]

Yo Fred!

On Thu, 29 Jul 2021 15:59:31 -0700 (PDT)
Fred Wright <fw@fwright.net> wrote:

> On Mon, 26 Jul 2021, Gary E. Miller wrote:
> 
> > I want to get gpsd 3.23 shipped, due to issue #144.
> >
> > Please test git head.  Unless a show-stopper bug pops up, I'd like
> > to get 3.23 shipped by Aug 2.  Version 3.22 was shipped Jan 8,
> > 2021.  
> 
> I haven't done any multi-platform testing yet, but I did find that 
> gpxlogger has become unquittable, due to a863161f8.

Well, that commit fixes a real problem.  It fixed calling a non-reentrant
funtion in a signal handler.  Let me ponder this and see
if there is a rifle shot fix to gpxlogger.  Bigger fixes should wait,
if possible.

> Previously, quit_handler() was doing things which were technically
> illegal but worked, including a gps_close() which terminated the
> gps_mainloop(). The new code seems to assume that gps_mainloop()
> returns frequently, which it doesn't, since it's designed to iterate
> internally, and the only reason for the outer loop was for
> reconnecting.  Adding the sig_flag check there doesn't help, because
> it's never reached.

Ouch.

> The real problem is the bad design of gps_mainloop(), which provides
> no way to stop its iterations other than by error or timeout.  Aside
> from the whole signal handler issue, relying on something like
> gps_close() to stop it is ugly and probably not thread-safe.  The
> obvious fix is to have the hook function return a value which
> indicates whether iterations should continue, perhaps with a
> different return value from gps_mainloop() itself to distinguish
> "stop iteration" from "error".  However, this would change the API.

Might be OK, since it is broken as is.

> Currently, the only use of gps_mainloop() within the project is in
> gpxlogger, though it's not known whether users are using it.  And
> testing the return value of a function declared void would lead to
> flaky behavior.  The conservative thing to do would be to introduce a
> new version of gps_mainloop() under a different name, and keep the
> broken one (probably a wrapper around the good one) around for a
> while for compatibility.

A good idea.

> Or a cheap fix for the impending release might be just to revert
> a863161f8 (at least the portion that applies to gpxlogger), and fix
> it properly later.

Which brings back a known bug.

>  Or skip the logging and have the signal handler
> call _exit() the way gpsctl is now doing.

Better idea.

Thanks!

RGDS
GARY
---------------------------------------------------------------------------
Gary E. Miller Rellim 109 NW Wilmington Ave., Suite E, Bend, OR 97703
        gem@rellim.com  Tel:+1 541 382 8588

            Veritas liberabit vos. -- Quid est veritas?
    "If you can't measure it, you can't improve it." - Lord Kelvin

pgpNBe0uL6ufR.pgp
Description: OpenPGP digital signature