| Message ID | CAPxu4we3FZf9n1p5iV7bt024WecBfat+wta-pRFzoOn3bHgdXQ@mail.gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | fix progress_ipc_receive when non-blocking | expand |
Hi Thomas, On 09.07.20 21:44, Thomas W. wrote: > From 77f4da898bff0941d0326caf793cd74515e4b765 Mon Sep 17 00:00:00 2001 > From: thomas <thomas.chiantia@gmail.com> > Date: Thu, 9 Jul 2020 14:50:03 -0400 > Subject: [PATCH] add non blocking progress-ipc support > > Signed-off-by: Thomas Chiantia <thomas.chiantia@gmail.com> > You should add Stefan's Signed-off-by, too, as first submitter for the patch. > --- > ipc/progress_ipc.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/ipc/progress_ipc.c b/ipc/progress_ipc.c > index 110d6b3..b9a1309 100644 > --- a/ipc/progress_ipc.c > +++ b/ipc/progress_ipc.c > @@ -7,6 +7,7 @@ > > #include <sys/socket.h> > #include <sys/un.h> > +#include <errno.h> > #include <string.h> > #include <stdio.h> > #include <stdlib.h> > @@ -78,11 +79,16 @@ int progress_ipc_connect(bool reconnect) > > int progress_ipc_receive(int *connfd, struct progress_msg *msg) { > int ret = read(*connfd, msg, sizeof(*msg)); > + > + if (ret == -1 && errno == EAGAIN) > + return 0; > + Who is responsible to set the socket in non blocking mode ? This is not done by progress_ipc_connect(). Is it responsibility of the caller to do this ? > if (ret != sizeof(*msg)) { > fprintf(stdout, "Connection closing..\n"); > close(*connfd); > *connfd = -1; > return -1; > } > + > return ret; > } > Regards, Stefano Babic
Signed-off-by: Thomas Chiantia <thomas.chiantia@gmail.com> Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com> --- ipc/progress_ipc.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ipc/progress_ipc.c b/ipc/progress_ipc.c index 110d6b3..b9a1309 100644 --- a/ipc/progress_ipc.c +++ b/ipc/progress_ipc.c @@ -7,6 +7,7 @@ #include <sys/socket.h> #include <sys/un.h> +#include <errno.h> #include <string.h> #include <stdio.h> #include <stdlib.h> @@ -78,11 +79,16 @@ int progress_ipc_connect(bool reconnect) int progress_ipc_receive(int *connfd, struct progress_msg *msg) { int ret = read(*connfd, msg, sizeof(*msg)); + + if (ret == -1 && errno == EAGAIN) + return 0; + if (ret != sizeof(*msg)) { fprintf(stdout, "Connection closing..\n"); close(*connfd); *connfd = -1; return -1; } + return ret; } -- 2.21.3 On Fri, Jul 10, 2020 at 5:44 AM Stefano Babic <sbabic@denx.de> wrote: > > Hi Thomas, > > On 09.07.20 21:44, Thomas W. wrote: > > From 77f4da898bff0941d0326caf793cd74515e4b765 Mon Sep 17 00:00:00 2001 > > From: thomas <thomas.chiantia@gmail.com> > > Date: Thu, 9 Jul 2020 14:50:03 -0400 > > Subject: [PATCH] add non blocking progress-ipc support > > > > Signed-off-by: Thomas Chiantia <thomas.chiantia@gmail.com> > > > > You should add Stefan's Signed-off-by, too, as first submitter for the > patch. > > > --- > > ipc/progress_ipc.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/ipc/progress_ipc.c b/ipc/progress_ipc.c > > index 110d6b3..b9a1309 100644 > > --- a/ipc/progress_ipc.c > > +++ b/ipc/progress_ipc.c > > @@ -7,6 +7,7 @@ > > > > #include <sys/socket.h> > > #include <sys/un.h> > > +#include <errno.h> > > #include <string.h> > > #include <stdio.h> > > #include <stdlib.h> > > @@ -78,11 +79,16 @@ int progress_ipc_connect(bool reconnect) > > > > int progress_ipc_receive(int *connfd, struct progress_msg *msg) { > > int ret = read(*connfd, msg, sizeof(*msg)); > > + > > + if (ret == -1 && errno == EAGAIN) > > + return 0; > > + > > Who is responsible to set the socket in non blocking mode ? This is not > done by progress_ipc_connect(). Is it responsibility of the caller to do > this ? > > > if (ret != sizeof(*msg)) { > > fprintf(stdout, "Connection closing..\n"); > > close(*connfd); > > *connfd = -1; > > return -1; > > } > > + > > return ret; > > } > > > > Regards, > Stefano Babic > > -- > ===================================================================== > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de > =====================================================================
> Who is responsible to set the socket in non blocking mode ? This is not > done by progress_ipc_connect(). Is it responsibility of the caller to do > this ? Yes - this is similar behavior to how mongoose will set non blocking mode when using network_ipc to begin an update. It might be better to have the connect API take some options to configure non blocking mode instead of requiring the caller to manipulate the socket outside of API. But because this is the pattern used with the network_ipc socket then the same pattern should be used here. On Fri, Jul 10, 2020 at 5:44 AM Stefano Babic <sbabic@denx.de> wrote: > > Hi Thomas, > > On 09.07.20 21:44, Thomas W. wrote: > > From 77f4da898bff0941d0326caf793cd74515e4b765 Mon Sep 17 00:00:00 2001 > > From: thomas <thomas.chiantia@gmail.com> > > Date: Thu, 9 Jul 2020 14:50:03 -0400 > > Subject: [PATCH] add non blocking progress-ipc support > > > > Signed-off-by: Thomas Chiantia <thomas.chiantia@gmail.com> > > > > You should add Stefan's Signed-off-by, too, as first submitter for the > patch. > > > --- > > ipc/progress_ipc.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/ipc/progress_ipc.c b/ipc/progress_ipc.c > > index 110d6b3..b9a1309 100644 > > --- a/ipc/progress_ipc.c > > +++ b/ipc/progress_ipc.c > > @@ -7,6 +7,7 @@ > > > > #include <sys/socket.h> > > #include <sys/un.h> > > +#include <errno.h> > > #include <string.h> > > #include <stdio.h> > > #include <stdlib.h> > > @@ -78,11 +79,16 @@ int progress_ipc_connect(bool reconnect) > > > > int progress_ipc_receive(int *connfd, struct progress_msg *msg) { > > int ret = read(*connfd, msg, sizeof(*msg)); > > + > > + if (ret == -1 && errno == EAGAIN) > > + return 0; > > + > > Who is responsible to set the socket in non blocking mode ? This is not > done by progress_ipc_connect(). Is it responsibility of the caller to do > this ? > > > if (ret != sizeof(*msg)) { > > fprintf(stdout, "Connection closing..\n"); > > close(*connfd); > > *connfd = -1; > > return -1; > > } > > + > > return ret; > > } > > > > Regards, > Stefano Babic > > -- > ===================================================================== > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de > =====================================================================
diff --git a/ipc/progress_ipc.c b/ipc/progress_ipc.c index 110d6b3..b9a1309 100644 --- a/ipc/progress_ipc.c +++ b/ipc/progress_ipc.c @@ -7,6 +7,7 @@ #include <sys/socket.h> #include <sys/un.h> +#include <errno.h> #include <string.h> #include <stdio.h> #include <stdlib.h> @@ -78,11 +79,16 @@ int progress_ipc_connect(bool reconnect) int progress_ipc_receive(int *connfd, struct progress_msg *msg) { int ret = read(*connfd, msg, sizeof(*msg)); + + if (ret == -1 && errno == EAGAIN) + return 0; + if (ret != sizeof(*msg)) { fprintf(stdout, "Connection closing..\n"); close(*connfd); *connfd = -1; return -1; } + return ret; }