Re: Trying to figure out a cppcheck warning

[Ervin Hegedüs]

Hi Nate,

On Sat, Jan 23, 2021 at 08:43:59PM -0600, Nate Bargmann wrote:
> I've been working through the code base with the cppcheck GUI
> program--remarkably few issues, actually--and have hit one in qtc_log.c
> that has me a bit stumped.  For reference start at line 198, the
> definition of the store_qtc() function:

[...]

First let me explain how QTC mechanism works on WAE. Now only the
CW/SSB contests count, the RTTY is a special case.

During the contest a DX station can send to EU stations data
from previous QSO's (except the current CALL): time (hour and
minutes), call and serial. The DX stations can send maximum 10
QCT, but they can send 1 per block, or max 10 in one block.

All previous QSO can only be sent once, so we MUST to mark the
QSO's which had sent. So if you sent me the QSO's above, you
can't send them anymore to anyone.

The `logline` in the code is the formatted QTC line. Consider you
participate on contest, you have one or more QSO, and made a
contact with me. I'll ask you, do you have QTC, you will answer
yes, and you will send me two QTC's:

01/02
0001 DL1A    002
0001 DL2A    003

The first line is the QTC header (serial and nr. of QTC's in
block), then the two QTC lines. The fields in the lines:
* 0001 means 00:01 was the time
* DL1A was the call
* 002 was the serial what _HE_ sent you

The attached code snippet responsible to mark the QSO's what
you've sent to avoid send them again.

A formatted log line looks like this:
 40CW  0004 0001 24-Jan-21 08:48   HA2OS          0001 0004 0001 DL1A           
002     7010.0
0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123
            ^                                     ^

I've put the indexes the next line, and two markers what this
code uses.

The first occurrence of getting the substring in line 216, and as
the comment shows, it gets the number of the QTC block, whixh is
1 in this case - but it doesn't count here.

The second occurrence is the problematic place what cppcheck
showed in line in 225.

This value (0001 at position 12) tell us YOUR serial of that QSO.
You have to mark this QSO you can't send anymore. This is your
first QSO (serial 001) which stored in 0. index, so you get value
(after atoi()) is 1.

But the next line (226) this value decremented by 1, because in
the list of array is 0 base indexed (first QSO stored in 0.
index).

> Now, it's quite possible that cppcheck is getting tripped up in some
> manner.  That said, I do think the array index and assignment on line
> 225 should be protected to assure the array index does not exceed
> MAX_QSOS, i.e. tempi be tested after its assignment by atoi().

no, I think the protection is not just that we never reach the
20000 QSO (in line 225) :).

MAX_QSOS value is 20000, so you can store 20000 items in the
list, on index 0..19999. If you can't store more QSO than 20000,
then this value won't bigger than 19999.

I think cppcheck is getting tripped here.

I think the problem is that there isn't any check which looks the
number of current QSO's: if we reach the MAX_QSOS number, we can
continue the QSO's (which is not true, perhaps there will be a
segfault - but it's independent from QTC's).

> 
> Here is the relevant part of the function:
> 
> 214     if (direction == SEND) {
> 215         /* find maximum sent QTC block serial */
> 216         g_strlcpy(temps, loglineptr + 50, 5);  // get serial of qtc block
> 217         tempi = atoi(temps);
> 218         if (tempi > nr_qtcsent) {
> 219             nr_qtcsent = tempi;
> 220         }
> 221 
> 222         /* mark corresponding qso line as used for QTC */
> 223         g_strlcpy(temps, loglineptr + 12, 5);  // qso nr in qso list
> 224         tempi = atoi(temps) - 1;
> 225         qsoflags_for_qtc[tempi] = 1;
> 226 
> 227         /* find first unused QSO number for QTCs */
> 228         if (tempi == next_qtc_qso && tempi < MAX_QSOS) {
> 229             while (qsoflags_for_qtc[tempi++] == 1) {
> 230                 if (tempi == MAX_QSOS)
> 231                     break;
> 232                 next_qtc_qso = tempi;
> 233             }
> 234         }
> 235     }
> 

Hope that's a bit more clear now :).


Thanks again,

73, Ervin
HA2OS