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.