On Thu, Apr 4, 2013 at 1:28 PM, Philip Martin <philip.mar...@wandisco.com> wrote: > Lieven Govaerts <l...@apache.org> writes: > >> On Thu, Apr 4, 2013 at 12:55 AM, phi...@apache.org <phi...@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
