[PATCH 3/3] libxl: use time_t for qmp_synchronous_send()'s last parameter

Jan Beulich posted 3 patches 3 years, 5 months ago
[PATCH 3/3] libxl: use time_t for qmp_synchronous_send()'s last parameter
Posted by Jan Beulich 3 years, 5 months ago
"int" is not a suitable type to hold / receive "time_t" values.

The parameter is presently unused, so no functional change.

Coverity ID: 1509377
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
An obvious alternative would be to drop the parameter for being unused,
but I assume there were plans to use it in some way.

--- a/tools/libs/light/libxl_qmp.c
+++ b/tools/libs/light/libxl_qmp.c
@@ -582,7 +582,7 @@ out:
 static int qmp_synchronous_send(libxl__qmp_handler *qmp, const char *cmd,
                                 libxl__json_object *args,
                                 qmp_callback_t callback, void *opaque,
-                                int ask_timeout)
+                                time_t ask_timeout)
 {
     int id = 0;
     int ret = 0;
Re: [PATCH 3/3] libxl: use time_t for qmp_synchronous_send()'s last parameter
Posted by Anthony PERARD 3 years, 5 months ago
On Thu, Aug 18, 2022 at 04:07:16PM +0200, Jan Beulich wrote:
> "int" is not a suitable type to hold / receive "time_t" values.
> 
> The parameter is presently unused, so no functional change.
> 
> Coverity ID: 1509377
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Anthony PERARD <anthony.perard@citrix.com>

> ---
> An obvious alternative would be to drop the parameter for being unused,
> but I assume there were plans to use it in some way.

I'd rather drop the function all together instead, it doesn't get much
use these days as it's been replaced in many cases.

On the other hand, there seems to be only one call site, it would
be only one extra change to drop it, so that would be one way to fix
this. As you wish.

Thanks,

-- 
Anthony PERARD
Re: [PATCH 3/3] libxl: use time_t for qmp_synchronous_send()'s last parameter
Posted by Juergen Gross 3 years, 5 months ago
On 18.08.22 16:07, Jan Beulich wrote:
> "int" is not a suitable type to hold / receive "time_t" values.
> 
> The parameter is presently unused, so no functional change.
> 
> Coverity ID: 1509377
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Juergen Gross <jgross@suse.com>

The severity of this issue is rather low IMO. A timeout of more than
60 years not being handled correctly seems to have no relevance at all.


Juergen
Re: [PATCH 3/3] libxl: use time_t for qmp_synchronous_send()'s last parameter
Posted by Jan Beulich 3 years, 5 months ago
On 18.08.2022 16:20, Juergen Gross wrote:
> On 18.08.22 16:07, Jan Beulich wrote:
>> "int" is not a suitable type to hold / receive "time_t" values.
>>
>> The parameter is presently unused, so no functional change.
>>
>> Coverity ID: 1509377
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Reviewed-by: Juergen Gross <jgross@suse.com>

Thanks.

> The severity of this issue is rather low IMO. A timeout of more than
> 60 years not being handled correctly seems to have no relevance at all.

Agreed. The tool can't tell that a time_t-type value is used here for
a timeout, not a time stamp, and using the correct type is Generally
Better™ anyway, imo.

Jan