| Message ID | 1554214787-16594-1-git-send-email-awais_belal@mentor.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v2] client: simply return if async start fails | expand |
Hi Awais, Thanks for the patch. On Wednesday, 3 April 2019 01:20:01 UTC+11, Awais Belal wrote: > The client should return after cleaning up if async > start fails for any reason otherwise the caller will > block because the client app will keep on waiting for > the mutex which will never be triggered. > > Signed-off-by: Awais Belal <awais_belal@mentor.com> > --- > tools/client.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/tools/client.c b/tools/client.c > index 6dd378c..aa29c0f 100644 > --- a/tools/client.c > +++ b/tools/client.c > @@ -113,8 +113,13 @@ static int send_file(const char* filename) { > > rc = swupdate_async_start(readimage, printstatus, > end, dryrun); > - if (rc) > - printf("swupdate_async_start returns %d\n", rc); > + printf("swupdate_async_start returns %d\n", rc); > + /* return if we've hit an error scenario */ > + if (rc < 0) { > + pthread_mutex_unlock(&mymutex); > + close(fd); > + return 1; It was suggested by the maintainer in a recent change in this area to use EXIT_FAILURE for return values (even though these return values don't translate into an exit status of the client utility). Suggest returning EXIT_FAILURE on error for consistency. I'd suggest also fixing the leak of the open fd in the success case. > + } > > /* Now block */ > pthread_mutex_lock(&mymutex); > -- > 2.7.4 Regards, Austin
Hi Austin, Thanks for providing feedback on this. > > It was suggested by the maintainer in a recent change in this area to use EXIT_FAILURE for return values (even though these return values don't translate into an exit status of the client utility). Suggest returning EXIT_FAILURE on error for consistency. > > I'd suggest also fixing the leak of the open fd in the success case. > Makes sense. I'll submit an update. BR, Awais
diff --git a/tools/client.c b/tools/client.c index 6dd378c..aa29c0f 100644 --- a/tools/client.c +++ b/tools/client.c @@ -113,8 +113,13 @@ static int send_file(const char* filename) { rc = swupdate_async_start(readimage, printstatus, end, dryrun); - if (rc) - printf("swupdate_async_start returns %d\n", rc); + printf("swupdate_async_start returns %d\n", rc); + /* return if we've hit an error scenario */ + if (rc < 0) { + pthread_mutex_unlock(&mymutex); + close(fd); + return 1; + } /* Now block */ pthread_mutex_lock(&mymutex);
The client should return after cleaning up if async start fails for any reason otherwise the caller will block because the client app will keep on waiting for the mutex which will never be triggered. Signed-off-by: Awais Belal <awais_belal@mentor.com> --- tools/client.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)