[systemd-devel] systemd coverity

[Lennart Poettering]

On Thu, 23.08.12 09:25, Lukáš Nykrýn (lnykryn at redhat.com) wrote:

> 
> diff --git a/src/readahead/readahead-analyze.c b/src/readahead/readahead-analyze.c
> index 11b2b2d..9a929c0 100644
> --- a/src/readahead/readahead-analyze.c
> +++ b/src/readahead/readahead-analyze.c
> @@ -144,6 +144,7 @@ int main_analyze(const char *pack_path) {
>          return EXIT_SUCCESS;
>  
>  fail:
> -        fclose(pack);
> +        if(pack)
> +                fclose(pack);
>          return EXIT_FAILURE;
>  }

I am wondering whether we should begin making use of gcc's cleanup
variable attribute to avoid problems with cleaning up dynamicly
allocated objects. That way we wouldn't have to manually clean up things
like this anymore, so we'd have less chance to forget it, to check for
NULL before and other things.

I think we should be conservative with the cleanup stuff and only use it
for very low level objects, not all objects though. i.e. stuff that only
needs "free()" or another glibc call to destruct sounds good, but I
wouldn't use it for stuff that needs a more complex destruction logic.

maybe we should add macros like:

        #define _cleanup_free_ __attribute__((cleanup(freep)))
        #define _cleanup_fclose_ __attribute__((cleanup(fclosep)))

and a few more. (Assuming that freep()/fclosep() are defined as needed).

Then, whenever we allocate something on the heap and need it only within
one function we'd write this:

        _cleanup_free_ char *s = NULL;

instead of:

        char *s = NULL;
        ...
        free(s);

What do you think?

Lennart

-- 
Lennart Poettering - Red Hat, Inc.