| Message ID | 1590669005-31453-1-git-send-email-madhavan.krishnan@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hello Madhavan, On Thu, May 28, 2020 at 02:30:05PM +0200, Madhavan Krishnan wrote: > In write() function, return value is not handled and stored. > So, we faced the compilation issue regarding this. Which compilation error did you get ? Return value ignored ? On which compiler ? I don't think we've seens this with latest GCC and clang. > > Handled to store the return value of write() function, and > modified the test execution based on the return values. Sorry for being picky, but commit messages read better if they present the problem and the solution (like your does) but in the present tense. Like "The return value of write() function is ignored, causing the following compiler error|warning with $compiler_version Fix this by storing the return value of write() and return a test error in case of failure." > > Signed-off-by: Madhavan Krishnan <madhavan.krishnan@linaro.org> > --- > test/file.cpp | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/test/file.cpp b/test/file.cpp > index 6262a6f..85eacb5 100644 > --- a/test/file.cpp > +++ b/test/file.cpp > @@ -27,10 +27,15 @@ protected: > { > fileName_ = "/tmp/libcamera.test.XXXXXX"; > int fd = mkstemp(&fileName_.front()); > + ssize_t ret; > + > if (fd == -1) > return TestFail; > > - write(fd, "libcamera", 9); > + ret = write(fd, "libcamera", 9); You could delcare ret here ssize_t ret = > + if(ret == -1) > + return TestFail; And you should close the fd before returning. Anwyay, I would be interested in knowing the exact error and the compiler version which reported it first. Thanks j > + > close(fd); > > return TestPass; > -- > 2.7.4 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Madhavan, On 28/05/2020 13:30, Madhavan Krishnan wrote: > In write() function, return value is not handled and stored. > So, we faced the compilation issue regarding this. As mentioned by Jacopo, if you hit specific compilation problems, it would be helpful to explain what here. But this topic is also highlighted by our coverity scans, so I am happy to see this one resolved all the same. > Handled to store the return value of write() function, and > modified the test execution based on the return values. Please add the following tag to this patch: Reported-by: Coverity CID=284605 > Signed-off-by: Madhavan Krishnan <madhavan.krishnan@linaro.org> > --- > test/file.cpp | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/test/file.cpp b/test/file.cpp > index 6262a6f..85eacb5 100644 > --- a/test/file.cpp > +++ b/test/file.cpp > @@ -27,10 +27,15 @@ protected: > { > fileName_ = "/tmp/libcamera.test.XXXXXX"; > int fd = mkstemp(&fileName_.front()); > + ssize_t ret; > + > if (fd == -1) > return TestFail; > > - write(fd, "libcamera", 9); > + ret = write(fd, "libcamera", 9); > + if(ret == -1) Indeed, as highlighted by Jacopo, this is missing a close(fd) on this error path. > + return TestFail; > + > close(fd); > Alternatively, you could make the return value dependant upon ret? return (ret == 9) ? TestPass : TestFail; > return TestPass; > With the necessary updates made, Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Hello, On Tue, Jun 02, 2020 at 11:07:18AM +0100, Kieran Bingham wrote: > On 28/05/2020 13:30, Madhavan Krishnan wrote: > > In write() function, return value is not handled and stored. > > So, we faced the compilation issue regarding this. > > As mentioned by Jacopo, if you hit specific compilation problems, it > would be helpful to explain what here. > > But this topic is also highlighted by our coverity scans, so I am happy > to see this one resolved all the same. > > > Handled to store the return value of write() function, and > > modified the test execution based on the return values. > > Please add the following tag to this patch: > > Reported-by: Coverity CID=284605 > > > Signed-off-by: Madhavan Krishnan <madhavan.krishnan@linaro.org> > > --- > > test/file.cpp | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/test/file.cpp b/test/file.cpp > > index 6262a6f..85eacb5 100644 > > --- a/test/file.cpp > > +++ b/test/file.cpp > > @@ -27,10 +27,15 @@ protected: > > { > > fileName_ = "/tmp/libcamera.test.XXXXXX"; > > int fd = mkstemp(&fileName_.front()); > > + ssize_t ret; > > + > > if (fd == -1) > > return TestFail; > > > > - write(fd, "libcamera", 9); > > + ret = write(fd, "libcamera", 9); > > + if(ret == -1) > > Indeed, as highlighted by Jacopo, this is missing a close(fd) on this > error path. > > > + return TestFail; > > + > > close(fd); > > > > Alternatively, you could make the return value dependant upon ret? > > return (ret == 9) ? TestPass : TestFail; No need for parentheses. > > return TestPass; > > > > With the necessary updates made, > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
diff --git a/test/file.cpp b/test/file.cpp index 6262a6f..85eacb5 100644 --- a/test/file.cpp +++ b/test/file.cpp @@ -27,10 +27,15 @@ protected: { fileName_ = "/tmp/libcamera.test.XXXXXX"; int fd = mkstemp(&fileName_.front()); + ssize_t ret; + if (fd == -1) return TestFail; - write(fd, "libcamera", 9); + ret = write(fd, "libcamera", 9); + if(ret == -1) + return TestFail; + close(fd); return TestPass;
In write() function, return value is not handled and stored. So, we faced the compilation issue regarding this. Handled to store the return value of write() function, and modified the test execution based on the return values. Signed-off-by: Madhavan Krishnan <madhavan.krishnan@linaro.org> --- test/file.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)