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.

Good point. I've kept it in an separated the cases for readability.