| Message ID | 20240722160745.67904-8-philmd@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | util/fifo8: Rework fifo8_pop_buf() | expand |
On 22/7/24 18:07, Philippe Mathieu-Daudé wrote: > Add the fifo8_discard() helper for clarity. > It is a simple wrapper over fifo8_pop_buf(). > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > include/qemu/fifo8.h | 8 ++++++++ > hw/scsi/esp.c | 2 +- > util/fifo8.c | 6 ++++++ > 3 files changed, 15 insertions(+), 1 deletion(-) > > diff --git a/include/qemu/fifo8.h b/include/qemu/fifo8.h > index 21c7a22937..53bafabd25 100644 > --- a/include/qemu/fifo8.h > +++ b/include/qemu/fifo8.h > @@ -129,6 +129,14 @@ const uint8_t *fifo8_pop_constbuf(Fifo8 *fifo, uint32_t max, uint32_t *numptr); > */ > const uint8_t *fifo8_peek_constbuf(Fifo8 *fifo, uint32_t max, uint32_t *numptr); > > +/** > + * fifo8_discard: > + * @fifo: FIFO to consume bytes > + * > + * Discard (consume) bytes from a FIFO. > + */ > +void fifo8_discard(Fifo8 *fifo, uint32_t len); > + > /** > * fifo8_reset: > * @fifo: FIFO to reset > diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c > index cec847b54a..c703fa7351 100644 > --- a/hw/scsi/esp.c > +++ b/hw/scsi/esp.c > @@ -351,7 +351,7 @@ static void do_message_phase(ESPState *s) > /* Ignore extended messages for now */ > if (s->cmdfifo_cdb_offset) { > int len = MIN(s->cmdfifo_cdb_offset, fifo8_num_used(&s->cmdfifo)); > - fifo8_pop_buf(&s->cmdfifo, NULL, len); > + fifo8_discard(&s->cmdfifo, len); > s->cmdfifo_cdb_offset = 0; > } > } > diff --git a/util/fifo8.c b/util/fifo8.c > index 6610b79182..ea39ca2552 100644 > --- a/util/fifo8.c > +++ b/util/fifo8.c > @@ -131,6 +131,12 @@ uint32_t fifo8_pop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen) > return n1 + n2; > } > > +void fifo8_consume(Fifo8 *fifo, uint32_t len) Sorry, forgot to s/fifo8_consume/fifo8_discard/. > +{ > + len -= fifo8_pop_buf(fifo, NULL, len); > + assert(len == 0); > +} > + > bool fifo8_is_empty(Fifo8 *fifo) > { > return (fifo->num == 0);
On 7/22/24 09:12, Philippe Mathieu-Daudé wrote: > On 22/7/24 18:07, Philippe Mathieu-Daudé wrote: >> Add the fifo8_discard() helper for clarity. >> It is a simple wrapper over fifo8_pop_buf(). >> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> --- >> include/qemu/fifo8.h | 8 ++++++++ >> hw/scsi/esp.c | 2 +- >> util/fifo8.c | 6 ++++++ >> 3 files changed, 15 insertions(+), 1 deletion(-) >> >> diff --git a/include/qemu/fifo8.h b/include/qemu/fifo8.h >> index 21c7a22937..53bafabd25 100644 >> --- a/include/qemu/fifo8.h >> +++ b/include/qemu/fifo8.h >> @@ -129,6 +129,14 @@ const uint8_t *fifo8_pop_constbuf(Fifo8 *fifo, uint32_t max, uint32_t *numptr); >> */ >> const uint8_t *fifo8_peek_constbuf(Fifo8 *fifo, uint32_t max, uint32_t *numptr); >> >> +/** >> + * fifo8_discard: >> + * @fifo: FIFO to consume bytes >> + * >> + * Discard (consume) bytes from a FIFO. >> + */ >> +void fifo8_discard(Fifo8 *fifo, uint32_t len); >> + >> /** >> * fifo8_reset: >> * @fifo: FIFO to reset >> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c >> index cec847b54a..c703fa7351 100644 >> --- a/hw/scsi/esp.c >> +++ b/hw/scsi/esp.c >> @@ -351,7 +351,7 @@ static void do_message_phase(ESPState *s) >> /* Ignore extended messages for now */ >> if (s->cmdfifo_cdb_offset) { >> int len = MIN(s->cmdfifo_cdb_offset, fifo8_num_used(&s->cmdfifo)); >> - fifo8_pop_buf(&s->cmdfifo, NULL, len); >> + fifo8_discard(&s->cmdfifo, len); >> s->cmdfifo_cdb_offset = 0; >> } >> } >> diff --git a/util/fifo8.c b/util/fifo8.c >> index 6610b79182..ea39ca2552 100644 >> --- a/util/fifo8.c >> +++ b/util/fifo8.c >> @@ -131,6 +131,12 @@ uint32_t fifo8_pop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen) >> return n1 + n2; >> } >> >> +void fifo8_consume(Fifo8 *fifo, uint32_t len) > > Sorry, forgot to s/fifo8_consume/fifo8_discard/. > >> +{ >> + len -= fifo8_pop_buf(fifo, NULL, len); >> + assert(len == 0); >> +} >> + >> bool fifo8_is_empty(Fifo8 *fifo) >> { >> return (fifo->num == 0); > > with this fix, Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
On 22/07/2024 17:07, Philippe Mathieu-Daudé wrote: > Add the fifo8_discard() helper for clarity. > It is a simple wrapper over fifo8_pop_buf(). > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > include/qemu/fifo8.h | 8 ++++++++ > hw/scsi/esp.c | 2 +- > util/fifo8.c | 6 ++++++ > 3 files changed, 15 insertions(+), 1 deletion(-) > > diff --git a/include/qemu/fifo8.h b/include/qemu/fifo8.h > index 21c7a22937..53bafabd25 100644 > --- a/include/qemu/fifo8.h > +++ b/include/qemu/fifo8.h > @@ -129,6 +129,14 @@ const uint8_t *fifo8_pop_constbuf(Fifo8 *fifo, uint32_t max, uint32_t *numptr); > */ > const uint8_t *fifo8_peek_constbuf(Fifo8 *fifo, uint32_t max, uint32_t *numptr); > > +/** > + * fifo8_discard: > + * @fifo: FIFO to consume bytes > + * Missing a reference to len in the comment above? > + * Discard (consume) bytes from a FIFO. > + */ > +void fifo8_discard(Fifo8 *fifo, uint32_t len); Perhaps fifo8_drop() is a more descriptive name here? Regardless: Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> ATB, Mark. > /** > * fifo8_reset: > * @fifo: FIFO to reset > diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c > index cec847b54a..c703fa7351 100644 > --- a/hw/scsi/esp.c > +++ b/hw/scsi/esp.c > @@ -351,7 +351,7 @@ static void do_message_phase(ESPState *s) > /* Ignore extended messages for now */ > if (s->cmdfifo_cdb_offset) { > int len = MIN(s->cmdfifo_cdb_offset, fifo8_num_used(&s->cmdfifo)); > - fifo8_pop_buf(&s->cmdfifo, NULL, len); > + fifo8_discard(&s->cmdfifo, len); > s->cmdfifo_cdb_offset = 0; > } > } > diff --git a/util/fifo8.c b/util/fifo8.c > index 6610b79182..ea39ca2552 100644 > --- a/util/fifo8.c > +++ b/util/fifo8.c > @@ -131,6 +131,12 @@ uint32_t fifo8_pop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen) > return n1 + n2; > } > > +void fifo8_consume(Fifo8 *fifo, uint32_t len) > +{ > + len -= fifo8_pop_buf(fifo, NULL, len); > + assert(len == 0); > +} > + > bool fifo8_is_empty(Fifo8 *fifo) > { > return (fifo->num == 0);
diff --git a/include/qemu/fifo8.h b/include/qemu/fifo8.h index 21c7a22937..53bafabd25 100644 --- a/include/qemu/fifo8.h +++ b/include/qemu/fifo8.h @@ -129,6 +129,14 @@ const uint8_t *fifo8_pop_constbuf(Fifo8 *fifo, uint32_t max, uint32_t *numptr); */ const uint8_t *fifo8_peek_constbuf(Fifo8 *fifo, uint32_t max, uint32_t *numptr); +/** + * fifo8_discard: + * @fifo: FIFO to consume bytes + * + * Discard (consume) bytes from a FIFO. + */ +void fifo8_discard(Fifo8 *fifo, uint32_t len); + /** * fifo8_reset: * @fifo: FIFO to reset diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c index cec847b54a..c703fa7351 100644 --- a/hw/scsi/esp.c +++ b/hw/scsi/esp.c @@ -351,7 +351,7 @@ static void do_message_phase(ESPState *s) /* Ignore extended messages for now */ if (s->cmdfifo_cdb_offset) { int len = MIN(s->cmdfifo_cdb_offset, fifo8_num_used(&s->cmdfifo)); - fifo8_pop_buf(&s->cmdfifo, NULL, len); + fifo8_discard(&s->cmdfifo, len); s->cmdfifo_cdb_offset = 0; } } diff --git a/util/fifo8.c b/util/fifo8.c index 6610b79182..ea39ca2552 100644 --- a/util/fifo8.c +++ b/util/fifo8.c @@ -131,6 +131,12 @@ uint32_t fifo8_pop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen) return n1 + n2; } +void fifo8_consume(Fifo8 *fifo, uint32_t len) +{ + len -= fifo8_pop_buf(fifo, NULL, len); + assert(len == 0); +} + bool fifo8_is_empty(Fifo8 *fifo) { return (fifo->num == 0);
Add the fifo8_discard() helper for clarity. It is a simple wrapper over fifo8_pop_buf(). Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- include/qemu/fifo8.h | 8 ++++++++ hw/scsi/esp.c | 2 +- util/fifo8.c | 6 ++++++ 3 files changed, 15 insertions(+), 1 deletion(-)