Re: [PATCH] no compile warnings from cyclictest

[John Kacur <jkacur@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3406 bytes --]



On Sat, 3 Jun 2017, rolf.freitag@email.de wrote:

> Hi,
> 
> compiling cyclictest gives 4 warnings of type
> 
>   warning: ignoring return value of 'write'
> 
> and three of type
> 
>   warning: ... may be used uninitialized in this function
> 
> So i eleminated the first sort with a trash variable, which adds up 
> the return values and recycles them (their entropy) at the end via
> srand(). This also avoids a write-only variable.
> I eleminated the second sort by initializing the variables.
> 
> --- a/src/cyclictest/cyclictest.c
> +++ b/src/cyclictest/cyclictest.c
> @@ -222,6 +222,7 @@ static int use_fifo = 0;
>  static pthread_t fifo_threadid;
>  static int laptop = 0;
>  static int use_histfile = 0;
> +static unsigned int trash; // for unused return values, to avoid compiler warnings because of ignored return values
>  
>  #ifdef ARCH_HAS_SMI_COUNTER
>  static int smi = 0;
> @@ -491,10 +492,10 @@ static void tracemark(char *fmt, ...)
>         va_end(ap);
>  
>         /* write the tracemark message */
> -       write(tracemark_fd, tracebuf, len);
> +       trash += write(tracemark_fd, tracebuf, len);
>  
>         /* now stop any trace */
> -       write(trace_fd, "0\n", 2);
> +       trash += write(trace_fd, "0\n", 2);
>  }
>  
>  
> @@ -510,7 +511,7 @@ static void tracing(int on)
>                 case KV_26_LT24: prctl(0, 1); break;
>                 case KV_26_33:
>                 case KV_30:
> -                       write(trace_fd, "1", 1);
> +                       trash += write(trace_fd, "1", 1);
>                         break;
>                 default:         break;
>                 }
> @@ -520,7 +521,7 @@ static void tracing(int on)
>                 case KV_26_LT24: prctl(0, 0); break;
>                 case KV_26_33:
>                 case KV_30:
> -                       write(trace_fd, "0", 1);
> +                       trash += write(trace_fd, "0", 1);
>                         break;
>                 default:        break;
>                 }
> @@ -984,8 +985,10 @@ static void *timerthread(void *param)
>         int stopped = 0;
>         cpu_set_t mask;
>         pthread_t thread;
> -       unsigned long smi_now, smi_old;
> +       unsigned long smi_now, smi_old=0;
>  
> +    memset(&stop, 0, sizeof(stop)); /* grrr */
> +    
>         /* if we're running in numa mode, set our memory node */
>         if (par->node != -1)
>                 rt_numa_set_numa_run_on_node(par->node, par->cpu);
> @@ -1066,7 +1069,6 @@ static void *timerthread(void *param)
>         tsnorm(&next);
>  
>         if (duration) {
> -               memset(&stop, 0, sizeof(stop)); /* grrr */
>                 stop = now;
>                 stop.tv_sec += duration;
>         }
> @@ -2623,5 +2625,7 @@ int main(int argc, char **argv)
>         if (affinity_mask)
>                 rt_bitmask_free(affinity_mask);
>  
> +    srand(trash); // recycling of the trash (unused return values)
> +    
>         exit(ret);
>  }
> --

I seen to recall some old conversations on lkml where these kind of
patches to supress compiler warnings were considered worse than the 
warnings. It is possible to supress warnings that we need to deal with.
In addition I see that some of your patch touches on tracing, which
I'm planning on ripping out and replacing with the ability to do the same
kind of tracing with external programs, specifically trace-cmd.

Thanks

John