Unchecked return value on third party code

[Christian MAUDERER]

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.

> 
>    + 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.

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.

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 at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
> 

-- 
--------------------------------------------
embedded brains GmbH
Herr Christian MAUDERER
Dornierstr. 4
82178 Puchheim
Germany
email: christian.mauderer at 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/