Re: [xslt] Work report/code review part 1

[Daniel Veillard <veillard redhat com>]

On Thu, Oct 11, 2001 at 07:08:24AM +1000, Keith Isdale wrote:
> This is the first part in series of xslt@gnome posts that explains what
> I've been doing.  Please post reguarding this message to the xslt@gnome mail 
> list.
> 
> This part presents a summary of requested changes to libxml in
> the order that they occur in libxml2-2.4.3_ki.diff. And has been created so
> the the changes I recommend can be reviewed. Most of the changes are to
> allow the reuse of code within debugXML.c. The attached file
> libxml2-2.4.3_ki.diff is much smaller than version on sourceforge and as I
> have removed all "noise" diff text about rebuildable files.

  Excellent, the patch looks clean. With the exception of purely
aesthetic tweaks needed (for example the structured headers for functions
are kept only in the C files, I don't keep them in the include files too,
it's a matter of taste, it allows to browse headers fasters when looking
up for something and detailed description is kept either in the C code
or extracted to build the documentation). For code formatting I use the
fllowing settings for Gnu indent:

indent -bad -bap -bbb -bli4 -br -ce -brs -cs -i4 -l75 -lc75 -nut -sbi4 -psl -saf -sai -saw -sbi4 -ss -sc -cdw -cli4 -npcs -nbc

> File: debugXML.c
> Reason for change : allow public access to debugXML.c functions
> Changes: make public functions named xmlLsCountNode, xmlLsOneNode
> 

  thanks for providing those details it allows me to work through patches
really efficiently, perfect !

  xmlGetLineNo() can be improved in case of non element node by
seeking the closest element ancestor and returning it's line number.
I already did something similar in xsltPrintErrorContext() and extended
your code in a similar way.
  xmlBoolToText() doesn't really make sense into libxml but I kept it
I changed it to 
   const char * xmlBoolToText(int bool);
  since it returns an immutable string
  Similary 
   void xmlShellPrintXPathError(int errorType, const char *arg);
  the string passed shall not be modified.

I compile locally with the following set of gcc flags and it catches those
really well:

 -g -O -pedantic -W -Wunused -Wimplicit -Wreturn-type -Wswitch -Wcomment
 -Wtrigraphs -Wformat -Wchar-subscripts -Wuninitialized -Wparentheses
 -Wshadow -Wpointer-arith -Wcast-align -Wwrite-strings -Waggregate-return
 -Wstrict-prototypes -Wmissing-prototypes -Wnested-externs -Winline

   thanks a lot, I applied the patch and commited !

Daniel

-- 
Daniel Veillard      | Red Hat Network http://redhat.com/products/network/
veillard@redhat.com  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/