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.
Lieven
Received on 2013-04-04 20:47:06 CEST