Re: [PATCH 1/4] new timeofday core subsystem (v. B2)

[john stultz]

On Thu, 2005-06-09 at 16:15 +0300, Pekka Enberg wrote:
> Hi John,
> 
> Some coding style comments below.

[snip]
> Please drop this redundant comment.
> 
> > +	interval_sum += interval;
> > +
> > +	write_seqlock_irqsave(&ntp_lock, flags);
> > +
> > +	/* decrement singleshot offset interval */
> 
> Ditto.
> 
> > +	ss_offset_len -= interval;
> > +	if(ss_offset_len < 0) /* make sure it doesn't go negative */
> 
> Ditto.
> 
> > +		ss_offset_len = 0;
> > +
> > +	/* Do second overflow code */
> 
> Drop redundant comment.
> 
> > +		ntp_maxerror += shiftR(ntp_tolerance, SHIFT_USEC);
> > +		if (ntp_maxerror > NTP_PHASE_LIMIT) {
> > +			ntp_maxerror = NTP_PHASE_LIMIT;
> > +			ntp_status |= STA_UNSYNC;
> > +		}
> > +
> > +		/* Calculate offset_adj for the next second */
> 
> Ditto.
> 
> > +		tmp = ntp_offset;
> 
> tmp could use a more descriptive name.
> 
> > +		if (!(ntp_status & STA_FLL))
> > +		    tmp = shiftR(tmp, SHIFT_KG + ntp_constant);
> > +
> > +		/* bound the adjustment to MAXPHASE/MINSEC */
> 
> Redundant comment.


Thanks for the uh, redundant redundant-comment comments ;)

Much is from the transition from psudo-code to actual code, so I
appreciate you pointing it out. There are a few spots where I feel there
is still value in the comments, so I've left them, but overall I think
you'll see an improvement.

thanks again for the feedback!
-john



-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/