On 04.04.2013 20:46, Lieven Govaerts wrote:
> On Thu, Apr 4, 2013 at 1:28 PM, Philip Martin
> <philip.martin_at_wandisco.com> wrote:
>> Lieven Govaerts <lgo_at_apache.org> writes:
>>
>>> On Thu, Apr 4, 2013 at 12:55 AM, philip_at_apache.org <philip_at_apache.org> wrote:
>>>> Author: philip
>>>> Date: Wed Apr 3 22:55:37 2013
>>>> New Revision: 1464228
>>>>
>>>> URL: http://svn.apache.org/r1464228
>>>> Log:
>>>> Remove (void) casts of ignored return values from ra_serf.
>>>>
>>>> * subversion/libsvn_ra_serf/util.c
>>>> (svn_ra_serf__conn_closed, svn_ra_serf__process_pending,
>>>> svn_ra_serf__handle_xml_parser, svn_ra_serf__credentials_callback,
>>>> svn_ra_serf__request_create, expat_start, expat_end,
>>>> expat_cdata, expat_response_handler): Remove cast.
>>> What's the benefit of removing these casts? Is the purpose to consider
>>> these as warnings in order to solve them later?
>> We were not consistently casting to (void) when ignoring return values.
>> Branko objected when I added such a cast recently so I decided to remove
>> them all.
>>
>>>> Modified: subversion/trunk/subversion/libsvn_ra_serf/util.c
>>>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/util.c?rev=1464228&r1=1464227&r2=1464228&view=diff
>>>> ==============================================================================
>>>> --- subversion/trunk/subversion/libsvn_ra_serf/util.c (original)
>>>> +++ subversion/trunk/subversion/libsvn_ra_serf/util.c Wed Apr 3 22:55:37 2013
>>>> @@ -497,7 +497,7 @@ svn_ra_serf__conn_closed(serf_connection
>>>>
>>>> err = svn_error_trace(connection_closed(ra_conn, why, pool));
>>>>
>>>> - (void) save_error(ra_conn->session, err);
>>>> + save_error(ra_conn->session, err);
>>>> }
>>>>
>>>>
>>>> @@ -1490,7 +1490,7 @@ svn_ra_serf__process_pending(svn_ra_serf
>>>>
>>>> /* Tell the parser that no more content will be parsed. Ignore the
>>>> return status. We just don't care. */
>>>> - (void) XML_Parse(parser->xmlp, NULL, 0, 1);
>>>> + XML_Parse(parser->xmlp, NULL, 0, 1);
>>> This confuses me a bit. You leave the comment so I guess you agree
>>> with the decision to ignore the return value here, yet you remove the
>>> (void) cast which makes the same thing clear from the code.
>> If the (void) cast helps then we should be using it everywhere we ignore
>> a return value.
> Using (void) says the same thing as /* Ignore the return status. We
> just don't care. */ but in a more compact and precise manner.
> I see no reason to not use it, in those cases where we're absolutely
> sure the return value is not needed of course.
Using (void) was invented to shut up Lint and its derivatives, back in
the day. It's ugly and unnecessary and doesn't tell the compiler
anything it doesn't already know. I see every reason not to use it -- it
clutters up the code in a way that distracts from the overall meaning.
-- Brane
--
Branko Čibej
Director of Subversion | WANdisco | www.wandisco.com
Received on 2013-04-04 20:59:44 CEST