From 67339dfbe27a044134df346e71b177f99c121145 Mon Sep 17 00:00:00 2001 From: miker Date: Tue, 1 Jan 2008 01:52:44 +0000 Subject: [PATCH] Patch from Scott McKellar: 1. Added the const qualifier in various places. 2. Eliminated some unnecessary calls to strlen(), where they were used merely to determine whether a string was empty. 3. Ensured that all members of a new transport_message are explicitly populated. 4. Plugged a memory leak in the case where strdup() fails. 5. Eliminated some unhelpful casts of malloc'd pointers. 6. Added some protective tests for NULL pointer parameters. 7. In several spots I replaced numeric literals with character literals, both to make the code more readable and to avoid a needless dependence on ASCII. 8. Rewrote the jid_get_resource function to avoid repeatedly overwriting the same buffer. git-svn-id: svn://svn.open-ils.org/OpenSRF/trunk@1194 9efc2488-bf62-4759-914b-345cdb29e865 --- include/opensrf/transport_message.h | 15 +-- src/libopensrf/transport_message.c | 149 ++++++++++++++++++---------- 2 files changed, 102 insertions(+), 62 deletions(-) diff --git a/include/opensrf/transport_message.h b/include/opensrf/transport_message.h index ddd8f39..6cf3a49 100644 --- a/include/opensrf/transport_message.h +++ b/include/opensrf/transport_message.h @@ -28,7 +28,7 @@ struct transport_message_struct { char* router_to; char* router_class; char* router_command; - char* osrf_xid; + char* osrf_xid; int is_error; char* error_type; int error_code; @@ -42,16 +42,17 @@ typedef struct transport_message_struct transport_message; // within this method. // Returns NULL on error // --------------------------------------------------------------------------------- -transport_message* message_init( char* body, char* subject, - char* thread, char* recipient, char* sender ); +transport_message* message_init( const char* body, const char* subject, + const char* thread, const char* recipient, const char* sender ); transport_message* new_message_from_xml( const char* msg_xml ); -void message_set_router_info( transport_message* msg, char* router_from, - char* router_to, char* router_class, char* router_command, int broadcast_enabled ); +void message_set_router_info( transport_message* msg, const char* router_from, + const char* router_to, const char* router_class, const char* router_command, + int broadcast_enabled ); -void message_set_osrf_xid( transport_message* msg, char* osrf_xid ); +void message_set_osrf_xid( transport_message* msg, const char* osrf_xid ); // --------------------------------------------------------------------------------- // Formats the Jabber message as XML for encoding. @@ -93,7 +94,7 @@ void jid_get_resource( const char* jid, char buf[], int size ); /** Puts the domain portion of the given jid into the pre-allocated buffer */ void jid_get_domain( const char* jid, char buf[], int size ); -void set_msg_error( transport_message*, char* error_type, int error_code); +void set_msg_error( transport_message*, const char* error_type, int error_code); #endif diff --git a/src/libopensrf/transport_message.c b/src/libopensrf/transport_message.c index 80b1ea9..c66df27 100644 --- a/src/libopensrf/transport_message.c +++ b/src/libopensrf/transport_message.c @@ -4,17 +4,16 @@ // --------------------------------------------------------------------------------- // Allocates and initializes a new transport_message // --------------------------------------------------------------------------------- -transport_message* message_init( char* body, - char* subject, char* thread, char* recipient, char* sender ) { +transport_message* message_init( const char* body, const char* subject, + const char* thread, const char* recipient, const char* sender ) { - transport_message* msg = - (transport_message*) safe_malloc( sizeof(transport_message) ); + transport_message* msg = safe_malloc( sizeof(transport_message) ); - if( body == NULL ) { body = ""; } + if( body == NULL ) { body = ""; } if( thread == NULL ) { thread = ""; } if( subject == NULL ) { subject = ""; } if( sender == NULL ) { sender = ""; } - if( recipient == NULL ) { recipient = ""; } + if( recipient == NULL ) { recipient = ""; } msg->body = strdup(body); msg->thread = strdup(thread); @@ -27,20 +26,52 @@ transport_message* message_init( char* body, msg->sender == NULL ) { osrfLogError(OSRF_LOG_MARK, "message_init(): Out of Memory" ); + free( msg->body ); + free( msg->thread ); + free( msg->subject ); + free( msg->recipient ); + free( msg->sender ); + free( msg ); return NULL; } + msg->router_from = NULL; + msg->router_to = NULL; + msg->router_class = NULL; + msg->router_command = NULL; + msg->osrf_xid = NULL; + msg->is_error = 0; + msg->error_type = NULL; + msg->error_code = 0; + msg->broadcast = 0; + msg->msg_xml = NULL; + return msg; } transport_message* new_message_from_xml( const char* msg_xml ) { - if( msg_xml == NULL || strlen(msg_xml) < 1 ) + if( msg_xml == NULL || *msg_xml == '\0' ) return NULL; - transport_message* new_msg = - (transport_message*) safe_malloc( sizeof(transport_message) ); + transport_message* new_msg = safe_malloc( sizeof(transport_message) ); + + new_msg->body = NULL; + new_msg->subject = NULL; + new_msg->thread = NULL; + new_msg->recipient = NULL; + new_msg->sender = NULL; + new_msg->router_from = NULL; + new_msg->router_to = NULL; + new_msg->router_class = NULL; + new_msg->router_command = NULL; + new_msg->osrf_xid = NULL; + new_msg->is_error = 0; + new_msg->error_type = NULL; + new_msg->error_code = 0; + new_msg->broadcast = 0; + new_msg->msg_xml = NULL; xmlKeepBlanksDefault(0); xmlDocPtr msg_doc = xmlReadDoc( BAD_CAST msg_xml, NULL, NULL, 0 ); @@ -54,7 +85,7 @@ transport_message* new_message_from_xml( const char* msg_xml ) { xmlChar* router_to = xmlGetProp( root, BAD_CAST "router_to" ); xmlChar* router_class= xmlGetProp( root, BAD_CAST "router_class" ); xmlChar* broadcast = xmlGetProp( root, BAD_CAST "broadcast" ); - xmlChar* osrf_xid = xmlGetProp( root, BAD_CAST "osrf_xid" ); + xmlChar* osrf_xid = xmlGetProp( root, BAD_CAST "osrf_xid" ); if( osrf_xid ) { message_set_osrf_xid( new_msg, (char*) osrf_xid); @@ -62,40 +93,40 @@ transport_message* new_message_from_xml( const char* msg_xml ) { } if( router_from ) { - new_msg->sender = strdup((char*)router_from); + new_msg->sender = strdup((const char*)router_from); } else { if( sender ) { - new_msg->sender = strdup((char*)sender); + new_msg->sender = strdup((const char*)sender); xmlFree(sender); } } if( recipient ) { - new_msg->recipient = strdup((char*)recipient); + new_msg->recipient = strdup((const char*)recipient); xmlFree(recipient); } if(subject){ - new_msg->subject = strdup((char*)subject); + new_msg->subject = strdup((const char*)subject); xmlFree(subject); } if(thread) { - new_msg->thread = strdup((char*)thread); + new_msg->thread = strdup((const char*)thread); xmlFree(thread); } if(router_from) { - new_msg->router_from = strdup((char*)router_from); + new_msg->router_from = strdup((const char*)router_from); xmlFree(router_from); } if(router_to) { - new_msg->router_to = strdup((char*)router_to); + new_msg->router_to = strdup((const char*)router_to); xmlFree(router_to); } if(router_class) { - new_msg->router_class = strdup((char*)router_class); + new_msg->router_class = strdup((const char*)router_class); xmlFree(router_class); } if(broadcast) { - if(strcmp((char*) broadcast,"0") ) + if(strcmp((const char*) broadcast,"0") ) new_msg->broadcast = 1; xmlFree(broadcast); } @@ -103,19 +134,19 @@ transport_message* new_message_from_xml( const char* msg_xml ) { xmlNodePtr search_node = root->children; while( search_node != NULL ) { - if( ! strcmp( (char*) search_node->name, "thread" ) ) { + if( ! strcmp( (const char*) search_node->name, "thread" ) ) { if( search_node->children && search_node->children->content ) - new_msg->thread = strdup( (char*) search_node->children->content ); + new_msg->thread = strdup( (const char*) search_node->children->content ); } - if( ! strcmp( (char*) search_node->name, "subject" ) ) { + if( ! strcmp( (const char*) search_node->name, "subject" ) ) { if( search_node->children && search_node->children->content ) - new_msg->subject = strdup( (char*) search_node->children->content ); + new_msg->subject = strdup( (const char*) search_node->children->content ); } - if( ! strcmp( (char*) search_node->name, "body" ) ) { + if( ! strcmp( (const char*) search_node->name, "body" ) ) { if( search_node->children && search_node->children->content ) - new_msg->body = strdup((char*) search_node->children->content ); + new_msg->body = strdup((const char*) search_node->children->content ); } search_node = search_node->next; @@ -135,16 +166,19 @@ transport_message* new_message_from_xml( const char* msg_xml ) { return new_msg; } -void message_set_osrf_xid( transport_message* msg, char* osrf_xid ) { +void message_set_osrf_xid( transport_message* msg, const char* osrf_xid ) { if(!msg) return; if( osrf_xid ) msg->osrf_xid = strdup(osrf_xid); else msg->osrf_xid = strdup(""); } -void message_set_router_info( transport_message* msg, char* router_from, - char* router_to, char* router_class, char* router_command, int broadcast_enabled ) { +void message_set_router_info( transport_message* msg, const char* router_from, + const char* router_to, const char* router_class, const char* router_command, + int broadcast_enabled ) { + if( !msg ) return; + if(router_from) msg->router_from = strdup(router_from); else @@ -178,8 +212,9 @@ void message_set_router_info( transport_message* msg, char* router_from, /* encodes the message for traversal */ int message_prepare_xml( transport_message* msg ) { - if( msg->msg_xml != NULL ) { return 1; } - msg->msg_xml = message_to_xml( msg ); + if( !msg ) return 0; + if( msg->msg_xml == NULL ) + msg->msg_xml = message_to_xml( msg ); return 1; } @@ -199,7 +234,7 @@ int message_free( transport_message* msg ){ free(msg->router_to); free(msg->router_class); free(msg->router_command); - free(msg->osrf_xid); + free(msg->osrf_xid); if( msg->error_type != NULL ) free(msg->error_type); if( msg->msg_xml != NULL ) free(msg->msg_xml); free(msg); @@ -227,7 +262,7 @@ char* message_to_xml( const transport_message* msg ) { if( ! msg ) { osrfLogWarning(OSRF_LOG_MARK, "Passing NULL message to message_to_xml()"); - return 0; + return NULL; } doc = xmlReadDoc( BAD_CAST "", NULL, NULL, XML_PARSE_NSCLEAN ); @@ -260,21 +295,21 @@ char* message_to_xml( const transport_message* msg ) { char* subject = msg->subject; char* thread = msg->thread; - if( thread && strlen(thread) > 0 ) { + if( thread && *thread ) { thread_node = xmlNewChild(message_node, NULL, (xmlChar*) "thread", NULL ); xmlNodePtr txt = xmlNewText((xmlChar*) thread); xmlAddChild(thread_node, txt); xmlAddChild(message_node, thread_node); } - if( subject && strlen(subject) > 0 ) { + if( subject && *subject ) { subject_node = xmlNewChild(message_node, NULL, (xmlChar*) "subject", NULL ); xmlNodePtr txt = xmlNewText((xmlChar*) subject); xmlAddChild(subject_node, txt); xmlAddChild( message_node, subject_node ); } - if( body && strlen(body) > 0 ) { + if( body && *body ) { body_node = xmlNewChild(message_node, NULL, (xmlChar*) "body", NULL); xmlNodePtr txt = xmlNewText((xmlChar*) body); xmlAddChild(body_node, txt); @@ -283,7 +318,7 @@ char* message_to_xml( const transport_message* msg ) { xmlBufferPtr xmlbuf = xmlBufferCreate(); xmlNodeDump( xmlbuf, doc, xmlDocGetRootElement(doc), 0, 0); - char* xml = strdup((char*) (xmlBufferContent(xmlbuf))); + char* xml = strdup((const char*) (xmlBufferContent(xmlbuf))); xmlBufferFree(xmlbuf); xmlFreeDoc( doc ); xmlCleanupParser(); @@ -294,16 +329,16 @@ char* message_to_xml( const transport_message* msg ) { void jid_get_username( const char* jid, char buf[], int size ) { - if( jid == NULL ) { return; } + if( jid == NULL || buf == NULL || size <= 0 ) { return; } /* find the @ and return whatever is in front of it */ int len = strlen( jid ); int i; for( i = 0; i != len; i++ ) { - if( jid[i] == 64 ) { /*ascii @*/ + if( jid[i] == '@' ) { if(i > size) i = size; - strncpy( buf, jid, i ); - buf[i] = '\0'; // strncpy doesn't provide the nul + memcpy( buf, jid, i ); + buf[i] = '\0'; return; } } @@ -311,17 +346,19 @@ void jid_get_username( const char* jid, char buf[], int size ) { void jid_get_resource( const char* jid, char buf[], int size) { - if( jid == NULL ) { return; } - int len = strlen( jid ); - int i; - for( i = 0; i!= len; i++ ) { - if( jid[i] == 47 ) { /* ascii / */ - const char* start = jid + i + 1; /* right after the '/' */ - int rlen = len - (i+1); - if(rlen > size) rlen = size; - strncpy( buf, start, rlen ); - buf[rlen] = '\0'; // strncpy doesn't provide the nul - } + if( jid == NULL || buf == NULL || size <= 0 ) { return; } + + // Find the last slash, if any + + const char* start = strrchr( jid, '/' ); + if( start ) { + + // Copy the text beyond the slash, up to a maximum size + + size_t len = strlen( ++start ); + if( len > size ) len = size; + memcpy( buf, start, len ); + buf[ len ] = '\0'; } } @@ -335,9 +372,9 @@ void jid_get_domain( const char* jid, char buf[], int size ) { int index2 = 0; for( i = 0; i!= len; i++ ) { - if(jid[i] == 64) /* ascii @ */ + if(jid[i] == '@') index1 = i + 1; - else if(jid[i] == 47 && index1 != 0) /* ascii / */ + else if(jid[i] == '/' && index1 != 0) index2 = i; } @@ -349,9 +386,11 @@ void jid_get_domain( const char* jid, char buf[], int size ) { } } -void set_msg_error( transport_message* msg, char* type, int err_code ) { +void set_msg_error( transport_message* msg, const char* type, int err_code ) { - if( type != NULL && strlen( type ) > 0 ) { + if( !msg ) return; + + if( type != NULL && *type ) { msg->error_type = safe_malloc( strlen(type)+1); strcpy( msg->error_type, type ); msg->error_code = err_code; -- 2.43.2