mirror of
https://github.com/ArduPilot/ardupilot.git
synced 2026-10-06 19:00:27 +08:00
GCS_MAVLink: burst reply offset calculation fix
We were calculating the reply offset using the index number times the requested read size, which has an unwritten assumption that the filesystem will never return a short non-zero read for any reason other than the end of the file, which might not be true for all virtual file backends (including future ones we might want to add). Additionally, the burst_complete flag makes the same flawed assumption. The burst_complete flag is only needed when we hit the burst packet limit or the NAK at the end of the file. It is not needed on the final ACK prior to the EOF NAK, and in fact, can lead to a pointless extra burst read from the client. As a side-effect, this fixes the offset calculation for the NAK on in two edge cases: - File size is an exact multiple of the read size - The request is at-or-beyond the end of the file, in which case, we should send the requested offset back in the NAK
This commit is contained in:
committed by
Andrew Tridgell
parent
294f1c076c
commit
a499d17eec
@@ -592,10 +592,12 @@ bool GCS_FTP::Session::handle_request(Transaction &request, Transaction &reply)
|
||||
|
||||
// this transfer size is enough for a full parameter file with max parameters
|
||||
const uint32_t transfer_size = 2000;
|
||||
reply.offset = request.offset;
|
||||
for (uint32_t i = 0; (i < transfer_size); i++) {
|
||||
// fill the buffer
|
||||
const ssize_t read_bytes = AP::FS().read(fd, reply.data, MIN(sizeof(reply.data), max_read));
|
||||
if (read_bytes == -1) {
|
||||
reply.burst_complete = true;
|
||||
GCS_FTP::error(reply, FTP_ERROR::FailErrno);
|
||||
break;
|
||||
}
|
||||
@@ -606,21 +608,20 @@ bool GCS_FTP::Session::handle_request(Transaction &request, Transaction &reply)
|
||||
}
|
||||
|
||||
if (read_bytes == 0) {
|
||||
reply.burst_complete = true;
|
||||
GCS_FTP::error(reply, FTP_ERROR::EndOfFile);
|
||||
break;
|
||||
}
|
||||
|
||||
reply.opcode = FTP_OP::Ack;
|
||||
reply.offset = request.offset + i * max_read;
|
||||
reply.burst_complete = ((read_bytes < max_read) || (i == (transfer_size - 1)));
|
||||
// Signal to the client that they need to request another burst read to get more data
|
||||
reply.burst_complete = (i == (transfer_size - 1));
|
||||
reply.size = (uint8_t)read_bytes;
|
||||
|
||||
push_reply(reply);
|
||||
|
||||
if (read_bytes < max_read) {
|
||||
// ensure the NACK which we send next is at the right offset
|
||||
reply.offset += read_bytes;
|
||||
}
|
||||
// update the offset for the next read
|
||||
reply.offset += read_bytes;
|
||||
|
||||
// prep the reply to be used again
|
||||
reply.seq_number++;
|
||||
|
||||
Reference in New Issue
Block a user