Re: [chrony-dev] [PATCH] ntp_core.c: Remove useless assignment `prev = inst->local_rx;`

[Paul Menzel <paulepanter@xxxxxxxxxxxxxxxxxxxxx>]

Am Donnerstag, den 03.10.2013, 14:48 +0200 schrieb Miroslav Lichvar:
> On Thu, Oct 03, 2013 at 02:34:46PM +0200, Paul Menzel wrote:
> > The Clang static analyzer scan-build found the following unneeded
> > assignment.
> > 
> >         /usr/share/clang/scan-build/ccc-analyzer -O2 -g  -c sources.c
> >         ntp_core.c:1545:3: warning: Value stored to 'prev' is never read
> >           prev = inst->local_rx;
> >           ^      ~~~~~~~~~~~~~~
> >         1 warning generated.
> 
> The value is used when compiled with tracing (./configure --enable-trace)..

Sorry, I overlooked that, just looking at the next `prev` assignment.

> > Indeed `prev` is not read before being assigned the same value again
> > some lines below.
> 
> The second prev assignemt is to local_tx (not local_rx).
> 
> Would the following patch fix the warning?
> 
> --- a/ntp_core.c
> +++ b/ntp_core.c
> @@ -1548,6 +1548,8 @@ NCR_SlewTimes(NCR_Instance inst, struct timeval *when, double dfreq, double doff
>  #ifdef TRACEON
>    LOG(LOGS_INFO, LOGF_NtpCore, "rx prev=[%s] new=[%s]",
>        UTI_TimevalToString(&prev), UTI_TimevalToString(&inst->local_rx));
> +#else
> +  (void)prev;
>  #endif
>    prev = inst->local_tx;
>    if (inst->local_tx.tv_sec || inst->local_tx.tv_usec)

Correct, this patch addresses the warning. It’s pretty ugly though just
for appeasing code checkers/analyzers. ;-) But on the other hand, it is
used in the code already. Besides including these assignments into if
statements I was not able to come up with a better solution either.


Thanks and sorry again,

Paul

Attachment: signature.asc
Description: This is a digitally signed message part