Yo Hal! On Fri, 31 Oct 2025 19:14:46 -0700 Hal Murray <[email protected]> wrote:
> - } else {
> - // no room for more
> + } else if (3 < (s_end - sentences)) {
> + // no room for more, but room for '...'
> *--s_end = '.';
> *--s_end = '.';
>
> > I've looked at that code a lot. Seems solid to me. What do you
> > not like?
>
> It's hard to understand and it doesn't fix the warning.
Easy: (s_end - sentences) is how many bytes in the buffer. Since
this code overwrites the lasst 3 bytes, it ensures there are three
bytes to overwrite. Thus --s_end can never get below (-1) that
start of the buffer.
> I would
> revert that change and add a comment saying the gcc 10 gives a bogus
> warnings.
Care to share the warning? That last one you gave was for gcc 14.
> Try to figure out what that new code is doing without knowing that it
> is trying to dodge a warning.
Did you just get my explanation above? What is not clear?
> The old code had a catch-all else at the end. The new code doesn't.
> What should happen if the new else fails?
Easy, the last 3 bytes will not be replaced by "...". a) no big
deal, b) can never happen.
> I see a way that the warning gets close to real. Suppose sentences
> is empty and fields[0] is big so it doesn't fit. s_end will point to
> the beginning of sentences so backing up is evil.
a) intentional, b) already tested for at least 3 bytes, so can't back up.
> But field0_len comes from:
> size_t field0_len = strnlen(fields[0], NMEA_MAX_FLD);
> include/gpsd.h:#define NMEA_MAX_FLD 100 // max fields
> in an NMEA sentence
> include/gpsd.h: char *field[NMEA_MAX_FLD];
> I can't figure that out.
field_len will be the smalle of the length of fields[0], or NMEA_MAX_FLD.
Clear?
> Is that max fields or max field length?
Oh, that is a bug. But the value of 100 still works. I just pushed a
patch to hange to MAX_FLD, which is 130. In either case we want it less
that 132. So seame effect.
> Assming max field length, then it will fit because 100<132
> static char sentences[132];
Right, so no problems.
> If the problem is that the compiler doesn't figure out that the
> fields[0] will always fit, that would explain why your patch didn't
> fix the warning.
No. The warnings is about a negative index into buf, not one that
is too large.
> if ((s_len + field0_len + 2) < max) {
> // room for more
> *s_end++ = ' ';
> *s_end = '\0';
> (void)strlcpy(s_end, fields[0], field0_len + 2);
> Is that +2 on the strlcpy corrent? You just bumped s_end by one to
> insert the space.
Yes, correct. That e,ore tha ensures the space and NUL fit.
RGDS
GARY
---------------------------------------------------------------------------
Gary E. Miller Rellim 109 NW Wilmington Ave., Suite E, Bend, OR 97703
[email protected] Tel:+1 541 382 8588
Veritas liberabit vos. -- Quid est veritas?
"If you can't measure it, you can't improve it." - Lord Kelvin
pgpHDAQOZXShg.pgp
Description: OpenPGP digital signature
