My modifications

[Lennart Hellström (ERA)]

> Version 2 ready for review, with the correct indentation I hope...
> 
> I've inserted some of your comments close to the areas concerned, to make it easier for you 
> to remember what needed to be fixed.
> 
> I'm still a bit uncertain about this first long switch-case clause.
> 
> 
> void release_addr (ip_addr)
>      struct iaddr;
> {
>   struct interface_info *ip;
>   struct client_lease *lp;
>   struct client_state *sp;
>   
>   for (ip = interfaces; ip; ip = ip -> next) {
>     /* switch taken from client_reinit handed through Ted.*/
>     /* What does it do, and is it complete? */
>     switch (ip -> client -> state) {
>     case S_SELECTING:
>       cancel_timeout (send_discover, ip);
>       break;
>       
>     case S_BOUND:
>       cancel_timeout (state_bound, ip);
>       break;
>       
>     case S_REBOOTING:
>     case S_REQUESTING:
>     case S_RENEWING:
>       cancel_timeout (send_request, ip);
>       break;
>       
>     case S_INIT:
>     case S_REBINDING:
>       break;
>     }
> 
> >Here you need an additional loop to go through all the client
> >structures hanging off of the interface.   Pseudo-interfaces are
> >implemented as a linked list of client state structures hanging off of
> >an interface structure.   The first client state in the list is the
> >one that is created when you declare the interface or when the DHCP
> >client discovers it at startup.
>     
>     /* scan all leases for a matching address */	
>     for (sp = ip -> client; sp; sp = sp -> next) {
>       pl = (struct client_lease *)0;
>       for (lp = sp -> leases; lp; lp = lp -> next) {  
> 	if (lp -> address.len == ip_addr.len &&
> 	    !memcmp (lp -> address.iabuf, ip_addr.iabuf,
> 		     ip_addr.len))
> 	  {
> 
> >What you should really probably do is check the lease to see if it's
> >expired, and if it hasn't, release it and rewrite it.   If you are
> >releasing leases after you've acquired new ones, the old lease will
> >never be active.
> 
> 	    if (lp -> expiry > cur_time ) { /* The lease has not
> 					       expired*/
> 	      make_release (ip, lp);
> 	      send_release (ip);
> 	      write_client_lease (ip, lp, 0);                     
> 		/* below is copied 
> 		   from clparse.c
> 		   if (pl)
> 		   pl -> next = lp -> next;
> 		   else 
> 		   client -> leases = lp -> next;
> 		   destroy_client_lease (lp); */
> 		   /* Would this make anything better? */
> 		return 1;
> 	    }
> 	  }
>       }
>     }
>   }
>   return 0; /* No matching leases found */
> }
> 
> 
> >> 				destroy_client_lease (lp);
> >You'd better not do this unless you take it out of the linked list of
> >leases and also make sure that the active pointer isn't pointing at
> >it.   Leases aren't refcnted, so there's no automatic cleanup - you
> >just have to traverse the list and do it yourself.
> 
> Do you think I should not destroy it, or should I use the commented piece of code 
> above, or perhaps something completely different?
> 
> 
> >You're correct - state_init() is specific to a client state structure,
> >not a lease structure.   You should add a loop to loop through the
> >client structures, and then only consider ip -> client -> active.
> >You should check the expiry time on the lease against cur_time to see
> >if the lease has expired - the expiry of a lease does not result in
> >client -> active being zeroed out.
> 
> Like this? 
> 
> void client_renew ()
> {
>   struct interface_info *ip;
>   struct client_state *sp;
>   /* cycle through all interfaces and try to renew the addresses */
>   for (ip = interfaces; ip; ip = ip -> next) {
>     for (sp -> client; sp; sp = sp -> next) {
>       if (sp -> active && sp -> active -> expiry > cur_time) { 
> 	    /* active lease still valid => state_bound */
> 		ip -> client -> state = S_BOUND;
> 	    state_bound (ip);
>       } 
>       else if (sp -> active && sp -> active -> expiry <= cur_time) {
> 		/* if the lease has expired we must(?) call state_init () */> 
> 		ip -> client -> state = S_INIT;
> 		state_init (ip);
>       }
>     }	
>   }
> }
> 
> 
> BTW, how's it going with that object management API of yours? Is it ready yet?
> 
> Thanks for spending time helping a novice like me! :-)
> 	
> 	/ Lelle