diff mbox series

[v2,7/7] util/fifo8: Introduce fifo8_discard()

Message ID 20240722160745.67904-8-philmd@linaro.org (mailing list archive)
State New, archived
Headers show
Series util/fifo8: Rework fifo8_pop_buf() | expand

Commit Message

Philippe Mathieu-Daudé July 22, 2024, 4:07 p.m. UTC
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(-)

Comments

Philippe Mathieu-Daudé July 22, 2024, 4:12 p.m. UTC | #1
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);
Pierrick Bouvier July 22, 2024, 9:07 p.m. UTC | #2
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>
Mark Cave-Ayland July 22, 2024, 9:52 p.m. UTC | #3
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 mbox series

Patch

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);