Re: [gpsd-dev] Weirdness in gpsrinex

[Gary E. Miller]

Yo Fred!

On Wed, 20 Mar 2019 15:30:30 -0700 (PDT)
Fred Wright <address@hidden> wrote:

> On Wed, 20 Mar 2019, Gary E. Miller wrote:
> > On Wed, 20 Mar 2019 12:35:45 -0700 (PDT)
> > Fred Wright <address@hidden> wrote:
> >  
> >> I was looking into some warnings in gpsrinex, but ran across
> >> something where it's really unclear what the intent was.  Both
> >> warnings relate to the obs_cnt array defined in lines 138-143.  
> >
> > Where do you see these warnings? gcc and 7.4.0m 8.2.0 and 8.3.0
> > compile clean for me.  
> 
> I've seen them on several platforms, including the Mac (where I work 
> primarily).  Adnittedly not on *all* platforms.
> 
> They all represent legitmate problems.

I don't see them as "problems", the code works.  It just neeeds to be
clearer.

> > Feel free to push a fix.  
> 
> I will, but along with the other fixes for gpsrinex.

Great.

> >> The weird one concerns the comparison in line 210 against the
> >> obs_cnts element.  It looks like the obs_codes enum should be the
> >> type of the *index* to the obs_cnts array, but it shouldn't be the
> >> type of its *value*, which most likely should be unsigned int.  
> >
> > The gpsrinex code works, but that is an error check, so likely
> > untested.  
> 
> I thought that it wasn't just an error check, but a cap on the value. 

Same thing.

> Admittedly, getting the count to pass 99999 would probably be fairly 
> unusual. :-)

Not as hard as you would think.  Not unusual to take two days of
RINEX to average out diurnal variation.  At one fix a second that
is just over 24 hours of data.  Less than 48.

> > It does look like obs_cnt_t obs_cnts should be a count, not an enum.
> > The enum is the index.  But since C uses ints for enums it ends up
> > working anyway.  
> 
> The reason for the warning is that it's comparing against 99999, but
> the enum range is known to be much less than that, so the compiler is 
> complaining about what should be an impossible condition.

But since an enum is an int, not impossible.  Just too ugly to live...

> > It would be nice if gpsrinex had a regression test...  If you patch
> > it, then I can easily test it.  
> 
> Yeah, fitting it into the existing test framework wouldn't be *too*
> hard, but not trivial.

On my long term TODO list.  I'm still finding junk in the NMEA right
now, and that is user obvious.

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

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

pgpWrCKL27Cwy.pgp
Description: OpenPGP digital signature