On 01/04/2021 09:15, Philippe Mathieu-Daudé wrote:
On 4/1/21 9:49 AM, Mark Cave-Ayland wrote:
Each FIFO currently has its own push functions with the only difference being
the capacity check. The original reason for this was that the fifo8
implementation doesn't have a formal API for retrieving the FIFO capacity,
however there are multiple examples within QEMU where the capacity field is
accessed directly.
So the Fifo8 API is not complete / practical.
I guess it depends what you consider to be public and private to Fifo8: what is
arguably missing is something like:
int fifo8_capacity(Fifo8 *fifo)
{
return fifo->capacity;
}
But given that most other users access fifo->capacity directly then I might as well
do the same and save myself requiring 2 separate implementations of esp_fifo_pop_buf() :)
Change esp_fifo_push() to access the FIFO capacity directly and then consolidate
esp_cmdfifo_push() into esp_fifo_push().
Signed-off-by: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk>
---
hw/scsi/esp.c | 27 ++++++++-------------------
1 file changed, 8 insertions(+), 19 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 26fe1dcb9d..16aaf8be93 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -98,16 +98,15 @@ void esp_request_cancelled(SCSIRequest *req)
}
}
-static void esp_fifo_push(ESPState *s, uint8_t val)
+static void esp_fifo_push(Fifo8 *fifo, uint8_t val)
{
- if (fifo8_num_used(&s->fifo) == ESP_FIFO_SZ) {
+ if (fifo8_num_used(fifo) == fifo->capacity) {
trace_esp_error_fifo_overrun();
return;
}
- fifo8_push(&s->fifo, val);
+ fifo8_push(fifo, val);
}
-
Spurious line removal?
Ooops yes. I'm not too worried about this, but if Paolo has any other changes then I
can roll this into a v4.
static uint8_t esp_fifo_pop(ESPState *s)
{
if (fifo8_is_empty(&s->fifo)) {
@@ -117,16 +116,6 @@ static uint8_t esp_fifo_pop(ESPState *s)
return fifo8_pop(&s->fifo);
}
Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org>
ATB,
Mark.
