Update getFileBlock to match PR 40 in the SARA-R5 Library #24
Conversation
| { | ||
| return UBX_CELL_ERROR_OUT_OF_MEMORY; | ||
| } | ||
| sprintf(command, "%s=\"%s\",%lu,%lu", UBX_CELL_FILE_SYSTEM_READ_BLOCK, filename.c_str(), (unsigned long) offset, (unsigned long) requestedLength); |
There was a problem hiding this comment.
Use snprintf() instead of sprintf()
| while (bytesRead < data_length) | ||
| { | ||
| // This method seems more reliable than reading a byte at a time. | ||
| size_t rc = _hardSerial->readBytes(&buffer[bytesRead], bytesRemaining); | ||
| bytesRead += rc; | ||
| bytesRemaining -= rc; | ||
| } |
There was a problem hiding this comment.
Is it possible for _hardSerial->readBytes() to return zeros forever? If so, this loop will never exit, so it'd be good to have a timeout
| command = ubx_cell_calloc_char(strlen(UBX_CELL_FILE_SYSTEM_READ_BLOCK) + filename.length() + 28); | ||
| if (command == nullptr) | ||
| { | ||
| return UBX_CELL_ERROR_OUT_OF_MEMORY; | ||
| } | ||
| sprintf(command, "%s=\"%s\",%lu,%lu", UBX_CELL_FILE_SYSTEM_READ_BLOCK, filename.c_str(), (unsigned long) offset, (unsigned long) requestedLength); |
There was a problem hiding this comment.
I'm not sure strlen(UBX_CELL_FILE_SYSTEM_READ_BLOCK) + filename.length() + 28 is big enough for what can go into command. After strlen(UBX_CELL_FILE_SYSTEM_READ_BLOCK) + filename.length(), you have the characters ="",, (5 characters), plus 2x long integers, which are 64-bit on ESP32 (up to 20 digits each, I believe). So replace 28 with 45, and I think this should be able to handle any input (and will be memory safe if snprintf() is used).
| while (quoteCount < 3 && bytesRead < minimumResponseAllocation) | ||
| { | ||
| ich = _hardSerial->read(); | ||
| if (ich < 0) | ||
| { | ||
| continue; | ||
| } | ||
| ch = (char)(ich & 0xFF); | ||
| response[bytesRead++] = ch; | ||
| if (ch == '"') | ||
| { | ||
| quoteCount++; | ||
| } | ||
| else if (ch == ',' && lengthIndex == 0 && quoteCount == 2) | ||
| { | ||
| lengthIndex = bytesRead; | ||
| } | ||
| } |
There was a problem hiding this comment.
Should lengthIndex be checked after this loop? Just thinking what happens if the response is malformed, should try to detect that.
| response = ubx_cell_calloc_char(minimumResponseAllocation); | ||
| if (response == nullptr) | ||
| { | ||
| if (_printDebug == true) | ||
| { | ||
| if (_printDebug == true) | ||
| { | ||
| _debugPort->println(F("getFileBlock: only works with a hardware UART")); | ||
| } | ||
| return UBX_CELL_ERROR_INVALID; | ||
| _debugPort->print(F("getFileBlock: response alloc failed: ")); | ||
| _debugPort->println(minimumResponseAllocation); | ||
| } | ||
|
|
||
| size_t cmdLen = filename.length() + 32; | ||
| char *cmd = ubx_cell_calloc_char(cmdLen); | ||
| if (cmd == nullptr) | ||
| return UBX_CELL_ERROR_OUT_OF_MEMORY; | ||
| snprintf(cmd, cmdLen, "at+urdblock=\"%s\",%zu,%zu\r\n", filename.c_str(), offset, requested_length); | ||
| sendCommand(cmd, false); | ||
|
|
||
| int ich; | ||
| char ch; | ||
| int quote_count = 0; | ||
| size_t comma_idx = 0; | ||
|
|
||
| while (quote_count < 3) | ||
| free(command); | ||
| return UBX_CELL_ERROR_OUT_OF_MEMORY; | ||
| } |
There was a problem hiding this comment.
response is a fixed size, so don't use ubx_cell_calloc_char(), just create it as an array:
char response[minimumResponseAllocation];
Makes
getFileBlockthe same as the SARA-R5 Library - solvessprintf%zucompilation errors on some platforms - see PR #40.