On Fri, 30 Oct 2020 12:59:48 +0100 Christian Schoenebeck <qemu_...@crudebyte.com> wrote:
> On Freitag, 30. Oktober 2020 12:44:18 CET Greg Kurz wrote: > > On Fri, 30 Oct 2020 09:19:46 +0100 > > > > Christian Schoenebeck <qemu_...@crudebyte.com> wrote: > > > Coverity wants the return value of mkdir() to be checked, so let's > > > pretend to do that. We're actually just making a dummy check and > > > ignore the result, because we actually only care if the required > > > directory exists and we have an existence check for that in place > > > already. > > > > I see that sometimes changelog shows a copy of the original > > coverity report (e.g. commit df1a312fea58). > > Ok, I'll add that. > > > > Reported-by: Greg Kurz <gr...@kaod.org> > > > > Please give credits to coverity, not me :-) > > > > And most importantly, we want to mention the CID in the changelog. > > > > e.g. > > > > Reported-by: Coverity (CID 1435963) > > Ok. > > It's not clear to me where this coverity report is accessible online. A quick > search only brought me to statistics about its latest check, but not the > details of the report you quoted. > I've been notified by mail because I have an account there. https://scan.coverity.com/users/sign_up > And more importantly: is there coverity CI support that one could enable on > github, so that pending patches were checked before upstream merge? > I see that Peter already provided the details. > > > > > Signed-off-by: Christian Schoenebeck <qemu_...@crudebyte.com> > > > --- > > > > > > tests/qtest/libqos/virtio-9p.c | 6 +++++- > > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > > > diff --git a/tests/qtest/libqos/virtio-9p.c > > > b/tests/qtest/libqos/virtio-9p.c index 6b22fa0e9a..0a7c0ee5d8 100644 > > > --- a/tests/qtest/libqos/virtio-9p.c > > > +++ b/tests/qtest/libqos/virtio-9p.c > > > @@ -48,9 +48,13 @@ static void init_local_test_path(void) > > > > > > static void create_local_test_dir(void) > > > { > > > > > > struct stat st; > > > > > > + int res; > > > > > > g_assert(local_test_path != NULL); > > > > > > - mkdir(local_test_path, 0777); > > > + res = mkdir(local_test_path, 0777); > > > + if (res < 0) { > > > + /* ignore error, dummy check to prevent error by coverity */ > > > > Why not printing an error message with errno there like you did in > > the previous patch ? > > Yeah, originally I didn't want to trigger false positives on automated CIs if > mkdir() failed just because the directory already exists. But OTOH mkdtemp() should buy you the directory doesn't exist. > g_test_message() is just an info-level message, so it is Ok to bark silently > and I will add it. > > > > > > + } > > > > > > /* ensure test directory exists now ... */ > > > g_assert(stat(local_test_path, &st) == 0); > > Thanks! > > Best regards, > Christian Schoenebeck > >