Am 23.02.21 um 17:14 schrieb Joel Sherrill:
On Tue, Feb 23, 2021 at 9:50 AM Christian MAUDERER
<christian.maude...@embedded-brains.de
<mailto:christian.maude...@embedded-brains.de>> wrote:
Hello Joel,
Am 23.02.21 um 16:23 schrieb Joel Sherrill:
> Hi
>
> Ryan has wandered into the land of third party code and Coverity
issues.
> It is not a very welcoming land and we need to decide what we
want to do
> as a project. I put one
>
> In these cases, we have a few patterns to fall back on. There are
> basically a few choices here:
>
> + ignore third party code in Coverity. I'm not a fan since we
still have
> code with issues.
Depends a bit on the third party code and the types of Coverity errors.
But in general you are right: It's not a good solution.
There isn't a good one.
True.
>
> + Do the minimum which is often just adding (void) in front of
the
> call not checking a return code..Still wrap in #ifdef __rtems__
as below
> with a more robust solution.
>
> + Add #ifdef __rtems__ so the original code and rtems changes are
> side by side. This is more like what we do with libbsd. But I would
> still tend to use the correct fix and not just slap a (void) in
front.
> If we are going to modify the file, we might as well fix it.
>
> #ifdef __rtems__
> rc = XXX
> _Assert_Unused_variable_equals'
> #else
> XXX
> #endif
>
> Any other options? Opinions.
Best option would be to fix it upstream (regardless whether it's a real
bug or a false positive) and update our copy of the code.
If that is not an option (patches not accepted; running out of time;
some other reason) I think it needs a case by case analysis:
Is it really a potential problem or is it a false positive of Coverity.
1) If it is a false positive we have two conflicting interests:
a) Have as few changes as possible so that we can easily upgrade third
party code to newer versions.
b) Have as few Coverity warnings as possible.
If possible I would rather disable specific warnings for specific third
party files (or on a function level - is that possible?) rather then
adding complexity to upgrades. Otherwise we might tend to rather not do
an upgrade because it's too much work to re-integrate all the patches.
This isn't possible but we can flag a specific case as "don't care" and
move on.
Hm. Would have been nice if it would have worked.
I'm prone to think the one in the patch below is OK to do this with
since even if the calls fails, the next call to fdt_next_tag() will also
fail and get caught.
That's what I meant with "false positive".
2) If it is not a false positive it should be definitively fixed (and
not only masked with a (void)). In that case I think the #if(n)def
__rtems__ like in libbsd is a good solution to make updates as
simple as
possible.
I suppose this is the solution and trying to at least file an issue
upstream.
I'm hesitant to hold up fixing it for us while waiting on a fix upstream
and then
an update on our side is twofold. First, the loop is quite time
consuming even
when it works.
Yes, I know. Upstream loops can be quite time consuming. I'm waiting
since about half a year that a FreeBSD patch for the i.MX6ULL SDHCI gets
reviewed (after someone here suggested it should go upstream). It's not
a chip that is used in FreeBSD so it's not a high priority.
Worse, my history with upstreams and Coverity issues
isn't great.
It is possible in the end that we get stuck with the issue after all. I
had this happen
with gzip where the end result was an email that said they had decided
it was a
false positive. But every project including gzip gets the same issue
flagged. :(
Only possibility: Find another bug at the same location and fix the
Coverity issue as a side effect ;-)
We'll try to be good open source citizens, file issues upstream, and do
what we
think is right locally.
Agreed.
Again: I think such fixes are case by case decisions. We should fix as
much as necessary and possible. But we have to make sure that we don't
make our updates more complex with unnecessary fixes.
Best regards
Christian
--joel
Best regards
Christian
>
> --joel
>
> ============================
> CID 1047324: Unchecked return value in fdt_add_subnode_namelen().
>
> Closes #4256
> ---
> cpukit/dtc/libfdt/fdt_rw.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/cpukit/dtc/libfdt/fdt_rw.c b/cpukit/dtc/libfdt/fdt_rw.c
> index 1385425..ceeeb44 100644
> --- a/cpukit/dtc/libfdt/fdt_rw.c
> +++ b/cpukit/dtc/libfdt/fdt_rw.c
> @@ -348,7 +348,7 @@ int fdt_add_subnode_namelen(void *fdt, int
parentoffset,
> return offset;
> /* Try to place the new node after the parent's properties */
> - fdt_next_tag(fdt, parentoffset, &nextoffset); /* skip the
BEGIN_NODE */
> + (void)fdt_next_tag(fdt, parentoffset, &nextoffset); /* skip the
> BEGIN_NODE */
> do {
> offset = nextoffset;
> tag = fdt_next_tag(fdt, offset, &nextoffset);
> --
> 1.8.3.1
>
> _______________________________________________
> devel mailing list
> devel@rtems.org <mailto:devel@rtems.org>
> http://lists.rtems.org/mailman/listinfo/devel
<http://lists.rtems.org/mailman/listinfo/devel>
>
--
--------------------------------------------
embedded brains GmbH
Herr Christian MAUDERER
Dornierstr. 4
82178 Puchheim
Germany
email: christian.maude...@embedded-brains.de
<mailto:christian.maude...@embedded-brains.de>
phone: +49-89-18 94 741 - 18
fax: +49-89-18 94 741 - 08
Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/
<https://embedded-brains.de/datenschutzerklaerung/>
--
--------------------------------------------
embedded brains GmbH
Herr Christian MAUDERER
Dornierstr. 4
82178 Puchheim
Germany
email: christian.maude...@embedded-brains.de
phone: +49-89-18 94 741 - 18
fax: +49-89-18 94 741 - 08
Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
