From d38a323526088f5eb679ff51c03e4373343fc0ef Mon Sep 17 00:00:00 2001 From: Emmanuel Ledoux Date: Thu, 19 Jun 2014 12:03:36 +0200 Subject: [PATCH] winpr-comm, winpr-file: better initialization of the static variables --- winpr/include/winpr/file.h | 2 +- winpr/libwinpr/comm/comm.c | 55 ++++++++++++++++++++++++-------------- winpr/libwinpr/file/file.c | 53 +++++++++++++++++++++++++----------- 3 files changed, 73 insertions(+), 37 deletions(-) diff --git a/winpr/include/winpr/file.h b/winpr/include/winpr/file.h index 9f8d748a8..ef5c4dd54 100644 --- a/winpr/include/winpr/file.h +++ b/winpr/include/winpr/file.h @@ -321,7 +321,7 @@ typedef HANDLE (*pcCreateFileA)(LPCSTR lpFileName, DWORD dwDesiredAccess, DWORD typedef struct _HANDLE_CREATOR { pcIsHandled IsHandled; - pcCreateFileA CreateFileA; /* TMP: FIXME: CreateFileA or CreateFile ? */ + pcCreateFileA CreateFileA; } HANDLE_CREATOR, *PHANDLE_CREATOR, *LPHANDLE_CREATOR; BOOL RegisterHandleCreator(PHANDLE_CREATOR pHandleCreator); diff --git a/winpr/libwinpr/comm/comm.c b/winpr/libwinpr/comm/comm.c index 39bde3bc1..b99d53255 100644 --- a/winpr/libwinpr/comm/comm.c +++ b/winpr/libwinpr/comm/comm.c @@ -26,15 +26,16 @@ #ifndef _WIN32 +#include +#include #include +#include #include #include #include #include #include -#include - #include #include @@ -749,31 +750,33 @@ typedef struct _COMM_DEVICE } COMM_DEVICE; /* FIXME: get a clever data structure, see also io.h functions */ -static COMM_DEVICE **_CommDevices = NULL; - +/* _CommDevices is a NULL-terminated array with a maximun of COMM_DEVICE_MAX COMM_DEVICE */ #define COMM_DEVICE_MAX 128 +static COMM_DEVICE **_CommDevices = NULL; static HANDLE_CREATOR *_CommHandleCreator = NULL; - -static void _CommDevicesInit() +static pthread_once_t _CommInitialized = PTHREAD_ONCE_INIT; +static void _CommInit() { - /* - * TMP: FIXME: What kind of mutex should be used here? - * better have to let DefineCommDevice() and QueryCommDevice() thread unsafe ? - * use of a module_init() ? - */ + /* NB: error management to be done outside of this function */ - if (_CommDevices == NULL) + assert(_CommDevices == NULL); + assert(_CommHandleCreator == NULL); + + _CommDevices = (COMM_DEVICE**)calloc(COMM_DEVICE_MAX+1, sizeof(COMM_DEVICE*)); + + _CommHandleCreator = (HANDLE_CREATOR*)malloc(sizeof(HANDLE_CREATOR)); + if (_CommHandleCreator) { - _CommDevices = (COMM_DEVICE**)calloc(COMM_DEVICE_MAX+1, sizeof(COMM_DEVICE*)); - - _CommHandleCreator = (HANDLE_CREATOR*)malloc(sizeof(HANDLE_CREATOR)); _CommHandleCreator->IsHandled = IsCommDevice; _CommHandleCreator->CreateFileA = CommCreateFileA; - + RegisterHandleCreator(_CommHandleCreator); } + + assert(_CommDevices != NULL); + assert(_CommHandleCreator != NULL); } @@ -818,6 +821,7 @@ static BOOL _IsReservedCommDeviceName(LPCTSTR lpName) * information, call GetLastError. * * ERRORS: + * ERROR_DLL_INIT_FAILED * ERROR_OUTOFMEMORY was not possible to get mappings. * ERROR_INVALID_DATA was not possible to add the device. */ @@ -827,10 +831,15 @@ BOOL DefineCommDevice(/* DWORD dwFlags,*/ LPCTSTR lpDeviceName, LPCTSTR lpTarget LPTSTR storedDeviceName = NULL; LPTSTR storedTargetPath = NULL; - _CommDevicesInit(); + if (pthread_once(&_CommInitialized, _CommInit) != 0) + { + SetLastError(ERROR_DLL_INIT_FAILED); + goto error_handle; + } + if (_CommDevices == NULL) { - SetLastError(ERROR_OUTOFMEMORY); + SetLastError(ERROR_DLL_INIT_FAILED); goto error_handle; } @@ -918,6 +927,7 @@ BOOL DefineCommDevice(/* DWORD dwFlags,*/ LPCTSTR lpDeviceName, LPCTSTR lpTarget * * ERRORS: * ERROR_SUCCESS + * ERROR_DLL_INIT_FAILED * ERROR_OUTOFMEMORY was not possible to get mappings. * ERROR_NOT_SUPPORTED equivalent QueryDosDevice feature not supported. * ERROR_INVALID_DATA was not possible to retrieve any device information. @@ -930,10 +940,15 @@ DWORD QueryCommDevice(LPCTSTR lpDeviceName, LPTSTR lpTargetPath, DWORD ucchMax) SetLastError(ERROR_SUCCESS); - _CommDevicesInit(); + if (pthread_once(&_CommInitialized, _CommInit) != 0) + { + SetLastError(ERROR_DLL_INIT_FAILED); + return 0; + } + if (_CommDevices == NULL) { - SetLastError(ERROR_OUTOFMEMORY); + SetLastError(ERROR_DLL_INIT_FAILED); return 0; } diff --git a/winpr/libwinpr/file/file.c b/winpr/libwinpr/file/file.c index 4908a7191..a48e1befa 100644 --- a/winpr/libwinpr/file/file.c +++ b/winpr/libwinpr/file/file.c @@ -42,7 +42,7 @@ /** * api-ms-win-core-file-l1-2-0.dll: - * + * * CreateFileA * CreateFileW * CreateFile2 @@ -150,8 +150,10 @@ #include #endif +#include #include #include +#include #include #include #include @@ -190,34 +192,45 @@ * winpr-collections to avoid a circular dependency * _HandleCreators = ArrayList_New(TRUE); */ +/* _HandleCreators is a NULL-terminated array with a maximun of HANDLE_CREATOR_MAX HANDLE_CREATOR */ +#define HANDLE_CREATOR_MAX 128 static HANDLE_CREATOR **_HandleCreators = NULL; -#define HANDLE_CREATOR_MAX 128 - +static pthread_once_t _HandleCreatorsInitialized = PTHREAD_ONCE_INIT; static void _HandleCreatorsInit() { - /* - * TMP: FIXME: What kind of mutex should be used here? use of - * a module_init()? - */ + /* NB: error management to be done outside of this function */ - if (_HandleCreators == NULL) - { - _HandleCreators = (HANDLE_CREATOR**)calloc(HANDLE_CREATOR_MAX+1, sizeof(HANDLE_CREATOR*)); - } + assert(_HandleCreators == NULL); + + _HandleCreators = (HANDLE_CREATOR**)calloc(HANDLE_CREATOR_MAX+1, sizeof(HANDLE_CREATOR*)); + + assert(_HandleCreators != NULL); } /** * Returns TRUE on success, FALSE otherwise. * * ERRORS: + * ERROR_DLL_INIT_FAILED * ERROR_INSUFFICIENT_BUFFER _HandleCreators full */ BOOL RegisterHandleCreator(PHANDLE_CREATOR pHandleCreator) { int i; - _HandleCreatorsInit(); + if (pthread_once(&_HandleCreatorsInitialized, _HandleCreatorsInit) != 0) + { + SetLastError(ERROR_DLL_INIT_FAILED); + return FALSE; + } + + if (_HandleCreators == NULL) + { + SetLastError(ERROR_DLL_INIT_FAILED); + return FALSE; + } + for (i=0; i