[prev in list] [next in list] [prev in thread] [next in thread] 

List:       activemq-dev
Subject:    Re: [DISCUSS] Coverity Scan for Artemis
From:       Clebert Suconic <clebert.suconic () gmail ! com>
Date:       2017-02-25 15:18:06
Message-ID: CAKF+bspP4mW1JsRJQjYW7_E-C1U6uh5at3YsoPzbA77Y0y54Vw () mail ! gmail ! com
[Download RAW message or body]

Where is this "// TODO BEFORE MERGE: (is null a good option here?)"


This some mark I usually do, I meant to fix it before I sent the commit :)

On Sat, Feb 25, 2017 at 5:27 AM, Jiri Danek <jdanek@redhat.com> wrote:
> (two corrections, I noticed I did not wrote what I meant, previously)
>
> On Sat, Feb 25, 2017 at 8:47 AM, Jiri Danek <jdanek@redhat.com> wrote:
>
>> The mkstemp() thing is supposed to be an issue only with old glibc on
>> Linux and on AIX and HP-UX. I googled out that there is a good way to set
>> umask() for the process, because there is JVM running in it too... So
>> probably ignore.
>
>
> I meant to say "there is _not_ a good way"
>
> On Sat, Feb 25, 2017 at 9:11 AM, Jiri Danek <jdanek@redhat.com> wrote:
>
>> Hello, let me sent one more e-mail in this thread ;P Trying to evaluate
>> what Coverity found, I opened some Jiras where I understood the problem and
>> it seemed real, and marked the Coverity find as ignored if .
>
>
> I meant to add "marked as ignored if I was reasonably sure that it is
> intended". You can filter for this on the Coverity web. There were two
> issues where it was obvious, and one where I think it is not worth fixing:
>
> 204                     if (futureListener != null) {
> 205
> // TODO BEFORE MERGE: (is null a good option here?)
>
> CID 1409858 (#1 of 1): Explicit null dereferenced (FORWARD_NULL)
> var_deref_model: Passing null to operationComplete, which dereferences it.
> (The virtual call resolves to io.netty.channel.embedded.EmbeddedChannel.1
> .operationComplete.)
> 206                        futureListener.operationComplete(null);
> 207                     }
>
> It fails the substitution principle (most of ChannelFutureListener
> implementations that exist in Netty would fail with NPE there), but what I
> found by searching in Intellij is that only null or a barebone
> ChannelFutureListener that does work are ever passed there. To fix it, one
> would have to implement proper instance of ChannelFuture to pass as
> argument instead of that null, which would be 100 lines of essentially
> fluff, and there would still be problem because the future needs to be able
> to return non-null instance of Channel, of which there isn't any. I simply
> cannot think of reasonable fix. And it seems to be working all right. Maybe
> change the comment ;)
> --
> Jiří Daněk



-- 
Clebert Suconic
[prev in list] [next in list] [prev in thread] [next in thread]