All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH fbtest] Initialize ticks in benchmark()
@ 2023-06-29  8:28 Philipp Zabel
  2023-07-05  9:23 ` Geert Uytterhoeven
  0 siblings, 1 reply; 3+ messages in thread
From: Philipp Zabel @ 2023-06-29  8:28 UTC (permalink / raw)
  To: linux-fbdev; +Cc: Geert Uytterhoeven, Helge Deller, Philipp Zabel

Fixes a build error with GCC 13:

  util.c: In function 'benchmark':
  util.c:177:17: error: 'ticks' may be used uninitialized [-Werror=maybe-uninitialized]
  util.c:161:14: note: 'ticks' was declared here

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 util.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util.c b/util.c
index cdf89b38618a..1193523990e7 100644
--- a/util.c
+++ b/util.c
@@ -158,7 +158,7 @@ static uint64_t get_ticks(void)
 
 double benchmark(void (*func)(unsigned long n, void *data), void *data)
 {
-    uint64_t ticks;
+    uint64_t ticks = 0;
     unsigned long n = 1;
 
     printf("Benchmarking... ");
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH fbtest] Initialize ticks in benchmark()
  2023-06-29  8:28 [PATCH fbtest] Initialize ticks in benchmark() Philipp Zabel
@ 2023-07-05  9:23 ` Geert Uytterhoeven
  2023-07-05  9:32   ` Philipp Zabel
  0 siblings, 1 reply; 3+ messages in thread
From: Geert Uytterhoeven @ 2023-07-05  9:23 UTC (permalink / raw)
  To: Philipp Zabel; +Cc: linux-fbdev, Helge Deller

Hi Philipp,

Thanks for your patch!

On Thu, Jun 29, 2023 at 10:28 AM Philipp Zabel <p.zabel@pengutronix.de> wrote:
> Fixes a build error with GCC 13:
>
>   util.c: In function 'benchmark':
>   util.c:177:17: error: 'ticks' may be used uninitialized [-Werror=maybe-uninitialized]
>   util.c:161:14: note: 'ticks' was declared here

I believe this is a false positive?
(unless your "long" type has only a single bit ;-)

But as this is built with -Werror, I agree it is better to play it safe,
and fix this.

>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>

> --- a/util.c
> +++ b/util.c
> @@ -158,7 +158,7 @@ static uint64_t get_ticks(void)
>
>  double benchmark(void (*func)(unsigned long n, void *data), void *data)
>  {
> -    uint64_t ticks;
> +    uint64_t ticks = 0;

Wouldn't it be better to preinitialize this to 1 instead?
Else an overzealous compiler might detect possible UB in the division
below, and more hell might break loose...

    return 1e6*n/ticks;

>      unsigned long n = 1;
>
>      printf("Benchmarking... ");

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH fbtest] Initialize ticks in benchmark()
  2023-07-05  9:23 ` Geert Uytterhoeven
@ 2023-07-05  9:32   ` Philipp Zabel
  0 siblings, 0 replies; 3+ messages in thread
From: Philipp Zabel @ 2023-07-05  9:32 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: linux-fbdev, Helge Deller

Hi Geert,

On Mi, 2023-07-05 at 11:23 +0200, Geert Uytterhoeven wrote:
> Hi Philipp,
> 
> Thanks for your patch!
> 
> On Thu, Jun 29, 2023 at 10:28 AM Philipp Zabel <p.zabel@pengutronix.de> wrote:
> > Fixes a build error with GCC 13:
> > 
> >   util.c: In function 'benchmark':
> >   util.c:177:17: error: 'ticks' may be used uninitialized [-Werror=maybe-uninitialized]
> >   util.c:161:14: note: 'ticks' was declared here
> 
> I believe this is a false positive?
> (unless your "long" type has only a single bit ;-)

Yes. I've clarified the commit message in v2.

> But as this is built with -Werror, I agree it is better to play it safe,
> and fix this.
> 
> > 
> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> 
> > --- a/util.c
> > +++ b/util.c
> > @@ -158,7 +158,7 @@ static uint64_t get_ticks(void)
> > 
> >  double benchmark(void (*func)(unsigned long n, void *data), void *data)
> >  {
> > -    uint64_t ticks;
> > +    uint64_t ticks = 0;
> 
> Wouldn't it be better to preinitialize this to 1 instead?
> Else an overzealous compiler might detect possible UB in the division
> below, and more hell might break loose...

Thanks, since we already are working around a confused compiler, this
seems like a good idea. Fixed in v2.

regards
Philipp

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-07-05  9:32 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-29  8:28 [PATCH fbtest] Initialize ticks in benchmark() Philipp Zabel
2023-07-05  9:23 ` Geert Uytterhoeven
2023-07-05  9:32   ` Philipp Zabel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.