apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Stein <gst...@lyra.org>
Subject Re: [contrib] apr_xml_parse_file
Date Tue, 07 Aug 2001 23:11:24 GMT
On Tue, Aug 07, 2001 at 04:42:35PM -0400, Cliff Woolley wrote:
> On 7 Aug 2001, Ian Holsman wrote:
> 
> > @@ -479,7 +480,38 @@
> >      return errbuf;
> >  }
> >
> > +APU_DECLARE(apr_status_t) apr_xml_parse_file(apr_pool_t *p,
> > apr_xml_parser**parser,apr_xml_doc **ppdoc, apr_file_t *xmlfd,int
> > buffer_length)
> 
> The patch was hosed because long lines wrapped... I've taken care of this
> on my end, though.

The above is also symptomatic of numerous problems throughout the patch in
terms of spacing and stylistic problems. Cliff: please either reject the
patch until Ian can fix them, or take the time yourself before checking in.

[ I see in a later post, you remarked on the style... ]

The buffer length should be an apr_size_t.

The parser should not be returned. There is no need: it is entirely an
internal concept. Note that it is created, then marked as "done". The caller
can't do anything with it at that point.

Is the point to return an error message or something?

> > Index: Makefile.in
> > ===================================================================
> > RCS file: /home/cvspublic/apr-util/test/Makefile.in,v
> > retrieving revision 1.12
> > diff -u -r1.12 Makefile.in
> > --- Makefile.in	2001/08/01 20:32:59	1.12
> > +++ Makefile.in	2001/08/07 19:54:29
> > @@ -1,7 +1,7 @@
> >  top_builddir=@top_builddir@
> >  INCLUDES=-I$(top_builddir)/include -I$(top_builddir)/include/private
> > -I@APR_SOURCE_DIR@/include
> >  TARGETS =
> > -PROGRAMS = testdbm testdate testmd4
> > +PROGRAMS = testdbm testdate testmd4 testxml
> 
> Did I miss something?  Where is this testxml file?

It was attached.

Ian: when posting a patch, please attach the files as text/plain (testxml.c
was text/x-c). Also, you need to prevent the wrapping. If you can't, then
attach the file.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Mime
View raw message