[systemd-devel] [PATCH] mount: use libmount to monitor mountinfo & utab

Tom Gundersen teg at jklm.no
Tue Jun 2 09:23:23 PDT 2015


On Tue, Jun 2, 2015 at 1:53 PM, Karel Zak <kzak at redhat.com> wrote:
> On Mon, Jun 01, 2015 at 05:06:56PM +0200, Tom Gundersen wrote:
>> > -                (void) sd_event_source_set_description(m->mount_utab_event_source, "mount-utab-dispatch");
>> > +                sd_event_source_set_description(m->mount_event_source, "mount-monitor-dispatch");
>>
>> This should be cast to (void) unless you check the return.
>
> Frankly, I don't like it. It's old-style programming garbage. For
> compiler it's probably irrelevant construction and for developers
> (code readers) we have better things like warn_unused_result.

The policy in systemd is that as a general rule we check the return
value of functions, only if it truly does not matter do we explicitly
indicate this by casting to void. This is as much for readers of the
code (who can then distinguish between accidentally forgetting to
check the return and intentionally ignoring it) as for static checkers
(Coverity in particular will complain).

> I have removed many of these (void) from util-linux and nobody
> complains.
>
> If you really want to force people to check return code than mark
> function by warn_unused_result and if you still want to ignore the
> result for these functions in some situations then you can use
> something like:

I don't think warn_unused_result is the right thing to do btw, that is
about the provider enforcing checking of result whereas our policy is
as a consumer (both of our internal functions and external libraries).

> # define ignore_result(x) __extension__ ({ \
>         __typeof__(x) __dummy __attribute__((__unused__)) = (x);
>         (void) __dummy; \
> })
>
>
> the result is more readable and obvious:
>
>    ignore_result( sd_event_source_set_description(foo, bar ) );
>
>
> Sometimes we use this macro to keep silent some crazy glibc functions.
>
>
> Anyway, if (void) is really systemd coding style then I'm going to
> update the patch. No problem ;-)

Thanks!

Cheers,

Tom


More information about the systemd-devel mailing list