[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r35164 - trunk/subversion/libsvn_subr

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Mon, 12 Jan 2009 10:14:52 +0000

Senthil Kumaran S wrote:
> Greg Stein wrote:
> > That is the wrong fix. You should be adding "const" to
> > svn_checksum_size... NOT dropping it from this function. This function
> > doesn't change the checksum, nor does the size computation, so they
> > *should* be const.
>
> Reverted r35164 in r35165. I ve attached the patch based on your comments
> above. If this is good, then I can go ahead and commit it.
>
> [[[
> Follow up r35156.
>
> Suppress warning of "argument 1 of 'svn_checksum_size' discards qualifiers
> from pointer target type".

This fix is good. Thank you.

I have just a meta-comment about describing changes like this one:

"Suppressing" is like "hiding" or "turning off". Remember that the
reason we are adding "const" is to fix the problem that the compiler
pointed out, not just to stop the compiler issuing the message. It would
be the right thing to do even if the compiler had not issued the
message, or if the compiler was still issuing the message (for whatever
reason) after the change.

So, when we fix a problem and that stops the compiler issuing a warning,
we should say "We've fixed this problem (which the compiler pointed
out)" rather than "We've stopped the compiler issuing a warning."

In this case, I might write:
[[[
Follow up r35156. Add "const" to an input pointer parameter. (Found by a
compiler warning.)

...
]]]

There is no need to edit this log message: there are already lots like
it; and this is not directed at you in particular: I'm just using this
log message as an example.

- Julian

> * subversion/libsvn_subr/checksum.c
> (svn_checksum_size): Accept 'const svn_checksum_t'.
>
> * subversion/include/svn_checksum.h
> (svn_checksum_size): Reflect above change.
>
> Suggested by: gstein
> ]]]

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1018731
Received on 2009-01-12 11:15:18 CET

This is an archived mail posted to the Subversion Dev mailing list.