qemu-trivial
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-trivial] [Qemu-devel] [PATCH RESEND] monitor: Fix return type


From: Yury Kotov
Subject: Re: [Qemu-trivial] [Qemu-devel] [PATCH RESEND] monitor: Fix return type of monitor_fdset_dup_fd_find
Date: Tue, 14 May 2019 17:51:49 +0300

14.05.2019, 17:05, "Eric Blake" <address@hidden>:
> On 5/14/19 8:15 AM, Yury Kotov wrote:
>>  monitor_fdset_dup_fd_find_remove() and monitor_fdset_dup_fd_find()
>>  returns mon_fdset->id which is int64_t. Downcast from int64_t to int leads 
>> to
>>  a bug with removing fd from fdset which id >= 2^32.
>>  So, fix return types for these function.
>
> fd's cannot exceed 2^32. We should instead be fixing anything that uses
> int64_t with an fd to be properly limited to 32 bits. That is, I think
> the real problem is in qapi/misc.json:
>
>  { 'struct': 'AddfdInfo', 'data': {'fdset-id': 'int', 'fd': 'int'} }
> instead of 'fd':'int32'. For that matter, 'fdset-id' larger than 32
> bits is unlikely to be useful (there's no reason to have more fdsets
> than you can have possible fds to put in those sets).
>
> NACK to this version, but a v2 that addresses the real problem is
> worthwhile.
>

Ok, so I will change fdset_id type int64_t -> int everywhere and
int64_t -> int32_t for qmp commands. Right?

>>  +++ b/include/monitor/monitor.h
>>  @@ -46,7 +46,7 @@ AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_=
>>  id, int64_t fdset_id,
>>   int monitor_fdset_get_fd(int64_t fdset_id, int flags);
>>   int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd);
>>   void monitor_fdset_dup_fd_remove(int dup_fd);
>>  -int monitor_fdset_dup_fd_find(int dup_fd);
>>  +int64_t monitor_fdset_dup_fd_find(int dup_fd);
>>  =20
>
> Your patch came through corrupted. You may want to double-check how you
> are sending them, to ensure they are not mangled.
>

Omg, sorry. I just copy-pasted my previous patch from mail client in raw format
and I did not notice the escape characters, because of which the patch is
incorrect.

Regards,
Yury



reply via email to

[Prev in Thread] Current Thread [Next in Thread]