[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [gpsd-dev] Weirdness in gpsrinex

From: Fred Wright
Subject: Re: [gpsd-dev] Weirdness in gpsrinex
Date: Wed, 20 Mar 2019 15:30:30 -0700 (PDT)
User-agent: Alpine 2.21 (LRH 202 2017-01-01)

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.

One warning is just for the missing initializer braces.  That one's
easy - it should have two levels of brace rather than one (one for
the array and one for the struct).

Feel free to push a fix.

I will, but along with the other fixes for gpsrinex.

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. Admittedly, getting the count to pass 99999 would probably be fairly unusual. :-)

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.

Meanwhile, there's a separate unindexed count element which doesn't
seem to be used anywhere.

I assume you mean obs_cnt_t count?  I also don't see it used.

The obvious fix is to fix the type of obs_cnts and remove the unused
count element, but I'm not sure if there's something else going on
that I'm missing.

I think you are correct.

OK, I'll do that.

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.

Fred Wright

reply via email to

[Prev in Thread] Current Thread [Next in Thread]