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

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@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); 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, ifkey, ipstr, srvstr) < 4) { virReportError(VIR_ERR_INTERNAL_ERROR, _("virNWFilterSnoopLeaseFileLoad lease file " -- 2.31.1

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@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@redhat.com> Michal
participants (2)
-
Michal Prívozník
-
Shaleen Bathla