On 04/01/07, Geoffrey Winn <[EMAIL PROTECTED]> wrote:
I have a couple of comments. 1. In the patch XSDHelperImpl.990 you _must_ add comments to explain what the code is doing and why. 2. Regarding xmlCanonicPath, performance is not an issue. This section of code won't be called often enough to matter, whereas reliability and clarity (for people reading it in future) are essential. So whether we use xmlCanonicPath or not should be based on which one is easier to understand in future. Caroline, I've looked on the libxml2 website and the description of xmlCanonicPath that I found is pretty thin. Do you have a pointer to a decent explanation of what it does?
Well, this is open source :-) Here's the prolog: */** * xmlCanonicPath: * @path: the resource locator in a filesystem notation * * Constructs a canonic path from the specified path. * * Returns a new canonic path, or a duplicate of the path parameter if the * construction fails. The caller is responsible for freeing the memory occupied * by the returned string. If there is insufficient memory available, or the * argument is NULL, the function returns NULL. */* and you can view the source at: http://cvs.gnome.org/viewcvs/libxml2/uri.c?view=markup (search for xmlCanonicPath) You'll see the the logic is very similar to Yang's, but IMHO it serves the project better to reuse a function from a supported open-source library that the project already depends on than to reinvent it. Personally I wouldn't see the extra memory allocation as an issue. -- Caroline
