[systemd-devel] [PATCH 0/5] Coverity fixes

[Tom Gundersen]

On Wed, Sep 10, 2014 at 12:51 PM, Philippe De Swert
<philippedeswert at gmail.com> wrote:
> Hi,
>
> On 10/09/14 13:30, Mantas Mikulėnas wrote:
>> On Wed, Sep 10, 2014 at 1:20 PM, Philippe De Swert
>>> On a side note I noticed this mail :
>>> http://permalink.gmane.org/gmane.comp.sysutils.systemd.devel/6248
>>>
>>> Talking about some freeing macros. I noticed there seem to be some
>>> here and there. So it would be good to know what they are. As a lot
>>> of the errors seem to be leaking fd's and the like.
>>
>> gcc has a "cleanup" attribute for variables, which makes the compiler
>> automatically add a call to the specified function as soon as the
>> variable goes out of scope.
>>
>> systemd uses this liberally, and has convenience macros like
>> _cleanup_close_ to automatically close fd's when returning from a
>> function, so they don't actually leak when declared like that.
>>
>> The message you linked to actually describes exactly this...
>
> Ok so it has been taken into use and can actually somehow be considered part of
> the coding style? The mail was still speculating about it and I could not
> quickly see a confirmation of this.
>
> So to fix an fd leak this would be the preferred way right, instead of doing
> a close() call?
>
> From cfd7f340bc7cdb36498993e665058795422c9152 Mon Sep 17 00:00:00 2001
> From: Philippe De Swert <philippedeswert at gmail.com>
> Date: Wed, 10 Sep 2014 13:45:33 +0300
> Subject: [PATCH] [fd leak] Stop leaking an fd in sd_journal_sendv
>
> Found with Coverity. Fixes: CID#996435
>
> Signed-off-by: Philippe De Swert <philippedeswert at gmail.com>

No need for s-o-b in systemd.

> ---
>  src/journal/journal-send.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/journal/journal-send.c b/src/journal/journal-send.c
> index bb1ef66..e9de638 100644
> --- a/src/journal/journal-send.c
> +++ b/src/journal/journal-send.c
> @@ -198,7 +198,7 @@ finish:
>
>  _public_ int sd_journal_sendv(const struct iovec *iov, int n) {
>          PROTECT_ERRNO;
> -        int fd;
> +        _cleanup_close_ int fd;

Yes, but assign it "fd = -1" in case you go out of scope before
assigning anything.

>          _cleanup_close_ int buffer_fd = -1;
>          struct iovec *w;
>          uint64_t *l;
> --
> 1.8.3.2
>
>
>
>
>
> _______________________________________________
> systemd-devel mailing list
> systemd-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel