| Message ID | 20200413104631.12276-2-email@uajain.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Umang, Thank you for the patch. On Mon, Apr 13, 2020 at 10:46:51AM +0000, Umang Jain wrote: > Pointed out by Coverity DefectId=279099 > > Signed-off-by: Umang Jain <email@uajain.com> > --- > test/ipc/unixsocket.cpp | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp > index f53042b..5348f35 100644 > --- a/test/ipc/unixsocket.cpp > +++ b/test/ipc/unixsocket.cpp > @@ -145,6 +145,7 @@ private: > > if (num < 0) { > cerr << "Read failed" << endl; > + close(outfd); > stop(-EIO); > return; > } else if (!num) > @@ -152,6 +153,7 @@ private: > > if (write(outfd, buf, num) < 0) { > cerr << "Write failed" << endl; > + close(outfd); > stop(-EIO); > return; > } I wonder if we should extend the File class to support read and write, and use it here to simplify the cleanup paths, but that's probably a bit too much just to fix this test, so Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Hi Laurent, On 13/04/2020 13:34, Laurent Pinchart wrote: > Hi Umang, > > Thank you for the patch. > > On Mon, Apr 13, 2020 at 10:46:51AM +0000, Umang Jain wrote: >> Pointed out by Coverity DefectId=279099 >> >> Signed-off-by: Umang Jain <email@uajain.com> >> --- >> test/ipc/unixsocket.cpp | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp >> index f53042b..5348f35 100644 >> --- a/test/ipc/unixsocket.cpp >> +++ b/test/ipc/unixsocket.cpp >> @@ -145,6 +145,7 @@ private: >> >> if (num < 0) { >> cerr << "Read failed" << endl; >> + close(outfd); >> stop(-EIO); >> return; >> } else if (!num) >> @@ -152,6 +153,7 @@ private: >> >> if (write(outfd, buf, num) < 0) { >> cerr << "Write failed" << endl; >> + close(outfd); >> stop(-EIO); >> return; >> } > > I wonder if we should extend the File class to support read and write, > and use it here to simplify the cleanup paths, but that's probably a bit > too much just to fix this test, so For 'now' I agree - that would then be extending our libcamera framework classes just to provide test functionality, however once we start needing to read/write to files for ... say configuration files - I can see we might well want to extend our File class. At that point - (after that's done) it might well be worth converting any file accesses to use our helper class to bring in all the RAII benefits and wrappings. But for this bug.. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> Though I see the File class didn't exist when this patch was posted either - but it's in now :-) > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >
diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp index f53042b..5348f35 100644 --- a/test/ipc/unixsocket.cpp +++ b/test/ipc/unixsocket.cpp @@ -145,6 +145,7 @@ private: if (num < 0) { cerr << "Read failed" << endl; + close(outfd); stop(-EIO); return; } else if (!num) @@ -152,6 +153,7 @@ private: if (write(outfd, buf, num) < 0) { cerr << "Write failed" << endl; + close(outfd); stop(-EIO); return; }
Pointed out by Coverity DefectId=279099 Signed-off-by: Umang Jain <email@uajain.com> --- test/ipc/unixsocket.cpp | 2 ++ 1 file changed, 2 insertions(+)