[open-ils-commits] r16494 - in trunk/Open-ILS: include/openils src/c-apps (scottmk)
svn at svn.open-ils.org
svn at svn.open-ils.org
Tue May 25 12:21:59 EDT 2010
Author: scottmk
Date: 2010-05-25 12:21:56 -0400 (Tue, 25 May 2010)
New Revision: 16494
Modified:
trunk/Open-ILS/include/openils/oils_utils.h
trunk/Open-ILS/src/c-apps/oils_sql.c
trunk/Open-ILS/src/c-apps/oils_utils.c
Log:
Plug some memory leaks, and eliminate some unnecessary
memory allocations.
In oils_utils.[ch]:
-- Create a new function, oilsFMGetStringConst(), similar to
oilsFMGetString() except that it doesn't allocate memory; it returns
a const pointer to a string internal to the specified object.
-- Add some comments, tidy up white space.
In oils_sql.c:
-- Replace oilsFMGetString() with oilsFMGetStringConst in a number
of places; partly to reduce memory churn, and partly to plug some
memory leaks where the function call was nested within a
parameter list.
-- Change org_tree_root() so as to return a const pointer to a
static buffer (which was already in use as a cache) instead of
allocating a copy of the string. This change reduces memory
churn. In addition the allocated string was leaking anyway, and
now that leak is plugged.
M Open-ILS/include/openils/oils_utils.h
M Open-ILS/src/c-apps/oils_sql.c
M Open-ILS/src/c-apps/oils_utils.c
Modified: trunk/Open-ILS/include/openils/oils_utils.h
===================================================================
--- trunk/Open-ILS/include/openils/oils_utils.h 2010-05-25 13:51:26 UTC (rev 16493)
+++ trunk/Open-ILS/include/openils/oils_utils.h 2010-05-25 16:21:56 UTC (rev 16494)
@@ -26,26 +26,10 @@
*/
osrfHash* oilsInitIDL( const char* idl_filename );
-/**
- Returns the string value for field 'field' in the given object.
- This method calls jsonObjectToSimpleString so numbers will be
- returned as strings.
- @param object The object to inspect
- @param field The field whose value is requsted
- @return The string at the given position, if none exists,
- then NULL is returned. The caller must free the returned string
- */
+const char* oilsFMGetStringConst( const jsonObject* object, const char* field );
+
char* oilsFMGetString( const jsonObject* object, const char* field );
-
-/**
- Returns the jsonObject found at the specified field in the
- given object.
- @param object The object to inspect
- @param field The field whose value is requsted
- @return The found object or NULL if none exists. Do NOT free the
- returned object.
- */
const jsonObject* oilsFMGetObject( const jsonObject* object, const char* field );
/**
Modified: trunk/Open-ILS/src/c-apps/oils_sql.c
===================================================================
--- trunk/Open-ILS/src/c-apps/oils_sql.c 2010-05-25 13:51:26 UTC (rev 16493)
+++ trunk/Open-ILS/src/c-apps/oils_sql.c 2010-05-25 16:21:56 UTC (rev 16494)
@@ -114,7 +114,7 @@
static const jsonObject* verifyUserPCRUD( osrfMethodContext* );
static int verifyObjectPCRUD( osrfMethodContext*, const jsonObject* );
-static char* org_tree_root( osrfMethodContext* ctx );
+static const char* org_tree_root( osrfMethodContext* ctx );
static jsonObject* single_hash( const char* key, const char* value );
static int child_initialized = 0; /* boolean */
@@ -1302,7 +1302,7 @@
if( !user )
return 0; // Not logged in? No access.
- int userid = atoi( oilsFMGetString( user, "id" ) );
+ int userid = atoi( oilsFMGetStringConst( user, "id" ) );
// Get a list of permissions from the permacrud entry.
osrfStringArray* permission = osrfHashGet( pcrud, "permission" );
@@ -1325,7 +1325,7 @@
osrfStringArray* context_org_array = osrfNewStringArray( 1 );
int err = 0;
- char* pkey_value = NULL;
+ const char* pkey_value = NULL;
if( str_is_true( osrfHashGet(pcrud, "global_required") ) ) {
// If the global_required attribute is present and true, then the only owning
// org unit is the root org unit, i.e. the one with no parent.
@@ -1333,7 +1333,7 @@
"global-level permissions required, fetching top of the org tree" );
// check for perm at top of org tree
- char* org_tree_root_id = org_tree_root( ctx );
+ const char* org_tree_root_id = org_tree_root( ctx );
if( org_tree_root_id ) {
osrfStringArrayAdd( context_org_array, org_tree_root_id );
osrfLogDebug( OSRF_LOG_MARK, "top of the org tree is %s", org_tree_root_id );
@@ -1357,13 +1357,13 @@
jsonObject *param = NULL;
if( obj->classname ) {
- pkey_value = oilsFMGetString( obj, pkey );
+ pkey_value = oilsFMGetStringConst( obj, pkey );
if( !fetch )
param = jsonObjectClone( obj );
osrfLogDebug( OSRF_LOG_MARK, "Object supplied, using primary key value of %s",
pkey_value );
} else {
- pkey_value = jsonObjectToSimpleString( obj );
+ pkey_value = jsonObjectGetString( obj );
fetch = 1;
osrfLogDebug( OSRF_LOG_MARK, "Object not supplied, using primary key value "
"of %s and retrieving from the database", pkey_value );
@@ -1404,9 +1404,6 @@
);
free( m );
- if( pkey_value )
- free( pkey_value );
-
return 0;
}
@@ -1419,7 +1416,7 @@
int i = 0;
const char* lcontext = NULL;
while ( (lcontext = osrfStringArrayGetString(local_context, i++)) ) {
- osrfStringArrayAdd( context_org_array, oilsFMGetString( param, lcontext ) );
+ osrfStringArrayAdd( context_org_array, oilsFMGetStringConst( param, lcontext ));
osrfLogDebug(
OSRF_LOG_MARK,
"adding class-local field %s (value: %s) to the context org list",
@@ -1548,7 +1545,7 @@
osrfStringArray* ctx_array = osrfHashGet( fcontext, "context" );
while ( (foreign_field = osrfStringArrayGetString( ctx_array, j++ )) ) {
osrfStringArrayAdd( context_org_array,
- oilsFMGetString( _fparam, foreign_field ) );
+ oilsFMGetStringConst( _fparam, foreign_field ));
osrfLogDebug(
OSRF_LOG_MARK,
"adding foreign class %s field %s (value: %s) to the context org list",
@@ -1584,7 +1581,7 @@
while( (context_org = osrfStringArrayGetString( context_org_array, j++ )) ) {
dbi_result result;
- if( pkey_value ) {
+ if( pkey_value ) {
osrfLogDebug(
OSRF_LOG_MARK,
"Checking object permission [%s] for user %d "
@@ -1683,8 +1680,6 @@
break;
}
- if( pkey_value )
- free(pkey_value);
osrfStringArrayFree( context_org_array );
return OK;
@@ -1702,7 +1697,7 @@
The calling code is responsible for freeing the returned string.
*/
-static char* org_tree_root( osrfMethodContext* ctx ) {
+static const char* org_tree_root( osrfMethodContext* ctx ) {
static char cached_root_id[ 32 ] = ""; // extravagantly large buffer
static time_t last_lookup_time = 0;
@@ -1740,13 +1735,12 @@
return NULL;
}
- char* root_org_unit_id = oilsFMGetString( tree_top, "id" );
+ const char* root_org_unit_id = oilsFMGetStringConst( tree_top, "id" );
osrfLogDebug( OSRF_LOG_MARK, "Top of the org tree is %s", root_org_unit_id );
- jsonObjectFree( result );
-
strcpy( cached_root_id, root_org_unit_id );
- return root_org_unit_id;
+ jsonObjectFree( result );
+ return cached_root_id;
}
/**
Modified: trunk/Open-ILS/src/c-apps/oils_utils.c
===================================================================
--- trunk/Open-ILS/src/c-apps/oils_utils.c 2010-05-25 13:51:26 UTC (rev 16493)
+++ trunk/Open-ILS/src/c-apps/oils_utils.c 2010-05-25 16:21:56 UTC (rev 16494)
@@ -22,24 +22,67 @@
if (!oilsIDLInit( filename )) {
osrfLogError(OSRF_LOG_MARK, "Problem loading IDL file [%s]!", filename);
- if(freeable_filename) free(freeable_filename);
+ if(freeable_filename)
+ free(freeable_filename);
return NULL;
}
- if(freeable_filename) free(freeable_filename);
+ if(freeable_filename)
+ free(freeable_filename);
return oilsIDL();
}
+/**
+ @brief Return a const string with the value of a specified column in a row object.
+ @param object Pointer to the row object.
+ @param field Name of the column.
+ @return Pointer to a const string representing the value of the specified column,
+ or NULL in case of error.
+
+ The row object must be a JSON_ARRAY with a classname. The column value must be a
+ JSON_STRING or a JSON_NUMBER. Any other object type results in a return of NULL.
+
+ The return value points into the internal contents of the row object, which
+ retains ownership.
+*/
+const char* oilsFMGetStringConst( const jsonObject* object, const char* field ) {
+ return jsonObjectGetString(oilsFMGetObject( object, field ));
+}
+
+/**
+ @brief Return a string with the value of a specified column in a row object.
+ @param object Pointer to the row object.
+ @param field Name of the column.
+ @return Pointer to a newly allocated string representing the value of the specified column,
+ or NULL in case of error.
+
+ The row object must be a JSON_ARRAY with a classname. The column value must be a
+ JSON_STRING or a JSON_NUMBER. Any other object type results in a return of NULL.
+
+ The calling code is responsible for freeing the returned string by calling free().
+ */
char* oilsFMGetString( const jsonObject* object, const char* field ) {
return jsonObjectToSimpleString(oilsFMGetObject( object, field ));
}
+/**
+ @brief Return a pointer to the value of a specified column in a row object.
+ @param object Pointer to the row object.
+ @param field Name of the column.
+ @return Pointer to the jsonObject representing the value of the specified column, or NULL
+ in case of error.
+ The row object must be a JSON_ARRAY with a classname.
+
+ The return value may point to a JSON_NULL, JSON_STRING, JSON_NUMBER, or JSON_ARRAY. It
+ points into the internal contents of the row object, which retains ownership.
+*/
const jsonObject* oilsFMGetObject( const jsonObject* object, const char* field ) {
if(!(object && field)) return NULL;
if( object->type != JSON_ARRAY || !object->classname ) return NULL;
int pos = fm_ntop(object->classname, field);
- if( pos > -1 ) return jsonObjectGetIndex( object, pos );
+ if( pos > -1 )
+ return jsonObjectGetIndex( object, pos );
return NULL;
}
@@ -68,7 +111,10 @@
long id = -1;
if(!obj) return id;
char* ids = oilsFMGetString( obj, "id" );
- if(ids) { id = atol(ids); free(ids); }
+ if(ids) {
+ id = atol(ids);
+ free(ids);
+ }
return id;
}
@@ -78,6 +124,8 @@
int i;
oilsEvent* evt = NULL;
+ // Find the root org unit, i.e. the one with no parent.
+ // Assumption: there is only one org unit with no parent.
if (orgid == -1) {
jsonObject* where_clause = jsonParse( "{\"parent_ou\":null}" );
jsonObject* org = oilsUtilsQuickReq(
@@ -87,60 +135,116 @@
);
jsonObjectFree( where_clause );
- orgid = (int)jsonObjectGetNumber( oilsFMGetObject( org, "id" ) );
+ orgid = (int)jsonObjectGetNumber( oilsFMGetObject( org, "id" ) );
- jsonObjectFree(org);
- }
+ jsonObjectFree(org);
+ }
for( i = 0; i < size && permissions[i]; i++ ) {
char* perm = permissions[i];
jsonObject* params = jsonParseFmt("[%d, \"%s\", %d]", userid, perm, orgid);
- jsonObject* o = oilsUtilsQuickReq( "open-ils.storage",
+ jsonObject* o = oilsUtilsQuickReq( "open-ils.storage",
"open-ils.storage.permission.user_has_perm", params );
char* r = jsonObjectToSimpleString(o);
- if(r && !strcmp(r, "0"))
+ if(r && !strcmp(r, "0"))
evt = oilsNewEvent3( OSRF_LOG_MARK, OILS_EVENT_PERM_FAILURE, perm, orgid );
jsonObjectFree(params);
jsonObjectFree(o);
free(r);
- if(evt) break;
+ if(evt)
+ break;
}
return evt;
}
+/**
+ @brief Perform a remote procedure call.
+ @param service The name of the service to invoke.
+ @param method The name of the method to call.
+ @param params The parameters to be passed to the method, if any.
+ @return A copy of whatever the method returns as a result, or a JSON_NULL if the method
+ doesn't return anything.
+
+ If the @a params parameter points to a JSON_ARRAY, pass each element of the array
+ as a separate parameter. If it points to any other kind of jsonObject, pass it as a
+ single parameter. If it is NULL, pass no parameters.
+
+ The calling code is responsible for freeing the returned object by calling jsonObjectFree().
+*/
jsonObject* oilsUtilsQuickReq( const char* service, const char* method,
const jsonObject* params ) {
if(!(service && method)) return NULL;
+
osrfLogDebug(OSRF_LOG_MARK, "oilsUtilsQuickReq(): %s - %s", service, method );
- osrfAppSession* session = osrfAppSessionClientInit( service );
+
+ // Open an application session with the service, and send the request
+ osrfAppSession* session = osrfAppSessionClientInit( service );
int reqid = osrfAppSessionSendRequest( session, params, method, 1 );
- osrfMessage* omsg = osrfAppSessionRequestRecv( session, reqid, 60 );
- jsonObject* result = jsonObjectClone(osrfMessageGetResult(omsg));
+
+ // Get the response
+ osrfMessage* omsg = osrfAppSessionRequestRecv( session, reqid, 60 );
+ jsonObject* result = jsonObjectClone( osrfMessageGetResult(omsg) );
+
+ // Clean up
osrfMessageFree(omsg);
osrfAppSessionFree(session);
return result;
}
+/**
+ @brief Call a method of the open-ils.storage service.
+ @param method Name of the method.
+ @param params Parameters to be passed to the method, if any.
+ @return A copy of whatever the method returns as a result, or a JSON_NULL if the method
+ doesn't return anything.
+
+ If the @a params parameter points to a JSON_ARRAY, pass each element of the array
+ as a separate parameter. If it points to any other kind of jsonObject, pass it as a
+ single parameter. If it is NULL, pass no parameters.
+
+ The calling code is responsible for freeing the returned object by calling jsonObjectFree().
+*/
jsonObject* oilsUtilsStorageReq( const char* method, const jsonObject* params ) {
return oilsUtilsQuickReq( "open-ils.storage", method, params );
}
+/**
+ @brief Call a method of the open-ils.cstore service.
+ @param method Name of the method.
+ @param params Parameters to be passed to the method, if any.
+ @return A copy of whatever the method returns as a result, or a JSON_NULL if the method
+ doesn't return anything.
+
+ If the @a params parameter points to a JSON_ARRAY, pass each element of the array
+ as a separate parameter. If it points to any other kind of jsonObject, pass it as a
+ single parameter. If it is NULL, pass no parameters.
+
+ The calling code is responsible for freeing the returned object by calling jsonObjectFree().
+*/
jsonObject* oilsUtilsCStoreReq( const char* method, const jsonObject* params ) {
return oilsUtilsQuickReq("open-ils.cstore", method, params);
}
+/**
+ @brief Given a username, fetch the corresponding row from the actor.usr table, if any.
+ @param name The username for which to search.
+ @return A Fieldmapper object for the relevant row in the actor.usr table, if it exists;
+ or a JSON_NULL if it doesn't.
+
+ The calling code is responsible for freeing the returned object by calling jsonObjectFree().
+*/
jsonObject* oilsUtilsFetchUserByUsername( const char* name ) {
if(!name) return NULL;
jsonObject* params = jsonParseFmt("{\"usrname\":\"%s\"}", name);
- jsonObject* user = oilsUtilsQuickReq(
+ jsonObject* user = oilsUtilsQuickReq(
"open-ils.cstore", "open-ils.cstore.direct.actor.user.search", params );
jsonObjectFree(params);
@@ -149,6 +253,17 @@
return user;
}
+/**
+ @brief Given a barcode, fetch the corresponding row from the actor.usr table, if any.
+ @param name The barcode for which to search.
+ @return A Fieldmapper object for the relevant row in the actor.usr table, if it exists;
+ or a JSON_NULL if it doesn't.
+
+ Look up the barcode in actor.card. Follow a foreign key from there to get a row in
+ actor.usr.
+
+ The calling code is responsible for freeing the returned object by calling jsonObjectFree().
+*/
jsonObject* oilsUtilsFetchUserByBarcode(const char* barcode) {
if(!barcode) return NULL;
@@ -159,14 +274,18 @@
"open-ils.cstore", "open-ils.cstore.direct.actor.card.search", params );
jsonObjectFree(params);
- if(!card) return NULL;
+ if(!card)
+ return NULL; // No such card
+ // Get the user's id as a double
char* usr = oilsFMGetString(card, "usr");
jsonObjectFree(card);
- if(!usr) return NULL;
+ if(!usr)
+ return NULL; // No user id (shouldn't happen)
double iusr = strtod(usr, NULL);
free(usr);
+ // Look up the user in actor.usr
params = jsonParseFmt("[%f]", iusr);
jsonObject* user = oilsUtilsQuickReq(
"open-ils.cstore", "open-ils.cstore.direct.actor.user.retrieve", params);
@@ -182,9 +301,9 @@
jsonObject* set = oilsUtilsQuickReq(
"open-ils.actor",
- "open-ils.actor.ou_setting.ancestor_default", params);
+ "open-ils.actor.ou_setting.ancestor_default", params);
- char* value = jsonObjectToSimpleString(jsonObjectGetKey(set, "value"));
+ char* value = jsonObjectToSimpleString(jsonObjectGetKey(set, "value"));
jsonObjectFree(params);
jsonObjectFree(set);
osrfLogDebug(OSRF_LOG_MARK, "Fetched org [%d] setting: %s => %s", orgid, setting, value);
@@ -201,7 +320,7 @@
jsonObject* params = jsonParseFmt("[\"%s\"]", uname);
- jsonObject* o = oilsUtilsQuickReq(
+ jsonObject* o = oilsUtilsQuickReq(
"open-ils.auth", "open-ils.auth.authenticate.init", params );
const char* seed = jsonObjectGetString(o);
@@ -221,7 +340,8 @@
if(o) {
const char* tok = jsonObjectGetString(
jsonObjectGetKey(jsonObjectGetKey(o,"payload"), "authtoken"));
- if(tok) token = strdup(tok);
+ if( tok )
+ token = strdup( tok );
}
free(fullhash);
@@ -235,7 +355,7 @@
jsonObject* oilsUtilsFetchWorkstation( long id ) {
jsonObject* p = jsonParseFmt("[%ld]", id);
jsonObject* r = oilsUtilsQuickReq(
- "open-ils.storage",
+ "open-ils.storage",
"open-ils.storage.direct.actor.workstation.retrieve", p );
jsonObjectFree(p);
return r;
@@ -243,11 +363,8 @@
jsonObject* oilsUtilsFetchWorkstationByName( const char* name ) {
jsonObject* p = jsonParseFmt("{\"name\":\"%s\"}", name);
- jsonObject* r = oilsUtilsCStoreReq(
- "open-ils.cstore.direct.actor.workstation.search", p);
+ jsonObject* r = oilsUtilsCStoreReq(
+ "open-ils.cstore.direct.actor.workstation.search", p);
jsonObjectFree(p);
- return r;
+ return r;
}
-
-
-
More information about the open-ils-commits
mailing list