[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [gpsd-dev] Cleanup patches for the mingw port
From: |
Eric S. Raymond |
Subject: |
Re: [gpsd-dev] Cleanup patches for the mingw port |
Date: |
Thu, 16 May 2013 14:20:47 -0400 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
ukyg9e5r6k7gubiekd6 <address@hidden>:
> >0007-Remove-ugly-intptr_t-casts-from-gpsdata-gps_fd.patch
> >
> > I agree those casts are ugly, but without them one of our
> > auditing tools gets confused. Not applied, but we can discusss
> > this further.
> Which auditing tool did these confuse? Maybe we could find some
> other way to make it happy.
I think it was cppcheck.
> >0008-Replace-tests-of-value-of-gps_fd-which-thus-assumed-.patch
> >
> > Failed to apply cleanly to libgps_core.c. Please resubmit
> > against head. Actually, what I'd like is a patch that
> > does all the GOODSOCK/BADSOCK changes in one go.
> >
> I think you've now performed most or all of the GOODSOCK changes, so
> I doubt there are any direct tests of the value of gps_fd left. I'll
> perform a quick pass over the latest source to check. Do you know of
> any leftovers here?
I don't know of any leftovers.
> >0010-Silence-some-more-splint-warnings-and-use-GOOD-BAD-S.patch
> >
> > Partly applied. I omitted the BADSOCK application.
> > I wasn't seeing splint warnings from this code, which
> > makes me wonder why you were.
> I'm using splint from Debian unstable, on both i386 and amd64. What
> are you using?
3.1.2, I think.
> Understood. Yes, these are really all part of "project errno", so
> I'll lump them in with 0025 and submit a patch. As long as you are
> OK with the approach generally, that is (which it sounds like you
> are).
I am very unlikely to make incompatible changes to the library API
at this point. But I'd like to see your patch and consider it.
> >0031-More-GOOD-BADSOCK.patch
> >
> > Not applied. I want to see this as part of a consolidated
> > patch to do that change everywhere.
> Again I think you've now effectively made and applied such a
> consolidated patch (and I'm sorry I didn't do that myself). Can you
> confirm/deny?
Not easily. I've forgotten what was in that patch.
> In summary, patches remaining that I'm to submit are:
>
> 1. 0007-Remove-ugly-intptr_t-casts-from-gpsdata-gps_fd.patch
> But without introducing the audit regression(s) to be described by Eric.
>
> 2. 0008-Replace-tests-of-value-of-gps_fd-which-thus-assumed-.patch
> If there are any of them left, of course.
>
> 3. 0011-Placate-splint-by-conditionally-not-declaring-defini.patch
>
> 4. 0022-Silence-splint-and-cppcheck-mostly-by-reducing-varia.patch a
> If anything remains to be done.
>
> 5. "Project errno"
> Incorporating at least:
>
> 0025-Instead-of-passing-errno-via-gps_fd-ensure-netlib_co.patch
> 0026-Use-strerror-rather-than-a-generic-error-message.-No.patch
> 0027-Make-libgps_core.c-s-extern-functions-leave-errno-wi.patch
> 0028-Use-errno-for-diagnostics-rather-than-ab-using-socke.patch
> 0029-Leave-errno-with-a-meaningful-value-on-return.patch
>
> In case it's not obvious, my intention is to satisfy:
> 5.1. If anything wants to get at an errno value, it does so via
> errno itself, not via something else whose value we hope was set to
> errno at some sufficiently-recent time in the past; and
> 5.2. All 'interesting' functions should leave errno in a valid
> state, reflecting the success or failure of the work the function
> attempted. One day I would like it if every function left errno in a
> valid state.
> 5.3. Every piece of code that sets errno on an error, shall also set
> errno to a valid value in its success case. This way the calling
> code either always or never uses errno.
> I think rules like 'you must only look at errno in an error path'
> are extremely error-prone. Also they can lead to really dumb
> behaviour, eg schizophrenic messages like "An error occurred:
> Success".
>
> 6. 0033-Replace-uint-with-unsigned-everywhere-obviating-one-.patch
> But use 'unsigned int' rather than 'unsigned'. C source is not ASCII art :-)
>
> 7. 0036-Remove-trailing-whitespace-from-C-and-C-source.patch
>
> Did I miss anything?
That sounds like a good program. Try to do everything except 1 and 5 first;
those may be messy. I want 5.2 and 5.3, but I'm going to scrutinize any
attempt at 5.1 very closely because I want to avoid an API compatibility
break.
> Also Eric, could you please elaborate on the need for the intptr_t casts?
I wish I could, but I don't remember where the issue was. Let's cross that
bridge when we get to it.
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>