Re: [PATCH] 9pfs: fix dead code in qemu_open_flags_tostr()

[Christian Schoenebeck <qemu_oss@crudebyte.com>]

On Tuesday, February 11, 2025 3:47:33 PM CET Peter Maydell wrote:
> On Mon, 10 Feb 2025 at 14:40, Christian Schoenebeck
> <qemu_oss@crudebyte.com> wrote:
> >
> > Coverity scan complained about expression "|LARGEFILE" to be non reachable
> > and the detailed Coverity report claims O_LARGEFILE was zero. I can't
> > reproduce this here, but I assume that means there are at least some
> > system(s) which define O_LARGEFILE as zero.
> >
> > This is not really an issue, but to silence this Coverity warning, add a
> > preprocessor wrapper that checks for O_LARGEFILE being non-zero for this
> > overall expression. The 'defined(O_LARGEFILE)' check is not necessary,
> > but it makes it more clear that we really want to check for the value of
> > O_LARGEFILE, not just whether the macro was defined.
> >
> > Fixes: 9a0dd4b3
> > Resolves: Coverity CID 1591178
> > Reported-by: Coverity Scan
> > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > ---
> >  hw/9pfs/9p-util-generic.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/hw/9pfs/9p-util-generic.c b/hw/9pfs/9p-util-generic.c
> > index 4c1e9c887d..02e359f17b 100644
> > --- a/hw/9pfs/9p-util-generic.c
> > +++ b/hw/9pfs/9p-util-generic.c
> > @@ -19,7 +19,9 @@ char *qemu_open_flags_tostr(int flags)
> >          #ifdef O_DIRECT
> >          (flags & O_DIRECT) ? "|DIRECT" : "",
> >          #endif
> > +        #if defined(O_LARGEFILE) && O_LARGEFILE != 0
> >          (flags & O_LARGEFILE) ? "|LARGEFILE" : "",
> > +        #endif
> >          (flags & O_DIRECTORY) ? "|DIRECTORY" : "",
> >          (flags & O_NOFOLLOW) ? "|NOFOLLOW" : "",
> >          #ifdef O_NOATIME
> 
> I don't think we need to make this change -- the code is
> correct, and osdep.h defines O_LARGEFILE if the system doesn't,
> exactly so that we don't need to put in extra ifdefs in the
> code itself. Coverity often fails to understand that some
> code is not dead in a different configuration or host OS
> than the one that got scanned. I've marked the issue as
> a false-positive in the Coverity UI.

Fine with me, thanks!

/Christian