Code review comment for lp:~martin-lp/hipl/update_ack_handling

On Wed, Aug 10, 2011 at 01:58:54PM +0000, David Martin wrote:
>
> On Wed, Aug 10, 2011 at 1:52 PM, René Hummen <email address hidden> wrote:
> > On 10.08.2011, at 13:16, David Martin wrote:
> >> David Martin has proposed merging lp:~martin-lp/hipl/update_ack_handling into lp:hipl.
> >> --- modules/update/hipd/update.c 2011-07-18 16:41:13 +0000
> >> +++ modules/update/hipd/update.c 2011-08-10 11:16:05 +0000
> >> @@ -157,6 +158,41 @@
> >> + if ((state->update_id_out_lower_bound <= hip_update_get_out_id(state) &&
> >> + ack_peer_update_id >= state->update_id_out_lower_bound &&
> >> + ack_peer_update_id <= hip_update_get_out_id(state)) ||
> >> + (state->update_id_out_lower_bound > hip_update_get_out_id(state) &&
> >> + (ack_peer_update_id >= state->update_id_out_lower_bound ||
> >> + ack_peer_update_id <= hip_update_get_out_id(state)))) {
> >> + state->update_id_out_lower_bound = ack_peer_update_id;
> >> + return true;
> >> + } else {
> >> + return false;
> >> + }
> >
> > Please break this down into a if () ... else if () ... else for higher readability.
>
> Is it even necessary to differentiate between the two cases? Come to
> think of it an overflow seems very unlikely. I would not count too
> much on my math skills but by my calculation it would take more than
> 130 years with an update every second to reach the end of that range
> as it is unsigned 32bit.

general comment that may or may not be relevant here: Do not calculate
based on some likely scenario. Calculate what may happen if one were
to (maliciously) hammer the daemon with update packets.

Diego