Re: svn commit: r1464228 - in /subversion/trunk/subversion/libsvn_ra_serf: sb_bucket.c util.c

Branko Čibej wrote on Fri, Apr 05, 2013 at 08:14:36 +0200:
> On 04.04.2013 23:17, Daniel Shahaf wrote:
> > FWIW, lgo's argument makes sense to me. I see no reason to remove
> > (void) casts that make our code more readable because some other
> > codebases use them to silence lint.
>
> My point is that they do not make the code more readable. Moreover: when
> someone does turn on the extra compiler options to find places where
> return values are ignored, the (void)-cast calls will not be flagged.

That's not true under gcc 4.2 with warn_unused_result:

    static int f(void) __attribute__ ((warn_unused_result));
    static int f(void) { return 42; }
    
    int main(void)
    {
      (void) f();
      return 0;
    }

    % gcc main.c
    main.c: In function 'main':
    main.c:11: warning: ignoring return value of 'f', declared with attribute warn_unused_result

> Who's to guarantee that the original author who decided to ignore the
> return value and dropped the (void) tu^Wbreadcrumb was right?

The bug here is ignoring the return value. Whether the cast is present
or not is of secondary importance.

> What if the semantics of the called function, or the calling code,
> changes in such a way that ignoring the return value is no longer
> valid?

Whoever made the incompatible API change to the callee should have
reviewed all callsites.

A change to the calling code that failed to stop ignoring the return
value is called "a change that introduced a bug". (and, again, the bug
wouldn't become less of a bug if the cast-to-void syntax wasn't used)

> And to respond to Lieven: yes, I (unprintable) well do expect an
> explanation for dropping the return value to the floor in a comment, if
> it's not for a trivially obvious reason. The (void) cast only says "I'm
> ignoring the return value," which is hardly useful information since
> anyone can see that even without the cast. Which does not imply that
> each and every such statement in the code needs its own comment.

Agreed that a comment with the reason should be in place when the reason
isn't obvious.
Received on 2013-04-05 10:20:40 CEST