Hi Mark,
On 22/7/24 23:26, Mark Cave-Ayland wrote:
On 22/07/2024 17:07, Philippe Mathieu-Daudé wrote:
Extract fifo8_pop_buf() from hw/scsi/esp.c and expose
it as part of the <qemu/fifo8.h> API. This function takes
care of non-contiguous (wrapped) FIFO buffer (which is an
implementation detail).
I wonder if it is also worth updating the comment for fifo8_pop_bufptr() indicating
that it returns a pointer to the internal buffer without checking for overflow,
We document:
* The function may return fewer bytes than requested when the data wraps
* around in the ring buffer; in this case only a contiguous part of the data
* is returned.
but I'll try to reword a bit.
and that in general fifo8_pop_buf() is recommended instead?
Yes, this was my first motivation but then I forgot to write it :)
BTW I now see fifo8_pop/peek_bufptr() as dangerous API and am thinking
of deprecating them (after release). AFAICT the difference is a pair of
memcpy(), when I expect to not be that important performance wise.
Otherwise:
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Thanks!
BTW I'll respin this series including the fifo8_peek_buf() patch that
I forgot and is the one I need in PL011. Preview:
+uint32_t fifo8_peek_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen)
+{
+ uint32_t tail_count, head_count = 0;
+
+ if (destlen == 0) {
+ return 0;
+ }
+
+ destlen = MIN(destlen, fifo->num);
+ tail_count = MIN(fifo->capacity - fifo->head, destlen);
+
+ if (dest) {
+ memcpy(dest, &fifo->data[fifo->head], tail_count);
+ }
+
+ /* Add FIFO wraparound if needed */
+ destlen -= tail_count;
+ head_count = MIN(destlen, fifo->head);
+ if (head_count && dest) {
+ memcpy(&dest[tail_count], &fifo->data[0], head_count);
+ }
+
+ return tail_count + head_count;
+}