Goal is to implement "ping" on MFC the same as PN533 does. This patch implements MFC re-selection, but do not take care about authentication; this means this will have side effect while you are authenticated on a MFC block.
Commit 54729fb458 removed some dead code spotted by Coverity
but it had as effect to trigger a gcc warning, which prefers to see all enum in a switch rather than dead code:
pn53x.c: In function 'pn53x_InJumpForDEP':
pn53x.c:2552:5: warning: enumeration value 'NBR_UNDEFINED' not handled in switch [-Wswitch]
pn53x.c:2552:5: warning: enumeration value 'NBR_847' not handled in switch [-Wswitch]
So both switches were merged, which slightly optimizes the code for speed.
Problem reported by Coverity:
CID 1090321 (#1 of 1): Unchecked return value (CHECKED_RETURN)
unchecked_value: No check of the return value of "pn53x_set_property_bool(pnd, NP_INFINITE_SELECT, true)".
Note that this could happen e.g. if a fake PN533 sends malicious frames over USB
CID 1090329 (#1 of 1): Overflowed return value (INTEGER_OVERFLOW)
overflow_sink: Overflowed or truncated value (or a value computed from an overflowed or truncated value) "res" used as return value.
Problem reported by Coverity:
dead_error_condition: The switch value "nbr" cannot be "NBR_UNDEFINED".
CID 1090326 (#1 of 2): Logically dead code (DEADCODE)
dead_error_begin: Execution cannot reach this statement "case NBR_UNDEFINED:".
CID 1090331 (#1 of 1): Out-of-bounds access (OVERRUN)
11. overrun-buffer-arg: Overrunning array "pnti->nai.abtUid" of 10 bytes by passing it to a function which accesses it at byte offset 11 using argument "pnti->nai.szUidLen" (which evaluates to 12).
as Coverity fails seeing that szTargetTypes will always be = 0 in the case believed to lead to reading unitialized data in apttTargetTypes.
CID 1090347 (#1 of 1): Uninitialized scalar variable (UNINIT)
4. uninit_use_in_call: Using uninitialized element of array "apttTargetTypes" when calling "pn53x_InAutoPoll(struct nfc_device *, pn53x_target_type const *, size_t const, uint8_t const, uint8_t const, nfc_target *, int const)".
A buffer overflow could occur is a triple-size UID card was read with a PN531.
Moreover the way cascade tags were removed was just wrong.
Problem reported by Coverity
CID 1090331 (#1 of 1): Out-of-bounds access (OVERRUN)
10. overrun-buffer-arg: Overrunning buffer pointed to by "&pnti->nai.abtUid[5]" of 10 bytes by passing it to a function which accesses it at byte offset 11 using argument "7UL".
Coverity reported a read out of bounds but actually the real problem if PN531 and triple-size UID will already occur at
memcpy(pnti->nai.abtUid, pbtRawData, pnti->nai.szUidLen); where abtUid is of size 10 and szUidLen of size 12
The switch case has a default rule and a return in every cases. So the
code after the switch will never be executed.
Problem reported by thei Coverity tool
CID 1090408 (#1 of 1): Structurally dead code (UNREACHABLE)unreachable:
This code cannot be reached: "if (pn53x_current_target_ne...".
acr122s.c:331:3: warning: Null pointer passed as an argument to a 'nonnull' parameter
memcpy(buf, data, data_size);
^ ~~~~
pn53x.c:3138:3: warning: Null pointer passed as an argument to a 'nonnull' parameter
memcpy(CHIP_DATA(pnd)->current_target, pnt, sizeof(nfc_target));
^ ~~~
pn53x.c:85:57: warning: Result of 'malloc' is converted to a pointer of type 'nfc_modulation_type', which is incompatible with sizeof operand type 'nfc_modulation'
CHIP_DATA(pnd)->supported_modulation_as_initiator = malloc(sizeof(nfc_modulation) * 9);
^~~~~~ ~~~~~~~~~~~~~~~~~~~~~~
- Fixed the suppression of the auto-fixup for linking against MS built libs
- Fixed all the formatting warnings by shifting to inttypes.h specifiers
- shifted to %lu for DWORD printf
This becomes more consistent with all other pass by pointer of most structures.
Additionally, this should lessen stack memory usage, as building strings with str_nfc_target would push the target (283 bytes) plus then a copy of the info objects (up to 275) onto the stack as it dives into the sprintf functions.
Lastly, this makes my attempt at a .NET wrapper easier, as I can make passing by pointer work, but passing by value seems to bomb on the interop right now.
Fix the following cppcheck warnings:
[libnfc/drivers/pn532_uart.c:492]: (style) The scope of the variable 'res' can be reduced
[libnfc/chips/pn53x.c:320]: (style) The scope of the variable 'res' can be reduced
[libnfc/chips/pn53x.c:616]: (style) The scope of the variable 'res' can be reduced
[libnfc/nfc.c:356]: (style) The scope of the variable '_device_found' can be reduced
[utils/nfc-mfclassic.c:178]: (style) The scope of the variable 'key_index' can be reduced
Fixes:
pn53x.c:523:11: warning: Value stored to 'pbtRawData' is never read
pbtRawData += szRawData - 8;
^ ~~~~~~~~~~~~~
pn53x.c:544:7: warning: Value stored to 'pbtRawData' is never read
pbtRawData += 2;
^ ~
pn53x.c:532:7: warning: Value stored to 'pbtRawData' is never read
pbtRawData += 8;
^ ~
pn53x.c:3085:3: warning: Value stored to 'buflen' is never read
buflen -= res;
^ ~~~
pn53x.c:3080:3: warning: Value stored to 'buf' is never read
buf += res;
^ ~~~
target-subr.c:466:7: warning: Value stored to 'dst' is never read
dst += sprintf(dst, "* Unknown card, sorry\n");
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
target-subr.c:480:3: warning: Value stored to 'dst' is never read
dst += sprint_hex(dst, nfi.abtSysCode, 2);
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
target-subr.c:490:3: warning: Value stored to 'dst' is never read
dst += sprint_hex(dst, nji.btId, 4);
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
target-subr.c:546:7: warning: Value stored to 'dst' is never read
dst += sprintf(dst, "\n");
^ ~~~~~~~~~~~~~~~~~~
target-subr.c:571:5: warning: Value stored to 'dst' is never read
dst += sprint_hex(dst, nii.abtAtr, nii.szAtrLen);
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
target-subr.c:580:3: warning: Value stored to 'dst' is never read
dst += sprint_hex(dst, nsi.abtUID, 8);
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
target-subr.c:593:3: warning: Value stored to 'dst' is never read
dst += sprintf(dst, " Fab Code: %02X\n", nci.btFabCode);
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
target-subr.c:608:5: warning: Value stored to 'dst' is never read
dst += sprint_hex(dst, ndi.abtGB, ndi.szGB);
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Fix compiler warnings and potential bug
pn53x.c:1526:19: warning: The right operand of '<' is a garbage value
for (i = 0; i < sz; i++) {
^ ~~
pn53x.c:1621:19: warning: The right operand of '<' is a garbage value
for (i = 0; i < sz; i++) {
^ ~~