Error in consoleM7::Read

[Gorge997]

Description

I was trying to send data via serial from the pc to the coral micro board.

After seeing that the freertos task that takes care of reading from serial does not work when connected to the usb port I decided to connect directly to the TX and RX pins via a usb-to-serial converter.

I was then able to send data to the board but, after a certain number of values, everything stopped working.

I noticed that there is an error in the Read function of the M7 console (coralmicro/libs/base/consolem7.cc).

int ConsoleM7::Read(char* buffer, int size) {
  if (!rx_task_) {
    return -1;
  }
  MutexLock lock(rx_mutex_);
  int bytes_to_return = std::min(size, static_cast<int>(rx_buffer_available_));

  if (!bytes_to_return) {
    return -1;
  }

  int bytes_to_read = bytes_to_return;
  if (rx_buffer_read_ > rx_buffer_write_) {
    memcpy(buffer, &rx_buffer_[rx_buffer_read_],
           kRxBufferSize - rx_buffer_read_);
    bytes_to_read -= kRxBufferSize - rx_buffer_read_;
    if (bytes_to_read) {
      memcpy(buffer + (bytes_to_return - bytes_to_read), &rx_buffer_[0],
             bytes_to_read);
    }
  } else {
    memcpy(buffer, &rx_buffer_[rx_buffer_read_], bytes_to_read);
  }
  rx_buffer_available_ -= bytes_to_return;
  rx_buffer_read_ = (rx_buffer_read_ + bytes_to_return) % kRxBufferSize;

  return bytes_to_return;
}

The function, in case the write pointer is in a position that precedes the read pointer (a case that occurs if the write pointer has wrapped) tries to insert into the buffer passed as a parameter all the characters from the read pointer to the end of the serial buffer. Without checking whether the buffer's size is sufficient.

I think (and I solved it by making this change into my code) that bytes_to_read should be checked before copying rx_buffer_ into buffer.

This is my solution:

int ConsoleM7::Read(char* buffer, int size) {
  if (!rx_task_) {
    return -1;
  }
  MutexLock lock(rx_mutex_);
  int bytes_to_return = std::min(size, static_cast<int>(rx_buffer_available_));

  if (!bytes_to_return) {
    return -1;
  }

  int bytes_to_read = bytes_to_return;
  if (rx_buffer_read_ > rx_buffer_write_) {
    // This line is wrong. There are some cases where the number of byte to read are less than
    // the number of bytes from the read pointer to the end of the buffer.
    //memcpy(buffer, &rx_buffer_[rx_buffer_read_], kRxBufferSize - rx_buffer_read_);  // ORIGINAL LINE

    // Add a check to see if the number of bytes to read is less than the number of bytes from the read pointer to the end of the buffer.
    if (bytes_to_read > kRxBufferSize - rx_buffer_read_) {
      memcpy(buffer, &rx_buffer_[rx_buffer_read_], kRxBufferSize - rx_buffer_read_);
    }
    else {
      memcpy(buffer, &rx_buffer_[rx_buffer_read_], bytes_to_read);
    }

    bytes_to_read -= kRxBufferSize - rx_buffer_read_;
    if (bytes_to_read > 0) {
      memcpy(buffer + (bytes_to_return - bytes_to_read), &rx_buffer_[0], bytes_to_read);
    }
  } else {
    memcpy(buffer, &rx_buffer_[rx_buffer_read_], bytes_to_read);
  }
  rx_buffer_available_ -= bytes_to_return;
  rx_buffer_read_ = (rx_buffer_read_ + bytes_to_return) % kRxBufferSize;

  return bytes_to_return;
}

Click to expand!

### Issue Type Bug ### Operating System _No response_ ### Coral Device Dev Board Micro ### Other Devices _No response_ ### Programming Language C++ ### Relevant Log Output _No response_