[PATCH] arpping in udhcpc

[Michael Conrad]

If you both look closely at the function in question ("monotonic_ms") 
you will see that it is already 64-bit, and using the linux 
"CLOCK_MONOTONIC" which is never affected by changes to the system time.

Adrian is correct that the 'long long' return of monotonic_ms is being 
stored in a 'unsigned', but the existing implementation seems perfectly 
correct as long it doesn't sleep continuously for more than 49 
real-world days. (and it doesn't because the timeout is hard-coded at 
2000ms)

I think the safe_poll function is much more suspicious:

/* Wrapper which restarts poll on EINTR or ENOMEM.
  * On other errors does perror("poll") and returns.
  * Warning! May take longer than timeout_ms to return! */
int FAST_FUNC safe_poll(struct pollfd *ufds, nfds_t nfds, int timeout)
{
         while (1) {
                 int n = poll(ufds, nfds, timeout);
                 if (n >= 0)
                         return n;
                 /* Make sure we inch towards completion */
                 if (timeout > 0)
                         timeout--;
                 /* E.g. strace causes poll to return this */
                 if (errno == EINTR)
                         continue;
                 /* Kernel is very low on memory. Retry. */
                 /* I doubt many callers would handle this correctly! */
                 if (errno == ENOMEM)
                         continue;
                 bb_perror_msg("poll");
                 return n;
         }
}

So if it gets hit with a signal that causes EINTR, it subtracts 1 
millisecond and tries again, regardless of how much time actually elapsed???

-Mike

On 4/11/2012 12:45 PM, Cathey, Jim wrote:
> Is not one of the ways to manage delta-time
> timeouts to use uptime() instead of time()?
> You don't care what time it is, you only want
> to manage a timeout on the order of seconds,
> so a monotonic time source like uptime() is
> actually preferable.
>
> It seems to me that 64-bit math is unnecessarily
> punitive, especially for many embedded processors.
>
> Also, DST doesn't affect the value of time()
> et al. at all, or should not.
>
> -- Jim
>
> -----Original Message-----
> From: busybox-bounces at busybox.net [mailto:busybox-bounces at busybox.net]
> On Behalf Of Adrian Szyndela
> Sent: Wednesday, April 11, 2012 1:41 AM
> To: busybox at busybox.net
> Subject: [PATCH] arpping in udhcpc
>
> Hi,
>
> I'm attaching a patch for way too long wait on poll() during arpping
> after IP address acquiring over DHCP.
>
> It happens on a platform which starts with Epoch time. Then, at some
> point, a correct time source appears, and an actual time is set.
>
> If an actual time is set during wait on poll(), computing of a timeout
> overflows on unsigned type. The patch just changes types to stay with
> long long computing.
>
> Before applying the patch the side effect of setting an actual time is
> close to hangup - with no IP address assigned. After applying the patch
> the side effect of time change is that poll() might wait shorter than
> desired 2 seconds in case of socket activity.
>
> I found a comment /* We don't expect to see 1000+ seconds delay,
> unsigned is enough */ in networking/arping.c (which probably is affected
> by the problem as well). The above case introduces 1000000000+ seconds
> delay :). 3600 seconds at daylight saving time change is a piece of
> cake.
>
> Regards,
> Adrian
>
> _______________________________________________
> busybox mailing list
> busybox at busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox