NetworkClientRxBuffer::clear() may not always clear

[TD-er]

Board

Any

Device Description

Hardware Configuration

Version

latest master (checkout manually)

IDE Name

PlatformIO

Operating System

Windows 11

Flash frequency

Any

PSRAM enabled

yes

Upload speed

115200

Description

See current implementation of clear():

arduino-esp32/libraries/Network/src/NetworkClient.cpp

Lines 149 to 155 in e70f4d3

	void clear() {
	if (r_available()) {
	fillBuffer();
	}
	_pos = _fill;
	}
	};

and fillBuffer():

arduino-esp32/libraries/Network/src/NetworkClient.cpp

Lines 63 to 88 in e70f4d3

	size_t fillBuffer() {
	if (!_buffer) {
	_buffer = (uint8_t *)malloc(_size);
	if (!_buffer) {
	log_e("Not enough memory to allocate buffer");
	_failed = true;
	return 0;
	}
	}
	if (_fill && _pos == _fill) {
	_fill = 0;
	_pos = 0;
	}
	if (!_buffer || _size <= _fill || !r_available()) {
	return 0;
	}
	int res = recv(_fd, _buffer + _fill, _size - _fill, MSG_DONTWAIT);
	if (res < 0) {
	if (errno != EWOULDBLOCK) {
	_failed = true;
	}
	return 0;
	}
	_fill += res;
	return res;
	}

When the rx buffer is full or there is more data available than would fit in the buffer, the buffer is not cleared.

Suggested code change:

  void clear() {
    if (r_available()) {
      _pos = _fill;
      while(fillBuffer())
       _pos = _fill;        
    }
    _pos = 0;
    _fill = 0;
  }

Only drawback I can think of with my suggested change is when there is a continuous amount of data to be cleared.
However this would in the current situation also have lead to issues.

Sketch

-

Debug Message

-

Other Steps to Reproduce

No response

I have checked existing issues, online documentation and the Troubleshooting Guide

• I confirm I have checked existing issues, online documentation and Troubleshooting guide.

[me-no-dev]

OK, I get what you mean, but I think that _pos = _fill; is wrong, because it will cause fillBuffer() to return 0. Instead it should do the following to make sure that each iteration the buffer is marked empty

void clear() {
  if (r_available()) {
    _pos = 0;
    _fill = 0;
    while(fillBuffer()){
      _pos = 0;
      _fill = 0;
    }
  }
  _pos = 0;
  _fill = 0;
}

[me-no-dev]

ah my mistake. i read it wrong... yes. your suggestion makes sense