diff mbox series

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

Message ID 9ac207d1-8a20-b880-e564-57494bc5b551@suse.com (mailing list archive)
State New, archived
Headers show
Series tools: address recently reported Coverity issues | expand

Commit Message

Jan Beulich Aug. 18, 2022, 2:07 p.m. UTC
"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.

Comments

Jürgen Groß Aug. 18, 2022, 2:20 p.m. UTC | #1
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
Jan Beulich Aug. 18, 2022, 2:35 p.m. UTC | #2
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
Anthony PERARD Aug. 18, 2022, 4:14 p.m. UTC | #3
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,
diff mbox series

Patch

--- 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;