| Message ID | 20180704033642.15996-1-caoxinhua@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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 > > >
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 >> >> >> > > >
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.
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;
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(-)