Patch from Scott McKellar:

1. I added the const qualifier to a number of function parameters.

2. I moved the prototype for _oilsEventParseEvents() from the header
into the implementation file, and made the function static.  No other
source file calls it, nor should it.

3. I removed an extra leading underscore from each of _oilsEventEvents
and _oilsEventDescriptions, and made them static.

3. I removed an unhelpful cast from a call to safe_malloc().

4. I made sure to initialize every member of a new oilsEvent.

5. In several spots where we update pointer members of an oilsEvent,
I preceded the update with a free, in order to avoid potential
memory leaks.

6. I replaced calls to oilsEventSetPermission() and
oilsEventSetPayload() with the equivalent inline code.

7. In oilsEventFree(), the original code would free the json member
or the payload member but not both.  We now free both.  We also
free the event member, which we didn't do before.

git-svn-id: svn://svn.open-ils.org/ILS/trunk@8548 dcc99617-32d9-48b4-a31d-7c20da2025e4

diff --git a/Open-ILS/include/openils/oils_event.h b/Open-ILS/include/openils/oils_event.h
index 64af307..e057386 100644 (file)
--- a/Open-ILS/include/openils/oils_event.h
+++ b/Open-ILS/include/openils/oils_event.h
@@ -20,37 +20,36 @@ typedef struct _oilsEventStruct oilsEvent;
 
 
 /** Creates a new event.  User is responsible for freeing event with oilsEventFree */
-oilsEvent* oilsNewEvent( char* file, int line, char* event );
+oilsEvent* oilsNewEvent( const char* file, int line, const char* event );
 
 /** Creates a new event with payload.  
  * User is responsible for freeing event with oilsEventFree */
-oilsEvent* oilsNewEvent2( char* file, int line, char* event, jsonObject* payload );
+oilsEvent* oilsNewEvent2( const char* file, int line, const char* event,
+               const jsonObject* payload );
 
 /** Creates a new event with permission and permission location.  
  * User is responsible for freeing event with oilsEventFree */
-oilsEvent* oilsNewEvent3( char* file, int line, char* event, char* perm, int permloc );
+oilsEvent* oilsNewEvent3( const char* file, int line, const char* event,
+               const char* perm, int permloc );
 
 /** Creates a new event with permission, permission location, and payload.  
  * User is responsible for freeing event with oilsEventFree */
-oilsEvent* oilsNewEvent4( char* file, int line, 
-               char* event, char* perm, int permloc, jsonObject* payload );
+oilsEvent* oilsNewEvent4( const char* file, int line, const char* event,
+               const char* perm, int permloc, const jsonObject* payload );
 
 /** Sets the permission info for the event */
-void oilsEventSetPermission( oilsEvent* event, char* perm, int permloc );
+void oilsEventSetPermission( oilsEvent* event, const char* perm, int permloc );
 
 /* Sets the payload for the event 
  * This clones the payload, so the user is responsible
  * for handling the payload object's memory
  * */
-void oilsEventSetPayload( oilsEvent* event, jsonObject* payload );
+void oilsEventSetPayload( oilsEvent* event, const jsonObject* payload );
 
 /** Creates the JSON associated with an event.  The JSON should NOT be
  * freed by the user.  It will be freed by oilsEventFree */
 jsonObject* oilsEventToJSON( oilsEvent* event );
 
-/* Parses the events file */
-void _oilsEventParseEvents();
-
 /* Frees an event object */
 void oilsEventFree( oilsEvent* event );
 

diff --git a/Open-ILS/src/c-apps/oils_event.c b/Open-ILS/src/c-apps/oils_event.c
index 135bd40..bc9b737 100644 (file)
--- a/Open-ILS/src/c-apps/oils_event.c
+++ b/Open-ILS/src/c-apps/oils_event.c
@@ -3,65 +3,82 @@
 #include <libxml/tree.h>
 #include "opensrf/osrf_settings.h"
 
-osrfHash* __oilsEventEvents = NULL;
-osrfHash* __oilsEventDescriptions = NULL;
+static void _oilsEventParseEvents();
 
-oilsEvent* oilsNewEvent( char* file, int line, char* event ) {
+// The following two osrfHashes are created when we
+// create the first osrfEvent, and are never freed.
+
+static osrfHash* _oilsEventEvents = NULL;
+static osrfHash* _oilsEventDescriptions = NULL;
+
+oilsEvent* oilsNewEvent( const char* file, int line, const char* event ) {
        if(!event) return NULL;
        osrfLogInfo(OSRF_LOG_MARK, "Creating new event: %s", event);
-       if(!__oilsEventEvents) _oilsEventParseEvents();
-       oilsEvent* evt =  (oilsEvent*) safe_malloc(sizeof(oilsEvent));
+       if(!_oilsEventEvents) _oilsEventParseEvents();
+       oilsEvent* evt = safe_malloc(sizeof(oilsEvent));
        evt->event = strdup(event);
+       evt->perm = NULL;
        evt->permloc = -1;
+       evt->payload = NULL;
+       evt->json = NULL;
        if(file) evt->file = strdup(file);
+       else evt->file = NULL;
        evt->line = line;
        return evt;
 }
 
-oilsEvent* oilsNewEvent2( char* file, int line, char* event, jsonObject* payload ) {
+oilsEvent* oilsNewEvent2( const char* file, int line, const char* event,
+               const jsonObject* payload ) {
        oilsEvent* evt = oilsNewEvent(file, line, event);
-       oilsEventSetPayload(evt, payload);
+       if(payload) evt->payload = jsonObjectClone(payload);
        return evt;
 }
 
-oilsEvent* oilsNewEvent3( char* file, int line, char* event, char* perm, int permloc ) {
+oilsEvent* oilsNewEvent3( const char* file, int line, const char* event,
+               const char* perm, int permloc ) {
        oilsEvent* evt = oilsNewEvent(file, line, event);
-       oilsEventSetPermission( evt, perm, permloc );
+       if(perm) {
+               evt->perm = strdup(perm);
+               evt->permloc = permloc;
+       }
        return evt;
 }
 
-oilsEvent* oilsNewEvent4( char* file, int line, 
-                       char* event, char* perm, int permloc, jsonObject* payload ) {
+oilsEvent* oilsNewEvent4( const char* file, int line, const char* event,
+               const char* perm, int permloc, const jsonObject* payload ) {
        oilsEvent* evt = oilsNewEvent3( file, line, event, perm, permloc );
-       if(evt) oilsEventSetPayload( evt, payload );
+       if(payload) evt->payload = jsonObjectClone(payload);
        return evt;
 }
 
-void oilsEventSetPermission( oilsEvent* event, char* perm, int permloc ) {
+void oilsEventSetPermission( oilsEvent* event, const char* perm, int permloc ) {
        if(!(event && perm)) return;
+       if(event->perm) free(event->perm);
        event->perm = strdup(perm);
        event->permloc = permloc;
 }
 
-void oilsEventSetPayload( oilsEvent* event, jsonObject* payload ) {
+void oilsEventSetPayload( oilsEvent* event, const jsonObject* payload ) {
        if(!(event && payload)) return;
+       if(event->payload) jsonObjectFree(event->payload);
        event->payload = jsonObjectClone(payload);
 }
 
 
 void oilsEventFree( oilsEvent* event ) {
        if(!event) return;
+       free(event->event);
        free(event->perm);
        free(event->file);
        if(event->json) jsonObjectFree(event->json);
-       else jsonObjectFree(event->payload);
+       if(event->payload) jsonObjectFree(event->payload);
        free(event);
 }
 
 
 jsonObject* oilsEventToJSON( oilsEvent* event ) {
        if(!event) return NULL;
-       char* code = osrfHashGet( __oilsEventEvents, event->event );
+       char* code = osrfHashGet( _oilsEventEvents, event->event );
 
        if(!code) {
                osrfLogError(OSRF_LOG_MARK,  "No such event name: %s", event->event );
@@ -71,7 +88,7 @@ jsonObject* oilsEventToJSON( oilsEvent* event ) {
 
        char* lang = "en-US"; /* assume this for now */
        char* desc = NULL;
-       osrfHash* h = osrfHashGet(__oilsEventDescriptions, lang);
+       osrfHash* h = osrfHashGet(_oilsEventDescriptions, lang);
        if(h) {
                osrfLogDebug(OSRF_LOG_MARK, "Loaded event lang hash for %s",lang);
                desc = osrfHashGet(h, code);
@@ -93,12 +110,14 @@ jsonObject* oilsEventToJSON( oilsEvent* event ) {
        if(event->perm) jsonObjectSetKey( json, "ilsperm", jsonNewObject(event->perm) );
        if(event->permloc != -1) jsonObjectSetKey( json, "ilspermloc", jsonNewNumberObject(event->permloc) );
        if(event->payload) jsonObjectSetKey( json, "payload", event->payload );
+       
+       if(event->json) jsonObjectFree(event->json);
        event->json = json;
        return json;
 }
 
-
-void _oilsEventParseEvents() {
+/* Parses the events file */
+static void _oilsEventParseEvents() {
        
        char* xml = osrf_settings_host_value("/ils_events");
 
@@ -110,8 +129,8 @@ void _oilsEventParseEvents() {
        xmlDocPtr doc = xmlParseFile(xml);
        free(xml);
        int success = 0;
-       __oilsEventEvents = osrfNewHash();
-       __oilsEventDescriptions = osrfNewHash();
+       _oilsEventEvents = osrfNewHash();
+       _oilsEventDescriptions = osrfNewHash();
 
        if( doc ) {
                xmlNodePtr root = xmlDocGetRootElement(doc);
@@ -122,7 +141,7 @@ void _oilsEventParseEvents() {
                                        xmlChar* code = xmlGetProp( child, BAD_CAST "code");
                                        xmlChar* textcode = xmlGetProp( child, BAD_CAST "textcode");
                                        if( code && textcode ) {
-                                               osrfHashSet( __oilsEventEvents, code, (char*) textcode );
+                                               osrfHashSet( _oilsEventEvents, code, (char*) textcode );
                                                success = 1;
                                        }
 
@@ -136,10 +155,10 @@ void _oilsEventParseEvents() {
                                                        if(lang) {
                                                                osrfLogDebug(OSRF_LOG_MARK, "Loaded event lang: %s", (char*) lang);
                                                                osrfHash* langHash = osrfHashGet(
-                                                                       __oilsEventDescriptions, (char*) lang);
+                                                                       _oilsEventDescriptions, (char*) lang);
                                                                if(!langHash) {
                                                                        langHash = osrfNewHash();
-                                                                       osrfHashSet(__oilsEventDescriptions, langHash, (char*) lang);
+                                                                       osrfHashSet(_oilsEventDescriptions, langHash, (char*) lang);
                                                                }
                                                                char* content;
                                                                if( desc->children && (content = (char*) desc->children->content) ) {