* [Qemu-devel] [PATCH] qemu-char: check errno together with ret < 0
@ 2018-07-04 3:36 xinhua.Cao
2018-07-04 9:20 ` Daniel P. Berrangé
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: xinhua.Cao @ 2018-07-04 3:36 UTC (permalink / raw)
To: pbonzini, marcandre.lureau, eblake, anton.nefedov, vsementsov
Cc: weidong.huang, weifuqiang, king.wang, arei.gonglei, liuyongan,
qemu-devel, xinhua.Cao
In the tcp_chr_write function, we checked errno,
but errno was not reset before a read or write operation.
Therefore, this check of errno's actions is often
incorrect after EAGAIN has occurred.
we need check errno together with ret < 0.
Signed-off-by: xinhua.Cao <caoxinhua@huawei.com>
---
chardev/char-socket.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 17519ec..efbad6e 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -134,8 +134,11 @@ static int tcp_chr_write(Chardev *chr, const uint8_t *buf, int len)
s->write_msgfds,
s->write_msgfds_num);
- /* free the written msgfds in any cases other than errno==EAGAIN */
- if (EAGAIN != errno && s->write_msgfds_num) {
+ /* free the written msgfds in any cases
+ * other than ret < 0 && errno == EAGAIN
+ */
+ if (!(ret < 0 && EAGAIN == errno)
+ && s->write_msgfds_num) {
g_free(s->write_msgfds);
s->write_msgfds = 0;
s->write_msgfds_num = 0;
--
2.8.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] qemu-char: check errno together with ret < 0
2018-07-04 3:36 [Qemu-devel] [PATCH] qemu-char: check errno together with ret < 0 xinhua.Cao
@ 2018-07-04 9:20 ` Daniel P. Berrangé
2018-07-04 9:36 ` Marc-André Lureau
2018-07-04 11:11 ` Markus Armbruster
2 siblings, 0 replies; 5+ messages in thread
From: Daniel P. Berrangé @ 2018-07-04 9:20 UTC (permalink / raw)
To: xinhua.Cao
Cc: pbonzini, marcandre.lureau, eblake, anton.nefedov, vsementsov,
weidong.huang, weifuqiang, qemu-devel, king.wang, liuyongan,
arei.gonglei
On Wed, Jul 04, 2018 at 11:36:42AM +0800, xinhua.Cao wrote:
> In the tcp_chr_write function, we checked errno,
> but errno was not reset before a read or write operation.
> Therefore, this check of errno's actions is often
> incorrect after EAGAIN has occurred.
> we need check errno together with ret < 0.
>
> Signed-off-by: xinhua.Cao <caoxinhua@huawei.com>
> ---
> chardev/char-socket.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index 17519ec..efbad6e 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -134,8 +134,11 @@ static int tcp_chr_write(Chardev *chr, const uint8_t *buf, int len)
> s->write_msgfds,
> s->write_msgfds_num);
>
> - /* free the written msgfds in any cases other than errno==EAGAIN */
> - if (EAGAIN != errno && s->write_msgfds_num) {
> + /* free the written msgfds in any cases
> + * other than ret < 0 && errno == EAGAIN
> + */
> + if (!(ret < 0 && EAGAIN == errno)
> + && s->write_msgfds_num) {
> g_free(s->write_msgfds);
> s->write_msgfds = 0;
> s->write_msgfds_num = 0;
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] qemu-char: check errno together with ret < 0
2018-07-04 3:36 [Qemu-devel] [PATCH] qemu-char: check errno together with ret < 0 xinhua.Cao
2018-07-04 9:20 ` Daniel P. Berrangé
@ 2018-07-04 9:36 ` Marc-André Lureau
2018-07-04 9:38 ` Paolo Bonzini
2018-07-04 11:11 ` Markus Armbruster
2 siblings, 1 reply; 5+ messages in thread
From: Marc-André Lureau @ 2018-07-04 9:36 UTC (permalink / raw)
To: xinhua.Cao
Cc: Paolo Bonzini, Eric Blake, Anton Nefedov,
Vladimir Sementsov-Ogievskiy, weidong.huang, weifuqiang, QEMU,
king.wang, liuyongan, Gonglei
On Wed, Jul 4, 2018 at 5:36 AM, xinhua.Cao <caoxinhua@huawei.com> wrote:
> In the tcp_chr_write function, we checked errno,
> but errno was not reset before a read or write operation.
> Therefore, this check of errno's actions is often
> incorrect after EAGAIN has occurred.
> we need check errno together with ret < 0.
>
> Signed-off-by: xinhua.Cao <caoxinhua@huawei.com>
Can you point out this is fixing commit
9fc53a10f81d3a9027b23fa810147d21be29e614 in commit message?
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> chardev/char-socket.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index 17519ec..efbad6e 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -134,8 +134,11 @@ static int tcp_chr_write(Chardev *chr, const uint8_t *buf, int len)
> s->write_msgfds,
> s->write_msgfds_num);
>
> - /* free the written msgfds in any cases other than errno==EAGAIN */
> - if (EAGAIN != errno && s->write_msgfds_num) {
> + /* free the written msgfds in any cases
> + * other than ret < 0 && errno == EAGAIN
> + */
> + if (!(ret < 0 && EAGAIN == errno)
> + && s->write_msgfds_num) {
> g_free(s->write_msgfds);
> s->write_msgfds = 0;
> s->write_msgfds_num = 0;
> --
> 2.8.3
>
>
>
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] qemu-char: check errno together with ret < 0
2018-07-04 9:36 ` Marc-André Lureau
@ 2018-07-04 9:38 ` Paolo Bonzini
0 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2018-07-04 9:38 UTC (permalink / raw)
To: Marc-André Lureau, xinhua.Cao
Cc: Eric Blake, Anton Nefedov, Vladimir Sementsov-Ogievskiy,
weidong.huang, weifuqiang, QEMU, king.wang, liuyongan, Gonglei
On 04/07/2018 11:36, Marc-André Lureau wrote:
> On Wed, Jul 4, 2018 at 5:36 AM, xinhua.Cao <caoxinhua@huawei.com> wrote:
>> In the tcp_chr_write function, we checked errno,
>> but errno was not reset before a read or write operation.
>> Therefore, this check of errno's actions is often
>> incorrect after EAGAIN has occurred.
>> we need check errno together with ret < 0.
>>
>> Signed-off-by: xinhua.Cao <caoxinhua@huawei.com>
>
>
> Can you point out this is fixing commit
> 9fc53a10f81d3a9027b23fa810147d21be29e614 in commit message?
Ok, done.
Thanks,
Paolo
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
>> ---
>> chardev/char-socket.c | 7 +++++--
>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
>> index 17519ec..efbad6e 100644
>> --- a/chardev/char-socket.c
>> +++ b/chardev/char-socket.c
>> @@ -134,8 +134,11 @@ static int tcp_chr_write(Chardev *chr, const uint8_t *buf, int len)
>> s->write_msgfds,
>> s->write_msgfds_num);
>>
>> - /* free the written msgfds in any cases other than errno==EAGAIN */
>> - if (EAGAIN != errno && s->write_msgfds_num) {
>> + /* free the written msgfds in any cases
>> + * other than ret < 0 && errno == EAGAIN
>> + */
>> + if (!(ret < 0 && EAGAIN == errno)
>> + && s->write_msgfds_num) {
>> g_free(s->write_msgfds);
>> s->write_msgfds = 0;
>> s->write_msgfds_num = 0;
>> --
>> 2.8.3
>>
>>
>>
>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] qemu-char: check errno together with ret < 0
2018-07-04 3:36 [Qemu-devel] [PATCH] qemu-char: check errno together with ret < 0 xinhua.Cao
2018-07-04 9:20 ` Daniel P. Berrangé
2018-07-04 9:36 ` Marc-André Lureau
@ 2018-07-04 11:11 ` Markus Armbruster
2 siblings, 0 replies; 5+ messages in thread
From: Markus Armbruster @ 2018-07-04 11:11 UTC (permalink / raw)
To: xinhua.Cao
Cc: pbonzini, marcandre.lureau, eblake, anton.nefedov, vsementsov,
weidong.huang, weifuqiang, qemu-devel, king.wang, liuyongan,
arei.gonglei
xinhua.Cao <caoxinhua@huawei.com> writes:
> In the tcp_chr_write function, we checked errno,
> but errno was not reset before a read or write operation.
> Therefore, this check of errno's actions is often
> incorrect after EAGAIN has occurred.
> we need check errno together with ret < 0.
>
> Signed-off-by: xinhua.Cao <caoxinhua@huawei.com>
> ---
> chardev/char-socket.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index 17519ec..efbad6e 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -134,8 +134,11 @@ static int tcp_chr_write(Chardev *chr, const uint8_t *buf, int len)
> s->write_msgfds,
> s->write_msgfds_num);
>
> - /* free the written msgfds in any cases other than errno==EAGAIN */
> - if (EAGAIN != errno && s->write_msgfds_num) {
> + /* free the written msgfds in any cases
> + * other than ret < 0 && errno == EAGAIN
> + */
> + if (!(ret < 0 && EAGAIN == errno)
> + && s->write_msgfds_num) {
> g_free(s->write_msgfds);
> s->write_msgfds = 0;
> s->write_msgfds_num = 0;
The comment feels pretty worthless to me.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-07-04 11:11 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-07-04 3:36 [Qemu-devel] [PATCH] qemu-char: check errno together with ret < 0 xinhua.Cao
2018-07-04 9:20 ` Daniel P. Berrangé
2018-07-04 9:36 ` Marc-André Lureau
2018-07-04 9:38 ` Paolo Bonzini
2018-07-04 11:11 ` Markus Armbruster
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).