You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ju...@apache.org on 2010/08/20 19:58:10 UTC
svn commit: r987592 - /subversion/trunk/subversion/libsvn_ra_neon/util.c
Author: julianfoad
Date: Fri Aug 20 17:58:10 2010
New Revision: 987592
URL: http://svn.apache.org/viewvc?rev=987592&view=rev
Log:
Add doc strings and remove an unused return value.
* subversion/libsvn_ra_neon/util.c
(start_207_element, end_207_element): Add minimal doc strings.
(multistatus_parser_create): Add a minimal doc string. Don't return a
pointer to the created parser because it's attached to the request and
that's all we need.
(svn_ra_neon__simple_request): Remove the comment and type cast indicating
that we don't use the return value from multistatus_parser_create().
Modified:
subversion/trunk/subversion/libsvn_ra_neon/util.c
Modified: subversion/trunk/subversion/libsvn_ra_neon/util.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_neon/util.c?rev=987592&r1=987591&r2=987592&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_neon/util.c (original)
+++ subversion/trunk/subversion/libsvn_ra_neon/util.c Fri Aug 20 17:58:10 2010
@@ -148,6 +148,7 @@ typedef struct
svn_boolean_t contains_error;
} multistatus_baton_t;
+/* Implements svn_ra_neon__startelm_cb_t. */
static svn_error_t *
start_207_element(int *elem, void *baton, int parent,
const char *nspace, const char *name, const char **atts)
@@ -193,6 +194,7 @@ start_207_element(int *elem, void *baton
return SVN_NO_ERROR;
}
+/* Implements svn_ra_neon__endelm_cb_t . */
static svn_error_t *
end_207_element(void *baton, int state,
const char *nspace, const char *name)
@@ -269,23 +271,23 @@ end_207_element(void *baton, int state,
}
-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,
+ start_207_element,
+ svn_ra_neon__xml_collect_cdata,
+ end_207_element, b);
b->cdata = svn_stringbuf_create("", req->pool);
b->description = svn_stringbuf_create("", req->pool);
b->req = req;
b->propname = svn_stringbuf_create("", req->pool);
b->propstat_description = svn_stringbuf_create("", req->pool);
-
- return multistatus_parser;
}
@@ -1341,9 +1343,7 @@ svn_ra_neon__simple_request(int *code,
SVN_ERR(svn_ra_neon__request_create(&req, ras, method, url, pool));
- /* we don't need the status parser: it's attached to the request
- and detected errors will be returned there... */
- (void) multistatus_parser_create(req);
+ multistatus_parser_create(req);
/* svn_ra_neon__request_dispatch() adds the custom error response
reader. Neon will take care of the Content-Length calculation */
Re: svn commit: r987592 -
/subversion/trunk/subversion/libsvn_ra_neon/util.c
Posted by Julian Foad <ju...@wandisco.com>.
On Fri, 2010-08-20, Greg Stein wrote:
> On Fri, Aug 20, 2010 at 13:58, <ju...@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
Re: svn commit: r987592 - /subversion/trunk/subversion/libsvn_ra_neon/util.c
Posted by Greg Stein <gs...@gmail.com>.
On Fri, Aug 20, 2010 at 13:58, <ju...@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.
>...
Cheers,
-g