Re: [RFC/T][PATCH v2 2/3] rc80211-pid: introduce PID sharpening factor

[Mattias Nissler <mattias.nissler@gmx.de>]

On Mon, 2007-12-10 at 08:21 +0100, Stefano Brivio wrote:
> On Mon, 10 Dec 2007 07:28:01 +0100
> Mattias Nissler <mattias.nissler@gmx.de> wrote:
> 
> > > -	/* If no frames were transmitted, we assume the old sample is
> > > -	 * still a good measurement and copy it. */
> > > -	if (spinfo->tx_num_xmit == 0)
> > > -		pf = spinfo->last_pf;
> > 
> > Please don't remove this check. We can never know when anybody starts
> > calling rate_control_pid_sample() without packets transmitted. Then it's
> > good to have it, else we divide by zero in the next line.
> 
> But, as you said, rate_control_pid_sample() only gets called by
> rate_control_pid_tx_status(). There, spinfo->tx_num_xmit always get
> increased. The only other spot where spinfo->tx_num_xmit gets changed is in
> rate_control_pid_sample(), where we set it to 0 after that that division
> gets done, and we obviously change its value after having been called by
> rate_control_pid_tx_status(). So, it always get increased before the
> division, and it's never set to a negative value. Therefore, I assume that
> it will never be zero in the division. Am I wrong?

You're totally right. But IMHO it's nicer to be a little defensive here.
You never know what happens to your code in the future. Just imagine
somebody coming up with: Hey, let's use a timer-based approach for
sampling. Ok, that's easy to do, just call sample() every time the timer
expires. And whoops, there's the division by zero when no packets are
transmitted...

Mattias