Re: [xslt] [BUG] Win32, xsltproc, libxml, libxslt, <xsl:include/>



> > I have one question: Is xsltParseStylesheetProcess() planned to be able
to
> > return a valid stylesheet pointer different to NULL and thus
invalidating
> > the one it received as a parameter? The current code does not do this,
but
> > the comments and the type of the return value suggest that this at least
was
> > the intention.
>
>   yes sounds fine. This function is mostly internal so doing this should
> not break code,

Eh? :-) My point was missed, I'll try again:

I wondered if you have planned reallocating the stylesheet within this
function anytime in the future. I mean, the function returns a pointer to a
stylesheet and the comment at the top states that it could return a
stylesheet different to the one passed to it. The code, however, does not
produce a new stylesheet anywhere, it just modifies the original. Which is
now to follow? The code, or the comment?

If xsltParseStylesheetProcess() is allowed to reallocate the stylesheet and
invalidate its parameter, then all functions which call it (except the one
which allocates the stylesheet) must also return the pointer to a
stylesheet, not an int. Not passing the return value of this function all
the way up would be fatal.

If xsltParseStylesheetProcess() is not expected to reallocate the
stylesheet, then functions which call it can report to their callers by
returning an int. I would know if in this case freeing the stylesheet memory
within xsltParseStylesheetProcess() is really necessary? The function is
being called recursively and does not free the whole stylesheet
(stylesheet->doc is not freed). Keeping track of what must be deallocated in
which recursion level will result in a mess sooner or later. Why not let the
caller free the memory it allocated itself?

Well, if the stylesheet were an array, then things would be different. It
being a structure, I am puzzled why would someone move it to another address
at any processing level.

So, my proposal is:
 * Lets hold that xsltParseStylesheetProcess() cannot make a new stylesheet,
it can only modify the contents of the one which was passed to it as a
parameter. If it succeeds, it returns the pointer to the same stylesheet it
received after modifying its contents. If it fails, it returns NULL. The
function does not free the stylesheet in any case. If the function allocates
memory and stores the addresses in various members of the stylesheet, that
memory remains there. It will be freed by the next call to
xsltFreeStylesheet(), which is the responsibility of the party who called
xsltNewStylesheet().
 * We modify the top-level call which allocated the stylesheet (that is
xsltParseStylesheetDoc()) to free that stylesheet incase there was an error.
 * We modify all functions which call xsltParseStylesheetProcess() and
return void to return an int indicating success or failure.
 * We modify all calls to any of these to actually check this return value.

What do you think?
Igor





[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]