| Message ID | 1554278235-24200-1-git-send-email-awais_belal@mentor.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v3] client: simply return if async start fails, fix fd leak | expand |
Hi Awais, On 03/04/19 09:57, 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. Also fix a > leaked file descriptor in case of a successful run > plus align return values. > > Signed-off-by: Awais Belal <awais_belal@mentor.com> > --- > tools/client.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/tools/client.c b/tools/client.c > index 6dd378c..c1cdde8 100644 > --- a/tools/client.c > +++ b/tools/client.c > @@ -101,7 +101,7 @@ static int send_file(const char* filename) { > int rc; > if ( (fd = open(filename, O_RDONLY)) < 0) { > printf ("I cannot open %s\n", filename); > - return 1; > + return EXIT_FAILURE; > } > > /* synchronize with a mutex */ > @@ -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); This is annoying - even in case there is no error, it prints on the stdout a not useful message. I will let the behavior as before (print something just in case of error). As I have taken a look again at the code, I will also suggest if you want to cleanup in a separate patch calls to printf and replacing them with fprintf(stderr,... > + /* return if we've hit an error scenario */ > + if (rc < 0) { > + pthread_mutex_unlock(&mymutex); > + close(fd); > + return EXIT_FAILURE; > + } > > /* Now block */ > pthread_mutex_lock(&mymutex); > @@ -122,6 +127,8 @@ static int send_file(const char* filename) { > /* End called, unlock and exit */ > pthread_mutex_unlock(&mymutex); > > + close(fd); > + > return end_status; > } > > Best regards, Stefano Babic
diff --git a/tools/client.c b/tools/client.c index 6dd378c..c1cdde8 100644 --- a/tools/client.c +++ b/tools/client.c @@ -101,7 +101,7 @@ static int send_file(const char* filename) { int rc; if ( (fd = open(filename, O_RDONLY)) < 0) { printf ("I cannot open %s\n", filename); - return 1; + return EXIT_FAILURE; } /* synchronize with a mutex */ @@ -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 EXIT_FAILURE; + } /* Now block */ pthread_mutex_lock(&mymutex); @@ -122,6 +127,8 @@ static int send_file(const char* filename) { /* End called, unlock and exit */ pthread_mutex_unlock(&mymutex); + close(fd); + return end_status; }
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. Also fix a leaked file descriptor in case of a successful run plus align return values. Signed-off-by: Awais Belal <awais_belal@mentor.com> --- tools/client.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)