|
From: | Fred Wright |
Subject: | Re: gpxlogger (x: ✘3.23 is near. Please test!) |
Date: | Sat, 31 Jul 2021 21:37:28 -0700 (PDT) |
User-agent: | Alpine 2.23 (OSX 453 2020-06-18) |
On Fri, 30 Jul 2021, Gary E. Miller wrote:
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.I think fixed in commit c9cdf7cba1bf87ae88f756b7dd63b133b12bd3f1
Partially - see below.
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.Agreed. So I put the exit in the mainloo callback. Then added an atexit() to send the gpx footer.
That works in the common case, but not in the case where the daemon isn't sending anything. I don't see any way to fix that without some change to the API, putting back the ugly gps_close() in the signal handler, or directly exiting from the signal handler (which is what it did way back when).
Needs more work, but after release.
Fixing it properly can wait, but I verified that the 3.22 version does handle this correctly, so it's indeed a regression as is.
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. 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.I added some comments that it needs to be fixed. I think the API can be saved by jsut extending it. After release.
Obviously my suggestion for the return value wouldn't work when the hook isn't getting called at all. :-) Needs more thought.
Fred Wright
[Prev in Thread] | Current Thread | [Next in Thread] |