From 2326eb2b4d0b0afcfd05f996b9cb097979c2a14a Mon Sep 17 00:00:00 2001 From: scottmk Date: Fri, 30 Oct 2009 05:04:01 +0000 Subject: [PATCH] 1. Tidy up the white space. 2. Add copious comments, mostly doxygen-style, to document the functions and the transport_client struct. 3. In client_connect(): plug a memory leak by freeing client->xmpp_id before overlaying it. Plug a potential similar leak in client_send_message(). 4. In client_send_message(): return 1 (an error) instead of 0 (success) when the first parameter is NULL. M include/opensrf/transport_client.h M src/libopensrf/transport_client.c git-svn-id: svn://svn.open-ils.org/OpenSRF/trunk@1833 9efc2488-bf62-4759-914b-345cdb29e865 --- include/opensrf/transport_client.h | 59 +++++-------- src/libopensrf/transport_client.c | 173 +++++++++++++++++++++++++++++++------ 2 files changed, 168 insertions(+), 64 deletions(-) diff --git a/include/opensrf/transport_client.h b/include/opensrf/transport_client.h index d7dffb9..83819f3 100644 --- a/include/opensrf/transport_client.h +++ b/include/opensrf/transport_client.h @@ -1,6 +1,14 @@ #ifndef TRANSPORT_CLIENT_H #define TRANSPORT_CLIENT_H +/** + @file transport_client.h + @brief Header for implementation of transport_client. + + The transport_client routines provide an interface for sending and receiving Jabber + messages, one at a time. +*/ + #include #include #include @@ -12,63 +20,36 @@ extern "C" { struct message_list_struct; -// --------------------------------------------------------------------------- -// Our client struct. We manage a list of messages and a controlling session -// --------------------------------------------------------------------------- +/** + @brief A collection of members used for keeping track of transport_messages. + + Among other things, this struct includes a queue of incoming messages, and + a Jabber ID for outgoing messages. +*/ struct transport_client_struct { - transport_message* msg_q_head; - transport_message* msg_q_tail; - transport_session* session; - int error; - char* host; - char* xmpp_id; + transport_message* msg_q_head; /**< Head of message queue */ + transport_message* msg_q_tail; /**< Tail of message queue */ + transport_session* session; /**< Manages lower-level message processing */ + int error; /**< Boolean: true if an error has occurred */ + char* host; /**< Domain name or IP address of the Jabber server */ + char* xmpp_id; /**< Jabber ID used for outgoing messages */ }; typedef struct transport_client_struct transport_client; -// --------------------------------------------------------------------------- -// Allocates and initializes a transport_client. This does no connecting -// The user must call client_free(client) when finished with the allocated -// object. -// if port > 0 => connect via TCP -// else if unix_path != NULL => connect via UNIX socket -// --------------------------------------------------------------------------- transport_client* client_init( const char* server, int port, const char* unix_path, int component ); - -// --------------------------------------------------------------------------- -// Connects to the Jabber server with the provided information. Returns 1 on -// success, 0 otherwise. -// --------------------------------------------------------------------------- int client_connect( transport_client* client, const char* username, const char* password, const char* resource, int connect_timeout, enum TRANSPORT_AUTH_TYPE auth_type ); - int client_disconnect( transport_client* client ); -// --------------------------------------------------------------------------- -// De-allocates memory associated with a transport_client object. Users -// must use this method when finished with a client object. -// --------------------------------------------------------------------------- int client_free( transport_client* client ); -// --------------------------------------------------------------------------- -// Sends the given message. The message must at least have the recipient -// field set. -// --------------------------------------------------------------------------- int client_send_message( transport_client* client, transport_message* msg ); -// --------------------------------------------------------------------------- -// Returns 1 if this client is currently connected to the server, 0 otherwise -// --------------------------------------------------------------------------- int client_connected( const transport_client* client ); -// --------------------------------------------------------------------------- -// If there are any messages in the message list, the 'oldest' message is -// returned. If not, this function will wait at most 'timeout' seconds -// for a message to arrive. Specifying -1 means that this function will not -// return unless a message arrives. -// --------------------------------------------------------------------------- transport_message* client_recv( transport_client* client, int timeout ); #ifdef __cplusplus diff --git a/src/libopensrf/transport_client.c b/src/libopensrf/transport_client.c index 8118d95..df82bf1 100644 --- a/src/libopensrf/transport_client.c +++ b/src/libopensrf/transport_client.c @@ -1,5 +1,15 @@ #include +/** + @file transport_client.c + @brief Collection of routines for sending and receiving single messages over Jabber. + + These functions form an API built on top of the transport_session API. They serve + two main purposes: + - They remember a Jabber ID to use when sending messages. + - They maintain a queue of input messages that the calling code can get one at a time. +*/ + static void client_message_handler( void* client, transport_message* msg ); //int main( int argc, char** argv ); @@ -12,11 +22,11 @@ int main( int argc, char** argv ) { transport_client* client = client_init( "spacely.georgialibraries.org", 5222 ); - // try to connect, allow 15 second connect timeout + // try to connect, allow 15 second connect timeout if( client_connect( client, "admin", "asdfjkjk", "system", 15 ) ) { printf("Connected...\n"); - } else { - printf( "NOT Connected...\n" ); exit(99); + } else { + printf( "NOT Connected...\n" ); exit(99); } while( (recv = client_recv( client, -1 )) ) { @@ -24,7 +34,7 @@ int main( int argc, char** argv ) { if( recv->body ) { int len = strlen(recv->body); char buf[len + 20]; - osrf_clearbuf( buf, 0, sizeof(buf)); + osrf_clearbuf( buf, 0, sizeof(buf)); sprintf( buf, "Echoing...%s", recv->body ); send = message_init( buf, "Echoing Stuff", "12345", recv->sender, "" ); } else { @@ -33,7 +43,7 @@ int main( int argc, char** argv ) { if( send == NULL ) { printf("something's wrong"); } client_send_message( client, send ); - + message_free( send ); message_free( recv ); } @@ -46,6 +56,19 @@ int main( int argc, char** argv ) { */ +/** + @brief Allocate and initialize a transport_client. + @param server Hostname or IP address where the Jabber server resides. + @param port Port used for connecting to Jabber (0 if using UNIX domain socket). + @param unix_path Name of Jabber's socket in file system (if using UNIX domain socket). + @param component Boolean; true if we're a Jabber component. + @return A pointer to a newly created transport_client. + + Create a transport_client with a transport_session and an empty message queue (but don't open a connection yet). + Install a callback function in the transport_session to enqueue incoming messages. + + The calling code is responsible for freeing the transport_client by calling client_free(). +*/ transport_client* client_init( const char* server, int port, const char* unix_path, int component ) { if(server == NULL) return NULL; @@ -56,46 +79,115 @@ transport_client* client_init( const char* server, int port, const char* unix_pa /* start with an empty message queue */ client->msg_q_head = NULL; client->msg_q_tail = NULL; - + /* build the session */ client->session = init_transport( server, port, unix_path, client, component ); client->session->message_callback = client_message_handler; client->error = 0; - client->host = strdup(server); + client->host = strdup(server); + client->xmpp_id = NULL; return client; } -int client_connect( transport_client* client, - const char* username, const char* password, const char* resource, +/** + @brief Open a Jabber session for a transport_client. + @param client Pointer to the transport_client. + @param username Jabber user name. + @param password Password for the Jabber logon. + @param resource Resource name for the Jabber logon. + @param connect_timeout How many seconds to wait for the connection to open. + @param auth_type An enum: either AUTH_PLAIN or AUTH_DIGEST (see notes). + @return 1 if successful, or 0 upon error. + + Besides opening the Jabber session, create a Jabber ID for future use. + + If @a connect_timeout is -1, wait indefinitely for the Jabber server to respond. If + @a connect_timeout is zero, don't wait at all. If @a timeout is positive, wait that + number of seconds before timing out. If @a connect_timeout has a negative value other + than -1, the results are not well defined. + + The value of @a connect_timeout applies to each of two stages in the logon procedure. + Hence the logon may take up to twice the amount of time indicated. + + If we connect as a Jabber component, we send the password as an SHA1 hash. Otherwise + we look at the @a auth_type. If it's AUTH_PLAIN, we send the password as plaintext; if + it's AUTH_DIGEST, we send it as a hash. + */ +int client_connect( transport_client* client, + const char* username, const char* password, const char* resource, int connect_timeout, enum TRANSPORT_AUTH_TYPE auth_type ) { - if(client == NULL) return 0; - client->xmpp_id = va_list_to_string("%s@%s/%s", username, client->host, resource); - return session_connect( client->session, username, + if( client == NULL ) + return 0; + + // Create and store a Jabber ID + if( client->xmpp_id ) + free( client->xmpp_id ); + client->xmpp_id = va_list_to_string( "%s@%s/%s", username, client->host, resource ); + + // Open a transport_session + return session_connect( client->session, username, password, resource, connect_timeout, auth_type ); } +/** + @brief Disconnect from the Jabber session. + @param client Pointer to the transport_client. + @return 0 in all cases. + If there are any messages still in the queue, they stay there; i.e. we don't free them here. +*/ int client_disconnect( transport_client* client ) { if( client == NULL ) { return 0; } return session_disconnect( client->session ); } +/** + @brief Report whether a transport_client is connected. + @param client Pointer to the transport_client. + @return Boolean: 1 if connected, or 0 if not. +*/ int client_connected( const transport_client* client ) { if(client == NULL) return 0; return session_connected( client->session ); } +/** + @brief Send a transport message to the current destination. + @param client Pointer to a transport_client. + @param msg Pointer to the transport_message to be sent. + @return 0 if successful, or -1 if not. + + Translate the transport_message into XML and send it to Jabber, using the previously + stored Jabber ID for the sender. +*/ int client_send_message( transport_client* client, transport_message* msg ) { - if(client == NULL) return 0; - if( client->error ) return -1; - msg->sender = strdup(client->xmpp_id); // free'd in message_free + if( client == NULL || client->error ) + return -1; + if( msg->sender ) + free( msg->sender ); + msg->sender = strdup(client->xmpp_id); return session_send_msg( client->session, msg ); } +/** + @brief Fetch an input message, if one is available. + @param client Pointer to a transport_client. + @param timeout How long to wait for a message to arrive, in seconds (see remarks). + @return A pointer to a transport_message if successful, or NULL if not. + If there is a message already in the queue, return it immediately. Otherwise read any + available messages from the transport_session (subject to a timeout), and return the + first one. + + If the value of @a timeout is -1, then there is no time limit -- wait indefinitely until a + message arrives (or we error out for other reasons). If the value of @a timeout is zero, + don't wait at all. + + The calling code is responsible for freeing the transport_message by calling message_free(). +*/ transport_message* client_recv( transport_client* client, int timeout ) { if( client == NULL ) { return NULL; } @@ -103,7 +195,25 @@ transport_message* client_recv( transport_client* client, int timeout ) { if( NULL == client->msg_q_head ) { - /* no messaage available? try to get one */ + // No message available on the queue? Try to get a fresh one. + + // When we call session_wait(), it reads a socket for new messages. When it finds + // one, it enqueues it by calling the callback function client_message_handler(), + // which we installed in the transport_session when we created the transport_client. + + // Since a single call to session_wait() may not result in the receipt of a complete + // message. we call it repeatedly until we get either a message or an error. + + // Alternatively, a single call to session_wait() may result in the receipt of + // multiple messages. That's why we have to enqueue them. + + // The timeout applies to the receipt of a complete message. For a sufficiently + // short timeout, a sufficiently long message, and a sufficiently slow connection, + // we could timeout on the first message even though we're still receiving data. + + // Likewise we could time out while still receiving the second or subsequent message, + // return the first message, and resume receiving messages later. + if( timeout == -1 ) { /* wait potentially forever for data to arrive */ int x; @@ -136,7 +246,7 @@ transport_message* client_recv( transport_client* client, int timeout ) { } while( NULL == client->msg_q_head && remaining > 0 ); } } - + transport_message* msg = NULL; if( error ) @@ -153,14 +263,22 @@ transport_message* client_recv( transport_client* client, int timeout ) { return msg; } -// --------------------------------------------------------------------------- -// This is the message handler required by transport_session. This handler -// takes an incoming message and adds it to the tail of a message queue. -// --------------------------------------------------------------------------- +/** + @brief Enqueue a newly received transport_message. + @param client A pointer to a transport_client, cast to a void pointer. + @param msg A new transport message. + + Add a newly arrived input message to the tail of the queue. + + This is a callback function. The transport_session parses the XML coming in through a + socket, accumulating various bits and pieces. When it sees the end of a message stanza, + it packages the bits and pieces into a transport_message that it passes to this function, + which enqueues the message for processing. +*/ static void client_message_handler( void* client, transport_message* msg ){ if(client == NULL) return; - if(msg == NULL) return; + if(msg == NULL) return; transport_client* cli = (transport_client*) client; @@ -175,8 +293,13 @@ static void client_message_handler( void* client, transport_message* msg ){ } +/** + @brief Free a transport_client, along with all resources it owns. + @param client Pointer to the transport_client to be freed. + @return 1 if successful, or 0 if not. The only error condition is if @a client is NULL. +*/ int client_free( transport_client* client ){ - if(client == NULL) return 0; + if(client == NULL) return 0; session_free( client->session ); transport_message* current = client->msg_q_head; @@ -189,8 +312,8 @@ int client_free( transport_client* client ){ current = next; } - free(client->host); - free(client->xmpp_id); + free(client->host); + free(client->xmpp_id); free( client ); return 1; } -- 2.11.0