Closed salieff closed 4 years ago
duplicate of #371
The code is never used for basic usage of this library. To fix this right you have to dig into the rfc specification and program code (there are some more smelling code). Or let it be... :-/
Please consider reopening this issue as it blocks compiling an Arduino sketch with -Werror
and ignoring warnings is generally a bad smell because other warnings mingle in with the known ones and get missed.
Besides, I think looking at the code shows up a bunch of problems in this area which suggest it has no users and could just be deleted or #ifdef
'd out until it's all fixed.
The prototype allows two optional parameters, both pointers, both defaulting to NULL.
StatusCode TCL_Transceive(TagInfo * tag, byte *sendData, byte sendLen,
byte *backData = NULL, byte *backLen = NULL);
The function body always unconditionally dereferences backLen
.
byte totalBackLen = *backLen;
I don't know C++, only C, but I assume this is just as invalid in C++?
That implies backLen
is never allowed to be NULL and thus must always be given but I don't think that's the author's intent so it's probably a bug and the parameter is intended to be optional and so NULL.
Then we have the two uses which guard the memcpy(3), e.g.
if (backData && (backLen > 0)) {
if (*backLen < in.inf.size)
return STATUS_NO_ROOM;
*backLen = in.inf.size;
memcpy(backData, in.inf.data, in.inf.size);
}
Given backLen
may be NULL the if-condition must guard against it. The test that the length is big enough is already in the body and correct so I think this just needs to change to
if (backData && backLen) {
Similarly for the second case:
if (backData && (backLen > 0)) {
if ((*backLen + ackDataSize) > totalBackLen)
return STATUS_NO_ROOM;
memcpy(&(backData[*backLen]), ackData, ackDataSize);
*backLen += ackDataSize;
}
We've already saved the original size of backData
by copying *backLen
into totalBackLen
and have altered *backLen
to reflect what we've used so far so the body's test takes all that into account and adjusts memcpy()'s destination. All that's left is to correct the guard as before. Changing the test to *backLen > 0
wouldn't be much use and wouldn't allow for in.inf.size
to be 0 in the first memcpy.
@salieff, does this seem accurate to you given you've submitted a patch in the past?
Please consider reopening this issue
@RalphCorderoy Why reopen? I mentioned that it is a duplicate of #371 . Please write there instead of splitting a problem into multiple issue reports.
Hi @Rotzbua, Sorry, I thought I was on that earlier issue. I've repeated the comment there.
Step 0: Are you in the right place?
Yes
Step 1: Describe your environment
Step 2: Describe the problem
Mistypo in if expression
Affected file(s) or example(s):
Steps to reproduce:
Observed Results:
Expected Results:
Relevant Code: