Static code analyzer annotations in driver code?

Static code analyzer annotations in driver code?

[Thomas Hellstrom]

Hi!

What is the general opinion about out-of-tree static analyzer
annotations in drm driver code, for example comments like

/* coverity[missing_lock] */

which typically squelches false positives in constructors or destructors
of refcounted structs that contain members that are elsewhere protected
by locks.

Thanks,

Thomas



_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Static code analyzer annotations in driver code?

[Jani Nikula]

On Wed, 04 Jan 2017, Thomas Hellstrom <thellstrom@vmware.com> wrote:
> What is the general opinion about out-of-tree static analyzer
> annotations in drm driver code, for example comments like
>
> /* coverity[missing_lock] */
>
> which typically squelches false positives in constructors or destructors
> of refcounted structs that contain members that are elsewhere protected
> by locks.

It's not about out-of-tree, it's about proprietary. We already have
annotations for sparse, though they're extra attributes rather than
comments. Anyone can run sparse, or other open source tools. Not so with
properietary tools. When you don't have the crowds maintaining the
annotations, they will bitrot, becoming just stale comments in source.

I know the intention is good, but I'm not convinced.


BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Static code analyzer annotations in driver code?

[Thomas Hellstrom]

Hi,

On 01/05/2017 12:38 PM, Jani Nikula wrote:
> On Wed, 04 Jan 2017, Thomas Hellstrom <thellstrom@vmware.com> wrote:
>> What is the general opinion about out-of-tree static analyzer
>> annotations in drm driver code, for example comments like
>>
>> /* coverity[missing_lock] */
>>
>> which typically squelches false positives in constructors or destructors
>> of refcounted structs that contain members that are elsewhere protected
>> by locks.
> It's not about out-of-tree, it's about proprietary. We already have
> annotations for sparse, though they're extra attributes rather than
> comments. Anyone can run sparse, or other open source tools. Not so with
> properietary tools. When you don't have the crowds maintaining the
> annotations, they will bitrot, becoming just stale comments in source.
>
> I know the intention is good, but I'm not convinced.

Thanks for your comments. In the coverity special case, though, Linux is
added to the
Coverity OSS effort:

https://scan.coverity.com/projects/linux

which makes it possible for anyone to use the tool to pinpoint defects.

Now admittedly there might be a number of tools, some open source, some
not, that may find false
positives in the exact same code statement and annotate for one tool
will still make the other ones
complain but at least then the issue can quickly be flagged as a false
positive..

/Thomas




>
> BR,
> Jani.
>
>


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel