Scott McKellar wrote:
In _jsonInsertParserItem(), we receive a pointer to a jsonObject named
newo.  Normally, newo is incorporated into the object that the parser
is building.  Whoever receives the object built by the parser is
responsible for freeing it.

However: if the sub-object currently being processed by the parser is
neither a hash nor an array, the default branch of the switch/case structure merely writes a message to stderr, without incorporating
newo into the growing jsonObject.

My first impulse was to add a line to the default branch, to free newo. Then I noticed that, after the switch/case, we reference newo
again.  So we don't want to free it as I had first thought.

The end result, if this situation ever occurs, is that we leak newo.
We may hang on to it for a while as the current object, but since we don't incorporate it into the object wo're building, we
eventually lose it.

Presumably this situation never arises in practice, or happens so
rarely that the memory leak isn't worth worrying about.  For all I
know it may be impossibe for this situation to arise at all, even for arbitrarily mangled JSON input text. I haven't tried to unravel
all the logic.

However the handling of this situation looks like leftover debugging
code rather than proper error reporting.  At a minimum we should use
the usual logging functions instead of writing directly to stderr.
In addition, I'm not sure that it's appropriate to point p->current
to newo in this situation.  I'd submit a patch, but I don't trust
myself to guess what the right response is.


Yep, that's old debugging code. The switch in _jsonInsertParserItem() should probably be changed to an if/else ( JSON_HASH vs. JSON_ARRAY). There's no other way to reach that chunk of code.

-bill

--
Bill Erickson
| VP, Software Development & Integration
| Equinox Software, Inc. / The Evergreen Experts
| phone: 877-OPEN-ILS (673-6457)
| email: [EMAIL PROTECTED]
| web: http://esilibrary.com

Reply via email to