coverity | Utilities for Coverity | REST library

This is a false positive. On a little-endian platform, htonl does an endian swap, extracting the bytes of the argument and putting them back together in the opposite order using the bitwise OR operator. Coverity correctly realizes that one of those bytes will always be zero because the original argument in this case is always either 0 or 1. But it is wrong to conclude that fact is unintentional.

I suggest reporting this back to Coverity's support team so they can get it fixed.

This sort of error indicates a problem in the Coverity tool. The Coverity compiler, cov-emit, is failing to compile source code that the native compiler (in this case GCC) accepts. Thus, it has some sort of unintended incompatibility.

In this case I think the main issue is the Coverity release is older than the compiler, and hence lacked support for it. GCC 8.3 was released in February 2019, while Coverity 8.7 was released in January 2017. For each new supported compiler release, the Coverity team may need to make adjustments specific to that compiler and its bundled header files. The Coverity documentation lists exactly what compiler versions are supported.

So, that suggests two possible solutions:

1. Use a more recent Coverity release.
2. Use an older compiler, one that was out and officially supported when Coverity 8.7 was released.

Association disclaimer: I used to work for Coverity/Synopsys.

There was a subtle change in contract between Java 7 and 8 regarding AutoCloseable, see the Javadoc:

Java 7 version

A resource that must be closed when it is no longer needed.

Note the word "must".

Java 8 version

An object that may hold resources (such as file or socket handles) until it is closed. The close() method of an AutoCloseable object is called automatically when exiting a try-with-resources block for which the object has been declared in the resource specification header. This construction ensures prompt release, avoiding resource exhaustion exceptions and errors that may otherwise occur.

API Note:

It is possible, and in fact common, for a base class to implement AutoCloseable even though not all of its subclasses or instances will hold releasable resources. For code that must operate in complete generality, or when it is known that the AutoCloseable instance requires resource release, it is recommended to use try-with-resources constructions. However, when using facilities such as Stream that support both I/O-based and non-I/O-based forms, try-with-resources blocks are in general unnecessary when using non-I/O-based forms.

This was done (probably) to allow for Stream to extend AutoCloseable for convenience of using streams with try-with-resources, despite the fact that almost all streams are not resourceful.

Unfortunately, this makes most static analysis tools useless when it comes to auto closeable detection. They might have hard-coded an exception for streams, but not for DSLContext.

You can safely ignore these errors when using jOOQ's DSLContext.

This has been a frequent issue for new jOOQ users, and could be considered an API design flaw. jOOQ 3.14 will remove the AutoCloseable type from the DSLContext super types and provide a dedicated CloseableDSLContext instead, which is returned only from relevant methods: https://github.com/jOOQ/jOOQ/issues/10512

The long type has been 64 bits since the first version of java came out 25+ years ago. Same for Date: it has always used a 64 bit value to count the milliseconds since the Unix epoch Jan 1 1970. The target architecture of the JVM (32/64 bit) plays no role here.

Your new code quality tool is warning that combining int and long types in bit arithmetic may have unexpected outcomes. I'm guessing it would prefer you wrote the code as:

To get a resource from the system classloader, use getSystemResource(String name).

So change the code as follows:

It is easy to show what the explanation means...

Change the loop to have: (33000 assumes a large array length, wider than short)

Deciding where to put certain checks in your build pipeline is a matter of what you want to get out of those checks.

A build pipeline should give you fast feedback first and foremost. You want to know as quickly as possible if there's anything significant that should stop your build from going out to production. That's why you tend to move checks that run fast to the earlier stages of your pipeline. This way you quickly check whether it's worth it to move on to the slower, more cumbersome steps of your pipeline.

If your static code analysis detects issues, do you want to fail the build? If so, this might be an indicator to put this step early into your pipeline.

How long does your static code analysis take to analyse your codebase? If it's a matter of a few seconds, you can put it into an early stage into your pipeline without thinking too much about it. If it takes significant time to build (maybe tens of seconds or even minutes) this is an indicator that you should move this to a later stage so that other, faster checks can run first.

You can but don't have to put static code analysis into one of your existing (test, build and deploy) stages but there's no one stopping you from creating a dedicated stage in your pipeline for that (verification maybe?).

There's no reason to be dogmatic about this. It's valuable to experiment and see what works for you. Putting emphasis on fast feedback is a good rule of thumb to come up with a build pipeline that doesn't require you to watch the build for 20 minutes only to see that you made an indentation error on line 24.

As one of the two people who originally designed the INFINITE_LOOP checker back in 2010 or so when I worked for Coverity (I do not anymore), I can say a bit about why this might not be reported, although I can't go into great detail because this pertains to Coverity (now Synopsys) proprietary intellectual property.

First, one must recognize that Coverity does not report all instances of any given defect type. That's related to the undecidability of the halting problem; it is mathematically impossible for a fully automatic static analysis to be perfectly accurate. Furthermore, most Coverity checkers are designed to report no more than about 20% false positives, and in order to do that, it requires that the code contain fairly strong evidence of a problem before it is willing to report.

The function div does not contain sufficiently strong evidence of a problem because it is plausible that divisor is never supposed to be passed in as zero. If instead it contained something like if (divisor==0) {...}, that would be strong evidence that a zero argument is supposed to be tolerated. That's not the only sort of evidence the tool recognizes, of course. If you added code like that, containing clear evidence of an internal contradiction in the logic, and the checker still did not report, then I would suggest reporting the example to the Coverity support team.

Now, in addition to div, your example includes a call site:

The output shown in your question is only one portion of the complete finding. It shows why service_id is considered to be "tainted" (i.e., under potential control of an attacker), but does not show what happens to the tainted data, and hence we can't know what the code is trying to do or how to fix it.

In the Coverity GUI there is an "events panel" in the lower-right corner that can be used to navigate to the rest of the finding. By clicking on the events in that panel you should be able to see what happens to service_id, and there is sometimes a recommendation from the tool about how to fix it.

Disclosure: I used to work for Coverity/Synopsys.