On Fri, 2010-08-20, Greg Stein wrote:
> On Fri, Aug 20, 2010 at 13:58, <julianfoad_at_apache.org> wrote:
> >...
> > +++ subversion/trunk/subversion/libsvn_ra_neon/util.c Fri Aug 20 17:58:10 2010
> >...
> > -static ne_xml_parser *
> > +/* Create a status parser attached to the request REQ. Detected errors
> > + will be returned there. */
> > +static void
> > multistatus_parser_create(svn_ra_neon__request_t *req)
> > {
> > multistatus_baton_t *b = apr_pcalloc(req->pool, sizeof(*b));
> > - ne_xml_parser *multistatus_parser =
> > - svn_ra_neon__xml_parser_create(req, ne_accept_207,
> > - start_207_element,
> > - svn_ra_neon__xml_collect_cdata,
> > - end_207_element, b);
> > +
> > + svn_ra_neon__xml_parser_create(req, ne_accept_207,
>
> Woah. You should have a (void) cast in there and (or?) a comment
> noting that the return value is not used. Otherwise, it looks like a
> programming error.
If you know that the called function attaches the parser to REQ, it
doesn't look so odd. Its doc string didn't say so, so I've tweaked it.
I also added a comment here at the point of call for those who still
think it looks odd. (I didn't add a cast to void, as ignoring return
values is a standard idiom in C yet that would make it stand out like a
sore thumb.)
I also wrote a doc string for static function attach_ne_body_reader(),
since it's involved here too.
r988068.
I did notice that all the other callers seem to keep a note of the
returned parser, and use it later in a call to
svn_ra_neon__check_parse_error(). I suppose there's something special
about this particular caller, and it doesn't need to do that. I'm just
mentioning this in case anyone thinks it's wrong; otherwise I'm assuming
it's as intended.
- Julian
Received on 2010-08-23 12:58:40 CEST