[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [sysvinit-devel] [patch] Get startpar building with clang

From: Mike Frysinger
Subject: Re: [sysvinit-devel] [patch] Get startpar building with clang
Date: Thu, 17 Apr 2014 15:50:13 -0400
User-agent: KMail/4.12.4 (Linux/3.14.0; KDE/4.12.4; x86_64; ; )

On Thu 10 Apr 2014 09:42:13 Petter Reinholdtsen wrote:
> I've submitted startpar for review with Coverty, and it reported four
> issues on <URL: >.  But the
> project is not yet accepted, so I can not figure out which issues this
> is.  So I had a look at building it with clang, and it found a few
> issues with ignored return values and using a GCC extention.  The
> following patch get rid of all warnings from Clang, but I am unsure if
> this is the correct fix.  Any comments?
> The timerdiff() change make sense to me, but simply continuing to ignore
> exit values do not seem like a safe way forward.  But I did not take the
> time to figure out why these exit values were ignored in the first
> place.  Werner, do you remember?
> Index: startpar.c
> ===================================================================
> --- startpar.c        (revision 178)
> +++ startpar.c        (working copy)
> @@ -78,7 +78,11 @@
>  # define attribute(attr)     __attribute__(attr)
>  #endif
> -#define timerdiff(n,l) (extension({
> (((n).tv_sec-(l).tv_sec)*1000)+(((n).tv_usec-(l).tv_usec)/1000); }))
> +long
> +timerdiff(const struct timeval n,const struct timeval l)
> +{
> +    return (((n).tv_sec-(l).tv_sec)*1000)+(((n).tv_usec-(l).tv_usec)/1000);
> +}

way more paren in there than needed.  they made sense in the old code as it 
was a define, but this isn't.

would be nice to also fix the (lack of) spacing while you're here.

timerdiff(const struct timeval n, const struct timeval l)
        return ((n.tv_sec - l.tv_sec) * 1000) + ((n.tv_usec - l.tv_usec) / 

> -  TEMP_FAILURE_RETRY(waitpid(splashpid, &status, 0));
> +  (void)TEMP_FAILURE_RETRY(waitpid(splashpid, &status, 0));

i don't think that defeats the must_check_return attribute.  you'd need 
something like:
        if (...) {
                /* Ignoring return. */

Attachment: signature.asc
Description: This is a digitally signed message part.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]