Skip to content

xerox_mfp: Avoid a buffering stall, when reading image data

Nomadbyte requested to merge nomadbyte/sane-backends:xerox_mfp-729-read into master

This addresses a buffering stall which could occur, when the length of free space after the buffered data at the end of the allocated buffer is less than USB_BLOCK_SIZE, i.e. (DATAROOM(dev) > 0 && DATAROOM(dev) < USB_BLOCK_SIZE).

In such case an expectation for the read request length to be a multiple of USB_BLOCK_SIZE, that is (DATAROOM(dev) & USB_BLOCK_MASK), would result in zero remaining buffer room. Combined with no data consumed out of the buffer, yet more data to receive for the current block, it would lead to an infinite loop (with buffering stalled, the buffer will eventually starve, and stored data length would fall below the defined dev->bytes_per_line). This was observed with Samsung SCX-4729FW in network (TCP) mode via Wi-Fi, with JPEG compression disabled.

The USB_BLOCK_SIZE is imposed by USB specs; USB 2.0 requires max 512 byte packet size for bulk transfers with high-speed endpoints. This is applicable to USB transport. TCP transport does not have such restriction.

NOTE: DATAROOM(dev) returns the length of free space from the end of the buffered data up till the allocated end of the buffer. That means, it may not always report the total free space left -- only the length of the contiguous space. When the free space "wraps around", the reported free space is short of dev->dataoff.

Fixes #729

  • also added a few notes about the cyclic/circular buffer implementation choice used in xerox_mfp code; perhaps this would add some clarity to future developers dealing with the code;
  • cleaned whitespace/indenting, it was causing confusion while reading the code due to inconsistency;
  • tested these changes with Samsung SCX-4729FW both in network (TCP) and USB modes;

I invite @ValdikSS1 and @dclaisse, possibly other users to further test this on their MFP models, if interested.

Edited by Nomadbyte

Merge request reports