On 26/08/2022 16.59, Bin Meng wrote:
> On Thu, Aug 25, 2022 at 9:04 PM Thomas Huth <thuth@redhat.com> wrote:
>>
>> On 24/08/2022 11.40, Bin Meng wrote:
>>> From: Xuzhou Cheng <xuzhou.cheng@windriver.com>
>>>
>>> Socket communication in the libqtest and libqmp codes uses read()
>>> and write() which work on any file descriptor on *nix, and sockets
>>> in *nix are an example of a file descriptor.
>>>
>>> However sockets on Windows do not use *nix-style file descriptors,
>>> so read() and write() cannot be used on sockets on Windows.
>>> Switch over to use send() and recv() instead which work on both
>>> Windows and *nix.
>>>
>>> Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
>>> Signed-off-by: Bin Meng <bin.meng@windriver.com>
>>> ---
>>>
>>> tests/qtest/libqmp.c | 4 ++--
>>> tests/qtest/libqtest.c | 4 ++--
>>> 2 files changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/tests/qtest/libqmp.c b/tests/qtest/libqmp.c
>>> index ade26c15f0..995a39c1f8 100644
>>> --- a/tests/qtest/libqmp.c
>>> +++ b/tests/qtest/libqmp.c
>>> @@ -36,7 +36,7 @@ typedef struct {
>>>
>>> static void socket_send(int fd, const char *buf, size_t size)
>>> {
>>> - size_t res = qemu_write_full(fd, buf, size);
>>> + ssize_t res = send(fd, buf, size, 0);
>>
>> This way we're losing the extra logic from qemu_write_full() here (i.e. the
>> looping and EINTR handling) ... not sure whether that's really OK? Maybe you
>> have to introduce a qemu_send_full() first?
>>
>
> I am not sure if qemu_send_full() is really needed because there is an
> assert() right after the send() call.
That's just a sanity check ... I think this function still has to take care
of EINTR - it originally looked like this:
https://git.qemu.org/?p=qemu.git;a=commitdiff;h=c3e5704af19ac6
... and you can also see the while loop there.
Agree, that would be the correct thing to do.
Fwiw, the SOCKET vs fd situation is giving me some nervous feelings, sometimes.
For ex, as I checked recently, it seems close(fd) correctly closes the underlying SOCKET - as if closesocket() was called on it.. but this is not really documented.
And it's easy to mix fd vs SOCKET in QEMU code paths (we cast/map SOCKET to "int fd" in general), and reach a close() on a SOCKET. That wouldn't be valid, and would likely create leaks or other issues.
Maybe we should introduce a type for safety / documentation purposes...