On 8/19/22 10:41, Erik Skultety wrote:
> Coverity reports:
>     virNWFilterSnoopIPLeaseUpdate(virNWFilterSnoopIPLease *ipl,
>                                   time_t timeout)
>     {
>         if (timeout < ipl->timeout)
>             return;  /* no take-backs */
> 
>         virNWFilterSnoopIPLeaseTimerDel(ipl);
>  >>> CID 396747:  High impact quality  (Y2K38_SAFETY)
>  >>> A "time_t" value is stored in an integer with too few bits
>      to accommodate it. The expression "timeout" is cast to
>      "unsigned int".
>         ipl->timeout = timeout;
>         virNWFilterSnoopIPLeaseTimerAdd(ipl);
>     }
> 
> This is a safe fix, since time_t is just long int and scales
> automatically with platform (more specifically it's 64bit on all
> platforms we care about).
> 
> Signed-off-by: Erik Skultety <eskul...@redhat.com>
> ---
>  src/nwfilter/nwfilter_dhcpsnoop.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c 
> b/src/nwfilter/nwfilter_dhcpsnoop.c
> index a10a14cfc1..4133d4c672 100644
> --- a/src/nwfilter/nwfilter_dhcpsnoop.c
> +++ b/src/nwfilter/nwfilter_dhcpsnoop.c
> @@ -146,7 +146,7 @@ struct _virNWFilterSnoopIPLease {
>      virSocketAddr              ipAddress;
>      virSocketAddr              ipServer;
>      virNWFilterSnoopReq *    snoopReq;
> -    unsigned int               timeout;
> +    time_t timeout;
>      /* timer list */
>      virNWFilterSnoopIPLease *prev;
>      virNWFilterSnoopIPLease *next;
> @@ -1580,7 +1580,7 @@ virNWFilterSnoopLeaseFileWrite(int lfd, const char 
> *ifkey,
>          return -1;
>  
>      /* time intf ip dhcpserver */
> -    lbuf = g_strdup_printf("%u %s %s %s\n", ipl->timeout, ifkey, ipstr, 
> dhcpstr);
> +    lbuf = g_strdup_printf("%lu %s %s %s\n", ipl->timeout, ifkey, ipstr, 
> dhcpstr);

Doesn't this use of ->timeout need to be typecasted? I mean, we are not
guaranteed that time_t is unsigned long. Elsewhere in the code we even
typecast to long long.

Michal

Reply via email to