diff mbox series

hw/input/tsc2005: Fix -Wchar-subscripts warning in tsc2005_txrx()

Message ID 20240502074158.85800-1-philmd@linaro.org (mailing list archive)
State New
Headers show
Series hw/input/tsc2005: Fix -Wchar-subscripts warning in tsc2005_txrx() | expand

Commit Message

Philippe Mathieu-Daudé May 2, 2024, 7:41 a.m. UTC
Check the function index is not negative and use an unsigned
variable to avoid the following warning with GCC 13.2.0:

  [666/5358] Compiling C object libcommon.fa.p/hw_input_tsc2005.c.o
  hw/input/tsc2005.c: In function 'tsc2005_timer_tick':
  hw/input/tsc2005.c:416:26: warning: array subscript has type 'char' [-Wchar-subscripts]
    416 |     s->dav |= mode_regs[s->function];
        |                         ~^~~~~~~~~~

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/input/tsc2005.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Peter Maydell May 2, 2024, 12:11 p.m. UTC | #1
On Thu, 2 May 2024 at 08:42, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Check the function index is not negative and use an unsigned
> variable to avoid the following warning with GCC 13.2.0:
>
>   [666/5358] Compiling C object libcommon.fa.p/hw_input_tsc2005.c.o
>   hw/input/tsc2005.c: In function 'tsc2005_timer_tick':
>   hw/input/tsc2005.c:416:26: warning: array subscript has type 'char' [-Wchar-subscripts]
>     416 |     s->dav |= mode_regs[s->function];
>         |                         ~^~~~~~~~~~
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/input/tsc2005.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/hw/input/tsc2005.c b/hw/input/tsc2005.c
> index 941f163d36..fa93eb5d25 100644
> --- a/hw/input/tsc2005.c
> +++ b/hw/input/tsc2005.c
> @@ -406,6 +406,7 @@ uint32_t tsc2005_txrx(void *opaque, uint32_t value, int len)
>  static void tsc2005_timer_tick(void *opaque)
>  {
>      TSC2005State *s = opaque;
> +    unsigned func_idx;
>
>      /* Timer ticked -- a set of conversions has been finished.  */
>
> @@ -413,7 +414,9 @@ static void tsc2005_timer_tick(void *opaque)
>          return;
>
>      s->busy = false;
> -    s->dav |= mode_regs[s->function];
> +    assert(s->function >= 0);
> +    func_idx = s->function;
> +    s->dav |= mode_regs[func_idx];
>      s->function = -1;
>      tsc2005_pin_update(s);
>  }

This code is in a deprecated device, but it makes sense to
avoid the compiler warning in the meantime. (I'm curious why this
only comes up now, since I thought this warning had been around
for a while. Maybe gcc 13 is applying it in a wider range of
situations.)

If we're going to assert I think we might as well catch
array overruns in both directions:

     unsigned int function = s->function;

     assert(function < ARRAY_SIZE(mode_regs);
     s->dav |= mode_regs[function];

thanks
-- PMM
diff mbox series

Patch

diff --git a/hw/input/tsc2005.c b/hw/input/tsc2005.c
index 941f163d36..fa93eb5d25 100644
--- a/hw/input/tsc2005.c
+++ b/hw/input/tsc2005.c
@@ -406,6 +406,7 @@  uint32_t tsc2005_txrx(void *opaque, uint32_t value, int len)
 static void tsc2005_timer_tick(void *opaque)
 {
     TSC2005State *s = opaque;
+    unsigned func_idx;
 
     /* Timer ticked -- a set of conversions has been finished.  */
 
@@ -413,7 +414,9 @@  static void tsc2005_timer_tick(void *opaque)
         return;
 
     s->busy = false;
-    s->dav |= mode_regs[s->function];
+    assert(s->function >= 0);
+    func_idx = s->function;
+    s->dav |= mode_regs[func_idx];
     s->function = -1;
     tsc2005_pin_update(s);
 }