Re: [OPEN-ILS-DEV] osrf_json_parser.c: memory leak when handling error

2008-01-27 Thread Bill Erickson

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



[OPEN-ILS-DEV] osrf_json_xml.c: memory leak and apparent bug

2008-01-27 Thread Scott McKellar
At the end of startElementHandler(), there is a branch of code that
runs when the name passed in is boolean.  It constructs a jsonObject
of type JSON_BOOL but doesn't do anything with it.  The pointer goes
out of scope and the memory leaks.

As a result, this branch of logic has no effect at all, except to
leak memory.

I would have expected a call to appendChild(), and probably to
osrfListPush() as well.  However the other branches are not completely
consistent in this regard -- for null we create a jsonObject and
pass it to appendChild(), but we don't call osrfListPush() for it.

Another option is to remove this branch altogether.  The resulting
code would behave exactly the same as it does now, but without 
leaking.

Not sure of how to proceed, I leave this decision to my betters.

Scott McKellar
http://home.swbell.net/mck9/ct/