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

2013-10-03 Thread Miroslav Lichvar
On Thu, Oct 03, 2013 at 03:03:09PM +0200, Paul Menzel wrote:
> Am Donnerstag, den 03.10.2013, 14:48 +0200 schrieb Miroslav Lichvar:
> > --- 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(), UTI_TimevalToString(>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.

Ok, thanks. I'll make that change.

Maybe we could introduce an IFTRACEON/IFTRACEOFF macros to avoid the
ugly #ifdef wrapping.

-- 
Miroslav Lichvar

--
To unsubscribe email chrony-dev-requ...@chrony.tuxfamily.org with "unsubscribe" 
in the subject.
For help email chrony-dev-requ...@chrony.tuxfamily.org with "help" in the 
subject.
Trouble?  Email listmas...@chrony.tuxfamily.org.



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

2013-10-03 Thread Paul Menzel
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(), UTI_TimevalToString(>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


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


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

2013-10-03 Thread 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).

> 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(), UTI_TimevalToString(>local_rx));
+#else
+  (void)prev;
 #endif
   prev = inst->local_tx;
   if (inst->local_tx.tv_sec || inst->local_tx.tv_usec)

-- 
Miroslav Lichvar

-- 
To unsubscribe email chrony-dev-requ...@chrony.tuxfamily.org with "unsubscribe" 
in the subject.
For help email chrony-dev-requ...@chrony.tuxfamily.org with "help" in the 
subject.
Trouble?  Email listmas...@chrony.tuxfamily.org.