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

[Mike Frysinger]

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: https://scan.coverity.com/projects/1719 >.  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.

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);
}

> -  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. */
        }
-mike

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