[PATCH 1/1] nwfilter: use time_t for timeout for consistency

[Michal Prívozník]

On 12/28/22 15:47, Shaleen Bathla wrote:
> Coverity scan reports:
> "A time_t value is stored in an integer with too few bits to accommodate
> it. The expression timeout is cast to unsigned int"
> 
> We are already casting and storing time_t timeout variable into unsigned int.
> We can use time_t for timeout and cast it to unsigned long (should be big enough) 
> instead of unsigned int in sscanf, g_strdup_printf as required.
> 
> Signed-off-by: Shaleen Bathla <shaleen.bathla at oracle.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 a10a14cfc16d..62586be35dfb 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);

I don't think this is portable. We are not guaranteed that time_t is
unsigned long, are we? Not to mention that users can chose the size of
time_t at compile time (-D_TIME_BITS=64). Therefore, we want explicit
typecast. And just to be on the safe side - typecast to ULL:

lbus = g_strdup_printf("%llu ...", (unsigned long long) ipl->timeout, ...);

>      len = strlen(lbuf);
>  
>      if (safewrite(lfd, lbuf, len) != len) {
> @@ -1739,7 +1739,7 @@ virNWFilterSnoopLeaseFileLoad(void)
>          }
>          ln++;
>          /* key len 54 = "VMUUID"+'-'+"MAC" */
> -        if (sscanf(line, "%u %54s %15s %15s", &ipl.timeout,
> +        if (sscanf(line, "%lu %54s %15s %15s", &ipl.timeout,

Same here:

sscanf(line, "%llu ...", (unsigned long long *) &ipl.timeout, ...);

I'll fix it before pushing.

Reviewed-by: Michal Privoznik <mprivozn at redhat.com>

Michal