[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 16/52] migration/rdma: Fix io_writev(), io_readv() methods to
From: |
Fabiano Rosas |
Subject: |
Re: [PATCH 16/52] migration/rdma: Fix io_writev(), io_readv() methods to obey contract |
Date: |
Mon, 18 Sep 2023 15:57:31 -0300 |
Markus Armbruster <armbru@redhat.com> writes:
> QIOChannelClass methods qio_channel_rdma_readv() and
> qio_channel_rdma_writev() violate their method contract when
> rdma->error_state is non-zero:
>
> 1. They return whatever is in rdma->error_state then. Only -1 will be
> fine. -2 will be misinterpreted as "would block". Anything less
> than -2 isn't defined in the contract. A positive value would be
> misinterpreted as success, but I believe that's not actually
> possible.
>
> 2. They neglect to set an error then. If something up the call stack
> dereferences the error when failure is returned, it will crash. If
> it ignores the return value and checks the error instead, it will
> miss the error.
>
> Crap like this happens when return statements hide in macros,
> especially when their uses are far away from the definition.
>
> I elected not to investigate how callers are impacted.
>
> Expand the two bad macro uses, so we can set an error and return -1.
> The next commit will then get rid of the macro altogether.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
- Re: [PATCH 17/52] migration/rdma: Replace dangerous macro CHECK_ERROR_STATE(), (continued)
- [PATCH 16/52] migration/rdma: Fix io_writev(), io_readv() methods to obey contract, Markus Armbruster, 2023/09/18
- [PATCH 22/52] migration/rdma: Fix rdma_getaddrinfo() error checking, Markus Armbruster, 2023/09/18
- [PATCH 45/52] migration/rdma: Silence qemu_rdma_connect(), Markus Armbruster, 2023/09/18
- [PATCH 48/52] migration/rdma: Silence qemu_rdma_block_for_wrid(), Markus Armbruster, 2023/09/18
- [PATCH 49/52] migration/rdma: Silence qemu_rdma_register_and_get_keys(), Markus Armbruster, 2023/09/18
- [PATCH 52/52] migration/rdma: Fix how we show device details on open, Markus Armbruster, 2023/09/18