On Wed, May 07, 2014 at 10:58:03PM +0200, Ingo Schwarze wrote:
> Hi Fritjof,
>
> Fritjof Bornebusch wrote on Wed, May 07, 2014 at 10:32:05PM +0200:
>
> > there are a few void casts in rcs. But I have a question about that.
> > Are these casts really necessary?
>
> No, they are not necessary.
>
> > I've read that the compiler warns, because of unused variables.
> > But no compiler warnings about that on amd64.
>
> Usually, you don't choose your style to please static analysis tools -
> there are too many of them, of too varying quality and signal-to-noise
> ratio, to please them all.
>
> Rather, it makes sense to tune static analysis tools, including
> compilers, to warn about dangerous style, but to not warn about style
> that is merely a matter of taste.
>
> That said, for functions that usually require checking of return
> values, the void cast can be used to indicate "this call was audited,
> and ignoring the return value is considered safe, either because
> this specific call cannot actually fail, or because failure does
> not need explicit handling at this point". Typical functions where
> this makes sense are snprintf(3), strlcpy(3), strlcat(3). So i
> clearly wouldn't remove the cast from strftime() unless you have
> reason to assume that no proper audit was done, or this is broken
> and needs fixing. In this case, the buffer is large enough, so
> the cast should stay.
>
> On the other hand, on functions that hardly ever require checking
> the return code, casting to void is pointless, so i tend to
> remove such casts when editing nearby code. A typical example
> is "(void)close(fd);" which is rarely more helpful than
> just "close(fd);".
>
> In this case, the rcsnum_tostr() return value can't be used for error
> checking in the first place, that function calls errx(3) on failure,
> so when editing nearby code, i'd probably remove that cast. But it
> doesn't warrant a commit on it's own imho.
>
> Yours,
> Ingo
>
I see, thank you for the explanations.
It was just for me to make it clear when (void) is used.
fritjof
>
> > Index: rlog.c
> > ===================================================================
> > RCS file: /cvs/src/usr.bin/rcs/rlog.c,v
> > retrieving revision 1.67
> > diff -u -p -r1.67 rlog.c
> > --- rlog.c 7 Jan 2014 14:08:16 -0000 1.67
> > +++ rlog.c 7 May 2014 20:24:37 -0000
> > @@ -517,7 +517,7 @@ rlog_rev_print(struct rcs_delta *rdp)
> > fmt = "%Y/%m/%d %H:%M:%S";
> > }
> >
> > - (void)strftime(timeb, sizeof(timeb), fmt, &t);
> > + strftime(timeb, sizeof(timeb), fmt, &t);
> >
> > printf("\ndate: %s; author: %s; state: %s;", timeb,
> > rdp->rd_author,
> > rdp->rd_state);
> > @@ -556,7 +556,7 @@ rlog_rev_print(struct rcs_delta *rdp)
> > TAILQ_FOREACH(rb, &(rdp->rd_branches), rb_list) {
> > RCSNUM *branch;
> > branch = rcsnum_revtobr(rb->rb_num);
> > - (void)rcsnum_tostr(branch, numb, sizeof(numb));
> > + rcsnum_tostr(branch, numb, sizeof(numb));
> > printf(" %s;", numb);
> > rcsnum_free(branch);
> > }
