[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH RESEND] monitor: Fix return type of monitor_fdse
From: |
Yury Kotov |
Subject: |
Re: [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