From d558e091e2a19e66b2033e721eb07e6c7b960432 Mon Sep 17 00:00:00 2001 From: Lammert Bies Date: Mon, 19 Dec 2016 13:10:28 +0100 Subject: [PATCH] Code cleanup --- doc/api/httplib_get_user_data.md | 2 +- src/httplib_get_user_data.c | 17 ++- src/httplib_ssl_error.c | 4 +- src/httplib_ssl_get_protocol.c | 6 +- src/httplib_start.c | 132 ++++++++++++++---------- src/httplib_store_body.c | 35 ++++--- src/httplib_suggest_connection_header.c | 14 ++- src/httplib_timer.c | 7 +- src/httplib_tls_dtor.c | 10 +- src/httplib_worker_thread.c | 47 ++++----- src/win32_pthread_cond_init.c | 6 +- 11 files changed, 162 insertions(+), 118 deletions(-) diff --git a/doc/api/httplib_get_user_data.md b/doc/api/httplib_get_user_data.md index f946aa76..fd69e4ec 100644 --- a/doc/api/httplib_get_user_data.md +++ b/doc/api/httplib_get_user_data.md @@ -12,7 +12,7 @@ | Type | Description | | :--- | :--- | -|`void *`|| +|`void *`|A pointer to the user data or NULL on error, or if no user data has been registered| ### Description diff --git a/src/httplib_get_user_data.c b/src/httplib_get_user_data.c index bcfd53cf..780cee97 100644 --- a/src/httplib_get_user_data.c +++ b/src/httplib_get_user_data.c @@ -22,13 +22,24 @@ * THE SOFTWARE. * * ============ - * Release: 1.8 + * Release: 2.0 */ #include "httplib_main.h" -void * httplib_get_user_data(const struct httplib_context *ctx) { +/* + * void *httplib_get_user_data( const struct httplib_context *ctx ); + * + * The function httplib_get_user_data() returns a pointer to user data which is + * associated with the context, or NULL if no user data has been registered. + * The user_data is specified when the context is allocated with a call to the + * httplib_start() function. + */ - return (ctx == NULL) ? NULL : ctx->user_data; +void *httplib_get_user_data( const struct httplib_context *ctx ) { + + if ( ctx == NULL ) return NULL; + + return ctx->user_data; } /* httplib_get_user_data */ diff --git a/src/httplib_ssl_error.c b/src/httplib_ssl_error.c index cae3264c..674a3fa6 100644 --- a/src/httplib_ssl_error.c +++ b/src/httplib_ssl_error.c @@ -22,7 +22,7 @@ * THE SOFTWARE. * * ============ - * Release: 1.8 + * Release: 2.0 */ #include "httplib_main.h" @@ -43,7 +43,7 @@ const char *XX_httplib_ssl_error( void ) { unsigned long err; err = ERR_get_error(); - return ((err == 0) ? "" : ERR_error_string(err, NULL)); + return ( (err == 0) ? "" : ERR_error_string( err, NULL ) ); } /* XX_httplib_ssl_error */ diff --git a/src/httplib_ssl_get_protocol.c b/src/httplib_ssl_get_protocol.c index 56513cfe..6b454827 100644 --- a/src/httplib_ssl_get_protocol.c +++ b/src/httplib_ssl_get_protocol.c @@ -22,7 +22,7 @@ * THE SOFTWARE. * * ============ - * Release: 1.8 + * Release: 2.0 */ #include "httplib_main.h" @@ -39,7 +39,9 @@ long XX_httplib_ssl_get_protocol( int version_id ) { - long ret = SSL_OP_ALL; + long ret; + + ret = SSL_OP_ALL; if ( version_id > 0 ) ret |= SSL_OP_NO_SSLv2; if ( version_id > 1 ) ret |= SSL_OP_NO_SSLv3; diff --git a/src/httplib_start.c b/src/httplib_start.c index 5185ea17..8ead573a 100644 --- a/src/httplib_start.c +++ b/src/httplib_start.c @@ -58,79 +58,87 @@ struct httplib_context *httplib_start( const struct httplib_callbacks *callbacks WSAStartup(MAKEWORD(2, 2), &data); #endif /* _WIN32 */ - /* Allocate context and initialize reasonable general case defaults. */ - if ((ctx = httplib_calloc(1, sizeof(*ctx))) == NULL) return NULL; + ctx = httplib_calloc( 1, sizeof(*ctx) ); + if ( ctx == NULL ) return NULL; /* Random number generator will initialize at the first call */ ctx->auth_nonce_mask = (uint64_t)XX_httplib_get_random() ^ (uint64_t)(ptrdiff_t)(options); - if (httplib_atomic_inc(&XX_httplib_sTlsInit) == 1) { + if ( httplib_atomic_inc( & XX_httplib_sTlsInit ) == 1 ) { #if defined(_WIN32) - InitializeCriticalSection(&global_log_file_lock); + InitializeCriticalSection( & global_log_file_lock ); #endif /* _WIN32 */ #if !defined(_WIN32) - pthread_mutexattr_init(&XX_httplib_pthread_mutex_attr); - pthread_mutexattr_settype(&XX_httplib_pthread_mutex_attr, PTHREAD_MUTEX_RECURSIVE); + pthread_mutexattr_init( & XX_httplib_pthread_mutex_attr ); + pthread_mutexattr_settype( & XX_httplib_pthread_mutex_attr, PTHREAD_MUTEX_RECURSIVE ); #endif - if (0 != pthread_key_create(&XX_httplib_sTlsKey, XX_httplib_tls_dtor)) { + if ( 0 != pthread_key_create( & XX_httplib_sTlsKey, XX_httplib_tls_dtor ) ) { /* Fatal error - abort start. However, this situation should * never * occur in practice. */ - httplib_atomic_dec(&XX_httplib_sTlsInit); - httplib_cry( XX_httplib_fc(ctx), "Cannot initialize thread local storage"); + httplib_atomic_dec( & XX_httplib_sTlsInit ); + httplib_cry( XX_httplib_fc(ctx), "Cannot initialize thread local storage" ); httplib_free( ctx ); + return NULL; } } else { /* TODO (low): istead of sleeping, check if XX_httplib_sTlsKey is already * initialized. */ - httplib_sleep(1); + httplib_sleep( 1 ); } tls.is_master = -1; - tls.thread_idx = (unsigned)httplib_atomic_inc(&XX_httplib_thread_idx_max); + tls.thread_idx = (unsigned)httplib_atomic_inc( & XX_httplib_thread_idx_max ); #if defined(_WIN32) tls.pthread_cond_helper_mutex = NULL; #endif - pthread_setspecific(XX_httplib_sTlsKey, &tls); + pthread_setspecific( XX_httplib_sTlsKey, &tls ); - ok = 0 == pthread_mutex_init(&ctx->thread_mutex, &XX_httplib_pthread_mutex_attr); + ok = 0 == pthread_mutex_init( & ctx->thread_mutex, &XX_httplib_pthread_mutex_attr ); #if !defined(ALTERNATIVE_QUEUE) - ok &= 0 == pthread_cond_init(&ctx->sq_empty, NULL); - ok &= 0 == pthread_cond_init(&ctx->sq_full, NULL); + ok &= 0 == pthread_cond_init( & ctx->sq_empty, NULL ); + ok &= 0 == pthread_cond_init( & ctx->sq_full, NULL ); #endif - ok &= 0 == pthread_mutex_init(&ctx->nonce_mutex, &XX_httplib_pthread_mutex_attr); - if (!ok) { + ok &= 0 == pthread_mutex_init( & ctx->nonce_mutex, & XX_httplib_pthread_mutex_attr ); + if ( ! ok ) { /* Fatal error - abort start. However, this situation should never * occur in practice. */ - httplib_cry( XX_httplib_fc(ctx), "Cannot initialize thread synchronization objects"); + httplib_cry( XX_httplib_fc( ctx ), "Cannot initialize thread synchronization objects" ); httplib_free( ctx ); - pthread_setspecific(XX_httplib_sTlsKey, NULL); + pthread_setspecific( XX_httplib_sTlsKey, NULL ); + return NULL; } - if (callbacks) { - ctx->callbacks = *callbacks; - exit_callback = callbacks->exit_context; + if ( callbacks ) { + + ctx->callbacks = *callbacks; + exit_callback = callbacks->exit_context; ctx->callbacks.exit_context = 0; } ctx->user_data = user_data; - ctx->handlers = NULL; + ctx->handlers = NULL; while (options && (name = *options++) != NULL) { + if ((idx = XX_httplib_get_option_index(name)) == -1) { + httplib_cry( XX_httplib_fc(ctx), "Invalid option: %s", name); XX_httplib_free_context(ctx); pthread_setspecific(XX_httplib_sTlsKey, NULL); return NULL; - } else if ((value = *options++) == NULL) { + } + + if ((value = *options++) == NULL) { httplib_cry( XX_httplib_fc(ctx), "%s: option value cannot be NULL", name); XX_httplib_free_context(ctx); pthread_setspecific(XX_httplib_sTlsKey, NULL); return NULL; } + if (ctx->config[idx] != NULL) { httplib_cry( XX_httplib_fc(ctx), "warning: %s: duplicate option", name); httplib_free( ctx->config[idx] ); @@ -141,21 +149,21 @@ struct httplib_context *httplib_start( const struct httplib_callbacks *callbacks /* Set default value if needed */ for (i = 0; XX_httplib_config_options[i].name != NULL; i++) { default_value = XX_httplib_config_options[i].default_value; - if (ctx->config[i] == NULL && default_value != NULL) { - ctx->config[i] = XX_httplib_strdup(default_value); - } + if (ctx->config[i] == NULL && default_value != NULL) ctx->config[i] = XX_httplib_strdup(default_value); } #if defined(NO_FILES) - if (ctx->config[DOCUMENT_ROOT] != NULL) { - httplib_cry( XX_httplib_fc(ctx), "%s", "Document root must not be set"); - XX_httplib_free_context(ctx); - pthread_setspecific(XX_httplib_sTlsKey, NULL); + if ( ctx->config[DOCUMENT_ROOT] != NULL ) { + + httplib_cry( XX_httplib_fc( ctx ), "%s", "Document root must not be set" ); + XX_httplib_free_context( ctx ); + pthread_setspecific( XX_httplib_sTlsKey, NULL ); + return NULL; } #endif - XX_httplib_get_system_name(&ctx->systemName); + XX_httplib_get_system_name( & ctx->systemName ); /* NOTE(lsm): order is important here. SSL certificates must * be initialized before listening ports. UID must be set last. */ @@ -168,15 +176,22 @@ struct httplib_context *httplib_start( const struct httplib_callbacks *callbacks !XX_httplib_set_uid_option(ctx) || #endif !XX_httplib_set_acl_option(ctx)) { - XX_httplib_free_context(ctx); - pthread_setspecific(XX_httplib_sTlsKey, NULL); + + XX_httplib_free_context( ctx ); + pthread_setspecific( XX_httplib_sTlsKey, NULL ); + return NULL; } #if !defined(_WIN32) - /* Ignore SIGPIPE signal, so if browser cancels the request, it - * won't kill the whole process. */ - (void)signal(SIGPIPE, SIG_IGN); + + /* + * Ignore SIGPIPE signal, so if browser cancels the request, it + * won't kill the whole process. + */ + + signal(SIGPIPE, SIG_IGN); + #endif /* !_WIN32 */ workerthreadcount = atoi(ctx->config[NUM_THREADS]); @@ -229,50 +244,55 @@ struct httplib_context *httplib_start( const struct httplib_callbacks *callbacks } #if defined(USE_TIMERS) - if (timers_init(ctx) != 0) { - httplib_cry( XX_httplib_fc(ctx), "Error creating timers"); - XX_httplib_free_context(ctx); - pthread_setspecific(XX_httplib_sTlsKey, NULL); + if ( timers_init( ctx ) != 0 ) { + + httplib_cry( XX_httplib_fc( ctx ), "Error creating timers" ); + XX_httplib_free_context( ctx ); + pthread_setspecific( XX_httplib_sTlsKey, NULL ); + return NULL; } #endif /* Context has been created - init user libraries */ - if (ctx->callbacks.init_context) { - ctx->callbacks.init_context(ctx); - } + if ( ctx->callbacks.init_context != NULL ) ctx->callbacks.init_context( ctx ); + ctx->callbacks.exit_context = exit_callback; - ctx->context_type = 1; /* server context */ + ctx->context_type = 1; /* server context */ /* Start master (listening) thread */ - XX_httplib_start_thread_with_id( XX_httplib_master_thread, ctx, &ctx->masterthreadid); + XX_httplib_start_thread_with_id( XX_httplib_master_thread, ctx, &ctx->masterthreadid ); /* Start worker threads */ for (i = 0; i < ctx->cfg_worker_threads; i++) { struct worker_thread_args *wta = httplib_malloc( sizeof(struct worker_thread_args) ); - if (wta) { - wta->ctx = ctx; + + if (wta != NULL ) { + + wta->ctx = ctx; wta->index = (int)i; } if ((wta == NULL) || (XX_httplib_start_thread_with_id(XX_httplib_worker_thread, wta, &ctx->workerthreadids[i]) != 0)) { /* thread was not created */ - if (wta != NULL) httplib_free( wta ); + if ( wta != NULL ) httplib_free( wta ); + + if ( i > 0 ) httplib_cry( XX_httplib_fc( ctx ), "Cannot start worker thread %i: error %ld", i + 1, (long)ERRNO ); + + else { + httplib_cry( XX_httplib_fc( ctx ), "Cannot create threads: error %ld", (long)ERRNO ); + XX_httplib_free_context( ctx ); + pthread_setspecific( XX_httplib_sTlsKey, NULL ); - if (i > 0) { - httplib_cry( XX_httplib_fc(ctx), "Cannot start worker thread %i: error %ld", i + 1, (long)ERRNO); - } else { - httplib_cry( XX_httplib_fc(ctx), "Cannot create threads: error %ld", (long)ERRNO); - XX_httplib_free_context(ctx); - pthread_setspecific(XX_httplib_sTlsKey, NULL); return NULL; } break; } } - pthread_setspecific(XX_httplib_sTlsKey, NULL); + pthread_setspecific( XX_httplib_sTlsKey, NULL ); + return ctx; } /* httplib_start */ diff --git a/src/httplib_store_body.c b/src/httplib_store_body.c index 71e1f0f0..5a8562d1 100644 --- a/src/httplib_store_body.c +++ b/src/httplib_store_body.c @@ -36,17 +36,20 @@ int64_t httplib_store_body( struct httplib_connection *conn, const char *path ) { char buf[MG_BUF_LEN]; - int64_t len = 0; + int64_t len; int ret; int n; struct file fi; - if (conn->consumed_content != 0) { - httplib_cry(conn, "%s: Contents already consumed", __func__); + len = 0; + + if ( conn->consumed_content != 0 ) { + + httplib_cry( conn, "%s: Contents already consumed", __func__ ); return -11; } - ret = XX_httplib_put_dir(conn, path); + ret = XX_httplib_put_dir( conn, path ); if (ret < 0) { /* -1 for path too long, * -2 for path can not be created. */ @@ -57,23 +60,27 @@ int64_t httplib_store_body( struct httplib_connection *conn, const char *path ) return 0; } - if (XX_httplib_fopen(conn, path, "w", &fi) == 0) return -12; + if ( XX_httplib_fopen( conn, path, "w", & fi ) == 0 ) return -12; - ret = httplib_read(conn, buf, sizeof(buf)); - while (ret > 0) { - n = (int)fwrite(buf, 1, (size_t)ret, fi.fp); - if (n != ret) { - XX_httplib_fclose(&fi); - XX_httplib_remove_bad_file(conn, path); + ret = httplib_read( conn, buf, sizeof(buf) ); + + while ( ret > 0) { + + n = (int)fwrite( buf, 1, (size_t)ret, fi.fp ); + if ( n != ret ) { + + XX_httplib_fclose( & fi ); + XX_httplib_remove_bad_file( conn, path ); return -13; } - ret = httplib_read(conn, buf, sizeof(buf)); + ret = httplib_read( conn, buf, sizeof(buf) ); } /* TODO: XX_httplib_fclose should return an error, * and every caller should check and handle it. */ - if (fclose(fi.fp) != 0) { - XX_httplib_remove_bad_file(conn, path); + if ( fclose(fi.fp) != 0 ) { + + XX_httplib_remove_bad_file( conn, path ); return -14; } diff --git a/src/httplib_suggest_connection_header.c b/src/httplib_suggest_connection_header.c index 36751dcc..208d2dda 100644 --- a/src/httplib_suggest_connection_header.c +++ b/src/httplib_suggest_connection_header.c @@ -22,13 +22,21 @@ * THE SOFTWARE. * * ============ - * Release: 1.8 + * Release: 2.0 */ #include "httplib_main.h" -const char * XX_httplib_suggest_connection_header( const struct httplib_connection *conn ) { +/* + * const char *XX_httplib_suggest_connection_header( const struct httlib_connection *conn ); + * + * Based on the connection type, the function XX_httplib_connection_header() + * returns a string to be used in the header which suggests the connection to + * be either closed, or kept alive for further requests. + */ - return XX_httplib_should_keep_alive(conn) ? "keep-alive" : "close"; +const char *XX_httplib_suggest_connection_header( const struct httplib_connection *conn ) { + + return XX_httplib_should_keep_alive( conn ) ? "keep-alive" : "close"; } /* XX_httplib_suggest_connection_header */ diff --git a/src/httplib_timer.c b/src/httplib_timer.c index 42469ed8..92604f3b 100644 --- a/src/httplib_timer.c +++ b/src/httplib_timer.c @@ -186,9 +186,10 @@ static int timers_init( struct httplib_context *ctx ) { static void timers_exit( struct httplib_context *ctx ) { - if (ctx->timers) { - (void)pthread_mutex_destroy(&ctx->timers->mutex); -httplib:_free(ctx->timers); + if ( ctx->timers != NULL ) { + + pthread_mutex_destroy( & ctx->timers->mutex ); + httplib_free( ctx->timers ); } } /* timers_exit */ diff --git a/src/httplib_tls_dtor.c b/src/httplib_tls_dtor.c index c76e4844..bfd97742 100644 --- a/src/httplib_tls_dtor.c +++ b/src/httplib_tls_dtor.c @@ -34,19 +34,23 @@ * void XX_httplib_tls_dtor( void *key ); * * The function XX_httplib_tls_dtor() is used sets the TLS key for the thread. + * The Thread Local Storage key is used in identifying thread specific storage. */ void XX_httplib_tls_dtor( void *key ) { struct httplib_workerTLS *tls = (struct httplib_workerTLS *)key; - /* key == pthread_getspecific(XX_httplib_sTlsKey); */ + /* key == pthread_getspecific( XX_httplib_sTlsKey ); */ + + if ( tls != NULL ) { - if (tls) { if (tls->is_master == 2) { + tls->is_master = -3; /* Mark memory as dead */ httplib_free( tls ); } } - pthread_setspecific(XX_httplib_sTlsKey, NULL); + + pthread_setspecific( XX_httplib_sTlsKey, NULL ); } /* XX_httplib_tls_dtor */ diff --git a/src/httplib_worker_thread.c b/src/httplib_worker_thread.c index 2358ce24..97d11431 100644 --- a/src/httplib_worker_thread.c +++ b/src/httplib_worker_thread.c @@ -48,7 +48,7 @@ void *XX_httplib_worker_thread( void *thread_func_param ) { #endif struct worker_thread_args *pwta = (struct worker_thread_args *)thread_func_param; - worker_thread_run(pwta); + worker_thread_run( pwta ); httplib_free( thread_func_param ); return 0; @@ -78,15 +78,12 @@ static void *worker_thread_run( struct worker_thread_args *thread_args ) { tls.pthread_cond_helper_mutex = CreateEvent(NULL, FALSE, FALSE, NULL); #endif - if (ctx->callbacks.init_thread) { - /* call init_thread for a worker thread (type 1) */ - ctx->callbacks.init_thread(ctx, 1); - } + if ( ctx->callbacks.init_thread != NULL ) ctx->callbacks.init_thread( ctx, 1 ); /* call init_thread for a worker thread (type 1) */ conn = httplib_calloc( 1, sizeof(*conn) + MAX_REQUEST_SIZE ); - if (conn == NULL) { - httplib_cry( XX_httplib_fc(ctx), "%s", "Cannot create new connection struct, OOM"); - } else { + if ( conn == NULL ) httplib_cry( XX_httplib_fc(ctx), "%s", "Cannot create new connection struct, OOM"); + + else { pthread_setspecific(XX_httplib_sTlsKey, &tls); conn->buf_size = MAX_REQUEST_SIZE; conn->buf = (char *)(conn + 1); @@ -96,12 +93,13 @@ static void *worker_thread_run( struct worker_thread_args *thread_args ) { /* Allocate a mutex for this connection to allow communication both * within the request handler and from elsewhere in the application */ - pthread_mutex_init(&conn->mutex, &XX_httplib_pthread_mutex_attr); + pthread_mutex_init( & conn->mutex, &XX_httplib_pthread_mutex_attr ); /* Call XX_httplib_consume_socket() even when ctx->stop_flag > 0, to let it * signal sq_empty condvar to wake up the master waiting in * produce_socket() */ while (XX_httplib_consume_socket(ctx, &conn->client, conn->thread_index)) { + conn->conn_birth_time = time(NULL); /* Fill in IP, port info early so even if SSL setup below fails, @@ -109,19 +107,15 @@ static void *worker_thread_run( struct worker_thread_args *thread_args ) { * Thanks to Johannes Winkelmann for the patch. */ #if defined(USE_IPV6) - if (conn->client.rsa.sa.sa_family == AF_INET6) { - conn->request_info.remote_port = - ntohs(conn->client.rsa.sin6.sin6_port); - } else + if ( conn->client.rsa.sa.sa_family == AF_INET6 ) conn->request_info.remote_port = ntohs(conn->client.rsa.sin6.sin6_port); + + else #endif { - conn->request_info.remote_port = - ntohs(conn->client.rsa.sin.sin_port); + conn->request_info.remote_port = ntohs(conn->client.rsa.sin.sin_port); } - XX_httplib_sockaddr_to_string(conn->request_info.remote_addr, - sizeof(conn->request_info.remote_addr), - &conn->client.rsa); + XX_httplib_sockaddr_to_string(conn->request_info.remote_addr, sizeof(conn->request_info.remote_addr), &conn->client.rsa); conn->request_info.is_ssl = conn->client.is_ssl; @@ -131,10 +125,10 @@ static void *worker_thread_run( struct worker_thread_args *thread_args ) { if ( XX_httplib_sslize( conn, conn->ctx->ssl_ctx, SSL_accept ) ) { /* Get SSL client certificate information (if set) */ - XX_httplib_ssl_get_client_cert_info(conn); + XX_httplib_ssl_get_client_cert_info( conn ); /* process HTTPS connection */ - XX_httplib_process_new_connection(conn); + XX_httplib_process_new_connection( conn ); /* Free client certificate info */ if ( conn->request_info.client_cert != NULL ) { @@ -149,20 +143,17 @@ static void *worker_thread_run( struct worker_thread_args *thread_args ) { } } #endif - } else { - /* process HTTP connection */ - XX_httplib_process_new_connection( conn ); - } + } else XX_httplib_process_new_connection( conn ); - XX_httplib_close_connection(conn); + XX_httplib_close_connection( conn ); } } - pthread_setspecific(XX_httplib_sTlsKey, NULL); + pthread_setspecific( XX_httplib_sTlsKey, NULL ); #if defined(_WIN32) - CloseHandle(tls.pthread_cond_helper_mutex); + CloseHandle( tls.pthread_cond_helper_mutex ); #endif - pthread_mutex_destroy(&conn->mutex); + pthread_mutex_destroy( & conn->mutex ); httplib_free( conn ); return NULL; diff --git a/src/win32_pthread_cond_init.c b/src/win32_pthread_cond_init.c index 7ba458e8..ab0984d2 100644 --- a/src/win32_pthread_cond_init.c +++ b/src/win32_pthread_cond_init.c @@ -30,11 +30,11 @@ #if defined(_WIN32) -int pthread_cond_init( pthread_cond_t *cv, const void *unused ) { +int pthread_cond_init( pthread_cond_t *cv, const void *attr ) { - (void)unused; + UNUSED_PARAMETER(attr); - InitializeCriticalSection(&cv->threadIdSec); + InitializeCriticalSection( & cv->threadIdSec ); cv->waiting_thread = NULL; return 0;