[libvirt] [libvirt-php][PATCH 00/11] Resolve reverse include order

These are not pushed yet as they might be somewhat controversial. I'll wait if somebody wants to review them. The ultimate goal is to, well drop libvirt-php.h completely. It is needless. But before going there we must distribute the interesting parts from it around. Therefore I'm introducing some modules (like it should have been done from the start). Anyway, happy coding! Michal Privoznik (11): vncfunc: Honour static function Move DEFAULT_LOG_MAXSIZE to libvirt-php.c Move VNC_MAX_AUTH_ATTEMPTS to vncfunc.c libvirt-php.c: Move system headers together src: Introduce util.h src: Introduce util.c src: Introduce vncfunc.h src: Introduce sockets.h vncfunc: Drop include of libvirt-php.h sockets: Drop include of libvirt-php.h Clean up debug macros configure.ac | 1 + src/Makefile.am | 5 ++-- src/config.m4 | 2 +- src/libvirt-php.c | 69 +++++++++++++++++---------------------------- src/libvirt-php.h | 81 ----------------------------------------------------- src/sockets.c | 24 +++++++++++----- src/sockets.h | 32 +++++++++++++++++++++ src/util.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util.h | 73 ++++++++++++++++++++++++++++++++++++++++++++++++ src/vncfunc.c | 83 +++++++++++++++++++++++++++++++++++++++++-------------- src/vncfunc.h | 38 +++++++++++++++++++++++++ 11 files changed, 325 insertions(+), 156 deletions(-) create mode 100644 src/sockets.h create mode 100644 src/util.c create mode 100644 src/util.h create mode 100644 src/vncfunc.h -- 2.8.4

In order to bring some order to the source code, lets start with marking static functions as static. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/vncfunc.c | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/src/vncfunc.c b/src/vncfunc.c index 70eaa8b..17cf050 100644 --- a/src/vncfunc.c +++ b/src/vncfunc.c @@ -48,7 +48,8 @@ typedef struct tServerFBParams { * Arguments: @sfd [int]: socket descriptor connected to the VNC server * Returns: 0 on success, -errno on error */ -int vnc_write_client_version(int sfd) +static int +vnc_write_client_version(int sfd) { unsigned char buf[12]; @@ -84,7 +85,8 @@ int vnc_write_client_version(int sfd) * Arguments: @sfd [int]: socket descriptor connected to the VNC server * Returns: 0 on success, -errno on error (incl. -EIO on invalid authorization) */ -int vnc_authorize(int sfd) +static int +vnc_authorize(int sfd) { unsigned char buf[4] = { 0 }; unsigned char buf2[32] = { 0 }; @@ -158,7 +160,8 @@ int vnc_authorize(int sfd) * @len [int]: length of the buffer * Returns: parameters structure of tServerFBParams */ -tServerFBParams vnc_parse_fb_params(unsigned char *buf, int len) +static tServerFBParams +vnc_parse_fb_params(unsigned char *buf, int len) { int nlen, little_endian = 0; int w1, w2, h1, h2, h, w; @@ -222,7 +225,8 @@ tServerFBParams vnc_parse_fb_params(unsigned char *buf, int len) * @release [bool]: flag to release the key immediately * Returns: 0 on success, -errno otherwise */ -int vnc_send_key(int sfd, unsigned char key, int modifier, int release) +static int +vnc_send_key(int sfd, unsigned char key, int modifier, int release) { unsigned char buf[8]; @@ -262,7 +266,8 @@ int vnc_send_key(int sfd, unsigned char key, int modifier, int release) * @pos_y [int]: Y position of mouse cursor * Returns: 0 on success, -errno otherwise */ -int vnc_send_client_pointer(int sfd, int clicked, int pos_x, int pos_y) +static int +vnc_send_client_pointer(int sfd, int clicked, int pos_x, int pos_y) { unsigned char buf[6] = { 0 }; @@ -299,7 +304,8 @@ int vnc_send_client_pointer(int sfd, int clicked, int pos_x, int pos_y) * @params [struct]: structure of parameters to set the data * Returns: 0 on success, -errno otherwise */ -int vnc_set_pixel_format(int sfd, tServerFBParams params) +static int +vnc_set_pixel_format(int sfd, tServerFBParams params) { unsigned char buf[20]; @@ -355,7 +361,8 @@ int vnc_set_pixel_format(int sfd, tServerFBParams params) * Arguments: @sfd [int]: socket descriptor for existing VNC client socket * Returns: 0 on success, -errno otherwise */ -int vnc_set_encoding(int sfd) +static int +vnc_set_encoding(int sfd) { unsigned char buf[8]; @@ -400,7 +407,8 @@ int vnc_set_encoding(int sfd) * @h [int]: height of frame * Returns: 0 on success, -errno otherwise */ -int vnc_send_framebuffer_update(int sfd, int incrementalUpdate, int x, int y, int w, int h) +static int +vnc_send_framebuffer_update(int sfd, int incrementalUpdate, int x, int y, int w, int h) { unsigned char buf[10]; @@ -440,7 +448,8 @@ int vnc_send_framebuffer_update(int sfd, int incrementalUpdate, int x, int y, in * @params [struct]: structure of parameters to request the update for full screen * Returns: 0 on success, -errno otherwise */ -int vnc_send_framebuffer_update_request(int sfd, int incrementalUpdate, tServerFBParams params) +static int +vnc_send_framebuffer_update_request(int sfd, int incrementalUpdate, tServerFBParams params) { return vnc_send_framebuffer_update(sfd, incrementalUpdate, 0, 0, params.width, params.height); } @@ -454,7 +463,8 @@ int vnc_send_framebuffer_update_request(int sfd, int incrementalUpdate, tServerF * @share [bool]: flag whether to share desktop or not * Returns: socket descriptor on success, -errno otherwise */ -int vnc_connect(char *server, char *port, int share) +static int +vnc_connect(char *server, char *port, int share) { int sfd, err; unsigned char buf[1024] = { 0 }; @@ -498,7 +508,8 @@ int vnc_connect(char *server, char *port, int share) * Arguments: @sfd [int]: socket file descriptor acquired by vnc_connect() call * Returns: parameters block */ -tServerFBParams vnc_read_server_init(int sfd) +static tServerFBParams +vnc_read_server_init(int sfd) { unsigned char *buf = NULL; unsigned char tmpbuf[25] = { 0 }; @@ -597,7 +608,8 @@ int vnc_get_dimensions(char *server, char *port, int *width, int *height) * @height [int]: height of the output image (for bitmap headers) * Returns: 0 on success, -errno otherwise */ -int vnc_raw_to_bmp(char *infile, char *outfile, int width, int height) +static int +vnc_raw_to_bmp(char *infile, char *outfile, int width, int height) { int i, ix, fd, fd2; tBMPFile fBMP = { 0 }; -- 2.8.4

There is no need to have this macro in the header file. It's used in one place after all. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt-php.c | 3 +++ src/libvirt-php.h | 3 --- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libvirt-php.c b/src/libvirt-php.c index 693f911..951375f 100644 --- a/src/libvirt-php.c +++ b/src/libvirt-php.c @@ -1097,6 +1097,9 @@ int has_builtin(char *name) return 0; } +/* Maximum size (in KiB) of log file when DEBUG_SUPPORT is enabled */ +#define DEFAULT_LOG_MAXSIZE 1024 + /* Information function for phpinfo() */ PHP_MINFO_FUNCTION(libvirt) { diff --git a/src/libvirt-php.h b/src/libvirt-php.h index a429ed8..0cd160f 100644 --- a/src/libvirt-php.h +++ b/src/libvirt-php.h @@ -29,9 +29,6 @@ /* Maximum number of authentication attempts */ #define VNC_MAX_AUTH_ATTEMPTS 10 -/* Maximum size (in KiB) of log file when DEBUG_SUPPORT is enabled */ -#define DEFAULT_LOG_MAXSIZE 1024 - /* Network constants */ #define VIR_NETWORKS_ACTIVE 1 #define VIR_NETWORKS_INACTIVE 2 -- 2.8.4

There is no need to have this macro in the header file. It's used in one place after all. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt-php.h | 3 --- src/vncfunc.c | 3 +++ 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libvirt-php.h b/src/libvirt-php.h index 0cd160f..ce885e5 100644 --- a/src/libvirt-php.h +++ b/src/libvirt-php.h @@ -26,9 +26,6 @@ #define ARRAY_CARDINALITY(array) (sizeof(array) / sizeof(array[0])) -/* Maximum number of authentication attempts */ -#define VNC_MAX_AUTH_ATTEMPTS 10 - /* Network constants */ #define VIR_NETWORKS_ACTIVE 1 #define VIR_NETWORKS_INACTIVE 2 diff --git a/src/vncfunc.c b/src/vncfunc.c index 17cf050..6e2f220 100644 --- a/src/vncfunc.c +++ b/src/vncfunc.c @@ -78,6 +78,9 @@ vnc_write_client_version(int sfd) return 0; } +/* Maximum number of authentication attempts */ +# define VNC_MAX_AUTH_ATTEMPTS 10 + /* * Private function name: vnc_authorize * Since version: 0.4.3 -- 2.8.4

And at the same time switch from "header.h" notation to <header.h>. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- configure.ac | 1 + src/libvirt-php.c | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 8d52862..f232756 100644 --- a/configure.ac +++ b/configure.ac @@ -4,6 +4,7 @@ AC_CONFIG_HEADERS([config.h]) AC_CONFIG_MACRO_DIR([m4]) AM_INIT_AUTOMAKE([-Wall -Werror -Wno-portability]) AM_MAINTAINER_MODE([enable]) +AC_GNU_SOURCE m4_ifndef([LT_INIT], [ AM_PROG_LIBTOOL diff --git a/src/libvirt-php.c b/src/libvirt-php.c index 951375f..6b145de 100644 --- a/src/libvirt-php.c +++ b/src/libvirt-php.c @@ -18,14 +18,15 @@ #define EXTWIN #endif -#include "stdafx.h" +#include <config.h> +#include <stdio.h> +#include <stdafx.h> #ifdef EXTWIN #define PHP_COMPILER_ID "VC9" #endif #include "libvirt-php.h" -#include "stdio.h" #ifndef EXTWIN // From vncfunc.c -- 2.8.4

Move out some macros that are shared between multiple source files into a separate file called util.h. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/Makefile.am | 1 + src/libvirt-php.c | 19 +++++++++++++++++ src/libvirt-php.h | 39 ---------------------------------- src/sockets.c | 1 + src/util.h | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/vncfunc.c | 1 + 6 files changed, 85 insertions(+), 39 deletions(-) create mode 100644 src/util.h diff --git a/src/Makefile.am b/src/Makefile.am index 68b6371..39a47e9 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -18,6 +18,7 @@ LIBVIRT_PHP_SYMBOL_FILE = \ php_plugindir = $(extensiondir) php_plugin_LTLIBRARIES = libvirt-php.la libvirt_php_la_SOURCES = \ + util.h \ vncfunc.c \ sockets.c \ libvirt-php.c libvirt-php.h diff --git a/src/libvirt-php.c b/src/libvirt-php.c index 6b145de..b9256db 100644 --- a/src/libvirt-php.c +++ b/src/libvirt-php.c @@ -27,6 +27,7 @@ #endif #include "libvirt-php.h" +#include "util.h" #ifndef EXTWIN // From vncfunc.c @@ -35,6 +36,10 @@ int vnc_get_dimensions(char *server, char *port, int *width, int *height); int connect_socket(char *server, char *port, int keepalive, int nodelay, int allow_server_override); #endif +#ifdef DEBUG_SUPPORT +int gdebug; +#endif + #ifdef DEBUG_CORE #define DPRINTF(fmt, ...) \ if (LIBVIRT_G(debug)) \ @@ -794,6 +799,20 @@ zend_module_entry libvirt_module_entry = { ZEND_GET_MODULE(libvirt) #endif +ZEND_BEGIN_MODULE_GLOBALS(libvirt) + char *last_error; + char *vnc_location; + zend_bool longlong_to_string_ini; + char *iso_path_ini; + char *image_path_ini; + zend_long max_connections_ini; +#ifdef DEBUG_SUPPORT + int debug; +#endif + resource_info *binding_resources; + int binding_resources_count; +ZEND_END_MODULE_GLOBALS(libvirt) + /* PHP init options */ PHP_INI_BEGIN() STD_PHP_INI_ENTRY("libvirt.longlong_to_string", "1", PHP_INI_ALL, OnUpdateBool, longlong_to_string_ini, zend_libvirt_globals, libvirt_globals) diff --git a/src/libvirt-php.h b/src/libvirt-php.h index ce885e5..39bd520 100644 --- a/src/libvirt-php.h +++ b/src/libvirt-php.h @@ -17,14 +17,6 @@ #ifndef PHP_LIBVIRT_H #define PHP_LIBVIRT_H 1 -#define DEBUG_SUPPORT - -#ifdef DEBUG_SUPPORT -#define DEBUG_CORE -#define DEBUG_VNC -#endif - -#define ARRAY_CARDINALITY(array) (sizeof(array) / sizeof(array[0])) /* Network constants */ #define VIR_NETWORKS_ACTIVE 1 @@ -151,20 +143,6 @@ typedef struct tTokenizer { int numTokens; } tTokenizer; -#define IS_BIGENDIAN (*(uint16_t *)"\0\xff" < 0x100) - -#define SWAP2_BY_ENDIAN(le, v1, v2) (((le && IS_BIGENDIAN) || (!le && !IS_BIGENDIAN)) ? ((v2 << 8) + v1) : ((v1 << 8) + v2)) -#define PUT2_BYTE_ENDIAN(le, val, v1, v2) { if ((le && IS_BIGENDIAN) || (!le && !IS_BIGENDIAN)) { v2 = val >> 8; v1 = val % 256; } else { v1 = val >> 8; v2 = val % 256; } } -#define SWAP2_BYTES_ENDIAN(le, a, b) { if ((le && IS_BIGENDIAN) || (!le && !IS_BIGENDIAN)) { uint8_t _tmpval; _tmpval = a; a = b; b = _tmpval; } } - -#define UINT32STR(var, val) \ - var[0] = (val >> 24) & 0xff; \ - var[1] = (val >> 16) & 0xff; \ - var[2] = (val >> 8) & 0xff; \ - var[3] = (val ) & 0xff; - -#define GETUINT32(var) (uint32_t)(((uint32_t)var[0] << 24) + ((uint32_t)var[1] << 16) + ((uint32_t)var[2] << 8) + ((uint32_t)var[3])) - typedef struct _resource_info { int type; virConnectPtr conn; @@ -172,20 +150,6 @@ typedef struct _resource_info { int overwrite; } resource_info; -ZEND_BEGIN_MODULE_GLOBALS(libvirt) - char *last_error; - char *vnc_location; - zend_bool longlong_to_string_ini; - char *iso_path_ini; - char *image_path_ini; - zend_long max_connections_ini; -#ifdef DEBUG_SUPPORT - int debug; -#endif - resource_info *binding_resources; - int binding_resources_count; -ZEND_END_MODULE_GLOBALS(libvirt) - #ifdef ZTS #define LIBVIRT_G(v) TSRMG(libvirt_globals_id, zend_libvirt_globals *, v) #else @@ -324,9 +288,6 @@ int set_logfile(char *filename, long maxsize TSRMLS_DC); char *get_datetime(void); char *get_string_from_xpath(char *xml, char *xpath, zval **val, int *retVal); char **get_array_from_xpath(char *xml, char *xpath, int *num); -#ifdef DEBUG_SUPPORT -int gdebug; -#endif #define PHP_LIBVIRT_CONNECTION_RES_NAME "Libvirt connection" #define PHP_LIBVIRT_DOMAIN_RES_NAME "Libvirt domain" diff --git a/src/sockets.c b/src/sockets.c index 71ceb7b..92ea373 100644 --- a/src/sockets.c +++ b/src/sockets.c @@ -8,6 +8,7 @@ */ #include "libvirt-php.h" +#include "util.h" #ifdef DEBUG_SOCKETS #define DPRINTF(fmt, ...) \ diff --git a/src/util.h b/src/util.h new file mode 100644 index 0000000..119d573 --- /dev/null +++ b/src/util.h @@ -0,0 +1,63 @@ +/* + * util.h: common, generic utility functions + * + * See COPYING for the license of this software + * + * Written by: + * Michal Privoznik <mprivozn@redhat.com> + */ + +#ifndef __UTIL_H__ +# define __UTIL_H__ + +# include <stdint.h> + +# define DEBUG_SUPPORT + +# ifdef DEBUG_SUPPORT +# define DEBUG_CORE +# define DEBUG_VNC +extern int gdebug; +# endif + +# define ARRAY_CARDINALITY(array) (sizeof(array) / sizeof(array[0])) + +# define IS_BIGENDIAN (*(uint16_t *)"\0\xff" < 0x100) + +# define SWAP2_BY_ENDIAN(le, v1, v2) \ + (((le && IS_BIGENDIAN) || (!le && !IS_BIGENDIAN)) ? ((v2 << 8) + v1) : ((v1 << 8) + v2)) + +# define PUT2_BYTE_ENDIAN(le, val, v1, v2) \ + do { \ + if ((le && IS_BIGENDIAN) || (!le && !IS_BIGENDIAN)) { \ + v2 = val >> 8; \ + v1 = val % 256; \ + } else { \ + v1 = val >> 8; \ + v2 = val % 256; \ + } \ + } while (0) + +# define SWAP2_BYTES_ENDIAN(le, a, b) \ + do { \ + if ((le && IS_BIGENDIAN) || (!le && !IS_BIGENDIAN)) { \ + uint8_t _tmpval; \ + _tmpval = a; \ + a = b; \ + b = _tmpval; \ + } \ + } while (0) + +# define UINT32STR(var, val) \ + var[0] = (val >> 24) & 0xff; \ + var[1] = (val >> 16) & 0xff; \ + var[2] = (val >> 8) & 0xff; \ + var[3] = (val ) & 0xff; + +# define GETUINT32(var) \ + (uint32_t)(((uint32_t)var[0] << 24) + \ + ((uint32_t)var[1] << 16) + \ + ((uint32_t)var[2] << 8) + \ + ((uint32_t)var[3])) + +#endif /* __UTIL_H__ */ diff --git a/src/vncfunc.c b/src/vncfunc.c index 6e2f220..393624f 100644 --- a/src/vncfunc.c +++ b/src/vncfunc.c @@ -8,6 +8,7 @@ */ #include "libvirt-php.h" +#include "util.h" #ifdef DEBUG_VNC #define DPRINTF(fmt, ...) \ -- 2.8.4

On 27.09.2016 15:11, Michal Privoznik wrote:
Move out some macros that are shared between multiple source files into a separate file called util.h.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/Makefile.am | 1 + src/libvirt-php.c | 19 +++++++++++++++++ src/libvirt-php.h | 39 ---------------------------------- src/sockets.c | 1 + src/util.h | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/vncfunc.c | 1 + 6 files changed, 85 insertions(+), 39 deletions(-) create mode 100644 src/util.h
diff --git a/src/libvirt-php.c b/src/libvirt-php.c index 6b145de..b9256db 100644 --- a/src/libvirt-php.c +++ b/src/libvirt-php.c
@@ -794,6 +799,20 @@ zend_module_entry libvirt_module_entry = { ZEND_GET_MODULE(libvirt) #endif
+ZEND_BEGIN_MODULE_GLOBALS(libvirt) + char *last_error; + char *vnc_location; + zend_bool longlong_to_string_ini; + char *iso_path_ini; + char *image_path_ini; + zend_long max_connections_ini; +#ifdef DEBUG_SUPPORT + int debug; +#endif + resource_info *binding_resources; + int binding_resources_count; +ZEND_END_MODULE_GLOBALS(libvirt) +
D'oh. In some compilers, forward declaration is not supported so this breaks. Therefore I am squashing this in: diff --git c/src/libvirt-php.c i/src/libvirt-php.c index b9256db..9218f71 100644 --- c/src/libvirt-php.c +++ i/src/libvirt-php.c @@ -159,8 +159,6 @@ int le_libvirt_nodedev; int le_libvirt_stream; int le_libvirt_snapshot; -ZEND_DECLARE_MODULE_GLOBALS(libvirt) - ZEND_BEGIN_ARG_INFO_EX(arginfo_libvirt_connect, 0, 0, 0) ZEND_ARG_INFO(0, url) ZEND_ARG_INFO(0, readonly) @@ -813,6 +811,8 @@ ZEND_BEGIN_MODULE_GLOBALS(libvirt) int binding_resources_count; ZEND_END_MODULE_GLOBALS(libvirt) +ZEND_DECLARE_MODULE_GLOBALS(libvirt) + /* PHP init options */ PHP_INI_BEGIN() STD_PHP_INI_ENTRY("libvirt.longlong_to_string", "1", PHP_INI_ALL, OnUpdateBool, longlong_to_string_ini, zend_libvirt_globals, libvirt_globals) Michal

So far there is just one function shared across our code: get_datetime(). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/Makefile.am | 2 +- src/config.m4 | 2 +- src/libvirt-php.c | 26 -------------------------- src/util.c | 41 +++++++++++++++++++++++++++++++++++++++++ src/util.h | 2 ++ 5 files changed, 45 insertions(+), 28 deletions(-) create mode 100644 src/util.c diff --git a/src/Makefile.am b/src/Makefile.am index 39a47e9..a12e729 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -18,7 +18,7 @@ LIBVIRT_PHP_SYMBOL_FILE = \ php_plugindir = $(extensiondir) php_plugin_LTLIBRARIES = libvirt-php.la libvirt_php_la_SOURCES = \ - util.h \ + util.c util.h \ vncfunc.c \ sockets.c \ libvirt-php.c libvirt-php.h diff --git a/src/config.m4 b/src/config.m4 index fd76286..ad8adb4 100644 --- a/src/config.m4 +++ b/src/config.m4 @@ -45,7 +45,7 @@ if test "$PHP_LIBVIRT" != "no"; then PHP_EVAL_LIBLINE($LIBXML_LIBRARY, LIBVIRT_SHARED_LIBADD) PHP_SUBST(LIBVIRT_SHARED_LIBADD) - PHP_NEW_EXTENSION(libvirt, libvirt-php.c sockets.c vncfunc.c, $ext_shared) + PHP_NEW_EXTENSION(libvirt, libvirt-php.c sockets.c vncfunc.c util.c, $ext_shared) else AC_MSG_ERROR(pkg-config not found) fi diff --git a/src/libvirt-php.c b/src/libvirt-php.c index b9256db..70405f2 100644 --- a/src/libvirt-php.c +++ b/src/libvirt-php.c @@ -862,32 +862,6 @@ PHP_RSHUTDOWN_FUNCTION(libvirt) } /* - * Private function name: get_datetime - * Since version: 0.4.2 - * Description: Function can be used to get date and time in the `YYYY-mm-dd HH:mm:ss` format, internally used for logging when debug logging is enabled using libvirt_set_logfile() API function. - * Arguments: None - * Returns: Date/time string in `YYYY-mm-dd HH:mm:ss` format - */ -char *get_datetime(void) -{ - /* Caution: Function cannot use DPRINTF() macro otherwise the neverending loop will be met! */ - char *outstr = NULL; - time_t t; - struct tm *tmp; - - t = time(NULL); - tmp = localtime(&t); - if (tmp == NULL) - return NULL; - - outstr = (char *)malloc(32 * sizeof(char)); - if (strftime(outstr, 32, "%Y-%m-%d %H:%M:%S", tmp) == 0) - return NULL; - - return outstr; -} - -/* * Private function name: set_logfile * Since version: 0.4.2 * Description: Function to set the log file. You can set log file to NULL to disable logging (default). Useful for debugging purposes. diff --git a/src/util.c b/src/util.c new file mode 100644 index 0000000..7d2b457 --- /dev/null +++ b/src/util.c @@ -0,0 +1,41 @@ +/* + * util.c: common, generic utility functions + * + * See COPYING for the license of this software + * + * Written by: + * Michal Privoznik <mprivozn@redhat.com> + */ + +#include <config.h> + +#include <stdlib.h> +#include <time.h> + +#include "util.h" + +/* + * Private function name: get_datetime + * Since version: 0.4.2 + * Description: Function can be used to get date and time in the `YYYY-mm-dd HH:mm:ss` format, internally used for logging when debug logging is enabled using libvirt_set_logfile() API function. + * Arguments: None + * Returns: Date/time string in `YYYY-mm-dd HH:mm:ss` format + */ +char *get_datetime(void) +{ + /* Caution: Function cannot use DPRINTF() macro otherwise the neverending loop will be met! */ + char *outstr = NULL; + time_t t; + struct tm *tmp; + + t = time(NULL); + tmp = localtime(&t); + if (tmp == NULL) + return NULL; + + outstr = (char *)malloc(32 * sizeof(char)); + if (strftime(outstr, 32, "%Y-%m-%d %H:%M:%S", tmp) == 0) + return NULL; + + return outstr; +} diff --git a/src/util.h b/src/util.h index 119d573..e907a49 100644 --- a/src/util.h +++ b/src/util.h @@ -60,4 +60,6 @@ extern int gdebug; ((uint32_t)var[2] << 8) + \ ((uint32_t)var[3])) +char *get_datetime(void); + #endif /* __UTIL_H__ */ -- 2.8.4

Instead of forward declaration of some vnc_* functions from vncfunc.c lets have a separate header file for that purpose. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/Makefile.am | 2 +- src/libvirt-php.c | 3 +-- src/libvirt-php.h | 5 ----- src/vncfunc.c | 1 + src/vncfunc.h | 38 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 41 insertions(+), 8 deletions(-) create mode 100644 src/vncfunc.h diff --git a/src/Makefile.am b/src/Makefile.am index a12e729..5720ddd 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -19,7 +19,7 @@ php_plugindir = $(extensiondir) php_plugin_LTLIBRARIES = libvirt-php.la libvirt_php_la_SOURCES = \ util.c util.h \ - vncfunc.c \ + vncfunc.c vncfunc.h \ sockets.c \ libvirt-php.c libvirt-php.h libvirt_php_la_CFLAGS = \ diff --git a/src/libvirt-php.c b/src/libvirt-php.c index 70405f2..01e9b57 100644 --- a/src/libvirt-php.c +++ b/src/libvirt-php.c @@ -28,10 +28,9 @@ #include "libvirt-php.h" #include "util.h" +#include "vncfunc.h" #ifndef EXTWIN -// From vncfunc.c -int vnc_get_dimensions(char *server, char *port, int *width, int *height); // From sockets.c int connect_socket(char *server, char *port, int keepalive, int nodelay, int allow_server_override); #endif diff --git a/src/libvirt-php.h b/src/libvirt-php.h index 39bd520..1225c0b 100644 --- a/src/libvirt-php.h +++ b/src/libvirt-php.h @@ -136,7 +136,6 @@ int connect_socket(char *server, char *port, int keepalive, int nodelay, int all int socket_has_data(int sfd, long maxtime, int ignoremsg); void socket_read(int sfd, long length); int socket_read_and_save(int sfd, char *fn, long length); -int vnc_get_bitmap(char *server, char *port, char *fn); typedef struct tTokenizer { char **tokens; @@ -280,10 +279,6 @@ typedef struct _php_libvirt_hash_key_info { } php_libvirt_hash_key_info; /* Private definitions */ -int vnc_refresh_screen(char *server, char *port, int scancode); -int vnc_send_keys(char *server, char *port, char *keys); -int vnc_send_pointer_event(char *server, char *port, int pos_x, int pos_y, int clicked, int release); - int set_logfile(char *filename, long maxsize TSRMLS_DC); char *get_datetime(void); char *get_string_from_xpath(char *xml, char *xpath, zval **val, int *retVal); diff --git a/src/vncfunc.c b/src/vncfunc.c index 393624f..b1a994d 100644 --- a/src/vncfunc.c +++ b/src/vncfunc.c @@ -7,6 +7,7 @@ * Michal Novotny <minovotn@redhat.com> */ +#include "vncfunc.h" #include "libvirt-php.h" #include "util.h" diff --git a/src/vncfunc.h b/src/vncfunc.h new file mode 100644 index 0000000..4758f95 --- /dev/null +++ b/src/vncfunc.h @@ -0,0 +1,38 @@ +/* + * vncfunc.h: VNC Client functions to be used for the graphical VNC console of libvirt-php + * + * See COPYING for the license of this software + * + * Written by: + * Michal Novotny <minovotn@redhat.com> + * Michal Privoznik <mprivozn@redhat.com> + */ + +#ifndef __VNCFUNC_H__ +# define __VNCFUNC_H__ + +int vnc_get_bitmap(char *server, + char *port, + char *fn); + +int vnc_get_dimensions(char *server, + char *port, + int *width, + int *height); + +int vnc_refresh_screen(char *server, + char *port, + int scancode); + +int vnc_send_keys(char *server, + char *port, + char *keys); + +int vnc_send_pointer_event(char *server, + char *port, + int pos_x, + int pos_y, + int clicked, + int release); + +#endif /* __VNCFUNC_H__ */ -- 2.8.4

Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/Makefile.am | 2 +- src/libvirt-php.c | 6 +----- src/libvirt-php.h | 5 ----- src/sockets.c | 1 + src/sockets.h | 32 ++++++++++++++++++++++++++++++++ src/vncfunc.c | 1 + 6 files changed, 36 insertions(+), 11 deletions(-) create mode 100644 src/sockets.h diff --git a/src/Makefile.am b/src/Makefile.am index 5720ddd..bbee667 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -20,7 +20,7 @@ php_plugin_LTLIBRARIES = libvirt-php.la libvirt_php_la_SOURCES = \ util.c util.h \ vncfunc.c vncfunc.h \ - sockets.c \ + sockets.c sockets.h \ libvirt-php.c libvirt-php.h libvirt_php_la_CFLAGS = \ $(AM_CFLAGS) \ diff --git a/src/libvirt-php.c b/src/libvirt-php.c index 01e9b57..faa7200 100644 --- a/src/libvirt-php.c +++ b/src/libvirt-php.c @@ -29,11 +29,7 @@ #include "libvirt-php.h" #include "util.h" #include "vncfunc.h" - -#ifndef EXTWIN -// From sockets.c -int connect_socket(char *server, char *port, int keepalive, int nodelay, int allow_server_override); -#endif +#include "sockets.h" #ifdef DEBUG_SUPPORT int gdebug; diff --git a/src/libvirt-php.h b/src/libvirt-php.h index 1225c0b..6b6df21 100644 --- a/src/libvirt-php.h +++ b/src/libvirt-php.h @@ -132,11 +132,6 @@ typedef long zend_long; typedef unsigned long zend_ulong; #endif /* PHP_MAJOR_VERSION < 7 */ -int connect_socket(char *server, char *port, int keepalive, int nodelay, int allow_server_override); -int socket_has_data(int sfd, long maxtime, int ignoremsg); -void socket_read(int sfd, long length); -int socket_read_and_save(int sfd, char *fn, long length); - typedef struct tTokenizer { char **tokens; int numTokens; diff --git a/src/sockets.c b/src/sockets.c index 92ea373..a175450 100644 --- a/src/sockets.c +++ b/src/sockets.c @@ -7,6 +7,7 @@ * Michal Novotny <minovotn@redhat.com> */ +#include "sockets.h" #include "libvirt-php.h" #include "util.h" diff --git a/src/sockets.h b/src/sockets.h new file mode 100644 index 0000000..52b5866 --- /dev/null +++ b/src/sockets.h @@ -0,0 +1,32 @@ +/* + * sockets.h: Socket functions for libvirt-php + * + * See COPYING for the license of this software + * + * Written by: + * Michal Novotny <minovotn@redhat.com> + * Michal Privoznik <mprivozn@redhat.com> + */ + +#ifndef __SOCKETS_H__ +# define __SOCKETS_H__ + +int connect_socket(char *server, + char *port, + int keepalive, + int nodelay, + int allow_server_override); + +int socket_has_data(int sfd, + long maxtime, + int ignoremsg); + +void socket_read(int sfd, + long length); + +int socket_read_and_save(int sfd, + char *fn, + long length); + +#endif /* __SOCKETS_H__ */ + diff --git a/src/vncfunc.c b/src/vncfunc.c index b1a994d..447881e 100644 --- a/src/vncfunc.c +++ b/src/vncfunc.c @@ -10,6 +10,7 @@ #include "vncfunc.h" #include "libvirt-php.h" #include "util.h" +#include "sockets.h" #ifdef DEBUG_VNC #define DPRINTF(fmt, ...) \ -- 2.8.4

The point is, libvirt-php.c uses vnc functions implemented in vncfunc.c, but for some weird historical reasons, it was vncfunc.c who included libvirt-php.h. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt-php.h | 21 --------------------- src/vncfunc.c | 32 +++++++++++++++++++++++++++++++- 2 files changed, 31 insertions(+), 22 deletions(-) diff --git a/src/libvirt-php.h b/src/libvirt-php.h index 6b6df21..6b34581 100644 --- a/src/libvirt-php.h +++ b/src/libvirt-php.h @@ -194,27 +194,6 @@ typedef struct tVMNetwork { char *model; } tVMNetwork; -#ifndef EXTWIN -typedef struct tBMPFile { - uint32_t filesz; - uint16_t creator1; - uint16_t creator2; - uint32_t bmp_offset; - - uint32_t header_sz; - int32_t height; - int32_t width; - uint16_t nplanes; - uint16_t bitspp; - uint32_t compress_type; - uint32_t bmp_bytesz; - int32_t hres; - int32_t vres; - uint32_t ncolors; - uint32_t nimpcolors; -} tBMPFile; -#endif - /* Libvirt-php types */ typedef struct _php_libvirt_connection { virConnectPtr conn; diff --git a/src/vncfunc.c b/src/vncfunc.c index 447881e..cb98341 100644 --- a/src/vncfunc.c +++ b/src/vncfunc.c @@ -7,8 +7,19 @@ * Michal Novotny <minovotn@redhat.com> */ +#include <config.h> + +#include <errno.h> +#include <fcntl.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <sys/socket.h> +#include <sys/stat.h> +#include <sys/types.h> +#include <unistd.h> + #include "vncfunc.h" -#include "libvirt-php.h" #include "util.h" #include "sockets.h" @@ -44,6 +55,25 @@ typedef struct tServerFBParams { unsigned char *desktopName; } tServerFBParams; +typedef struct tBMPFile { + uint32_t filesz; + uint16_t creator1; + uint16_t creator2; + uint32_t bmp_offset; + + uint32_t header_sz; + int32_t height; + int32_t width; + uint16_t nplanes; + uint16_t bitspp; + uint32_t compress_type; + uint32_t bmp_bytesz; + int32_t hres; + int32_t vres; + uint32_t ncolors; + uint32_t nimpcolors; +} tBMPFile; + /* * Private function name: vnc_write_client_version * Since version: 0.4.3 -- 2.8.4

Just like in the previous commit, socket is offering its functions to the rest of the code and therefore it should not include libvirt-php.h. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt-php.h | 5 ----- src/sockets.c | 17 ++++++++++++++++- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/libvirt-php.h b/src/libvirt-php.h index 6b34581..9978e27 100644 --- a/src/libvirt-php.h +++ b/src/libvirt-php.h @@ -84,11 +84,6 @@ #include <stdint.h> #include <libgen.h> -#ifdef __APPLE__ -#include <netinet/tcp.h> -#else -#include <linux/tcp.h> -#endif #else #define PRIx32 "I32x" diff --git a/src/sockets.c b/src/sockets.c index a175450..6620e17 100644 --- a/src/sockets.c +++ b/src/sockets.c @@ -7,8 +7,23 @@ * Michal Novotny <minovotn@redhat.com> */ +#include <config.h> + +#include <errno.h> +#include <fcntl.h> +#include <netdb.h> +#include <string.h> +#include <sys/socket.h> +#include <sys/stat.h> +#include <sys/types.h> +#include <unistd.h> +#ifdef __APPLE__ +#include <netinet/tcp.h> +#else +#include <linux/tcp.h> +#endif + #include "sockets.h" -#include "libvirt-php.h" #include "util.h" #ifdef DEBUG_SOCKETS -- 2.8.4

Each single file in our source code redefines the same DPRINTF macro. What we want instead is to have the macro shared and also we might to avoid global variable shared across all the code. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt-php.c | 15 ++------------- src/sockets.c | 9 +-------- src/util.c | 34 +++++++++++++++++++++++++++++++++- src/util.h | 12 ++++++++++-- src/vncfunc.c | 9 +-------- 5 files changed, 47 insertions(+), 32 deletions(-) diff --git a/src/libvirt-php.c b/src/libvirt-php.c index faa7200..e129481 100644 --- a/src/libvirt-php.c +++ b/src/libvirt-php.c @@ -31,18 +31,7 @@ #include "vncfunc.h" #include "sockets.h" -#ifdef DEBUG_SUPPORT -int gdebug; -#endif - -#ifdef DEBUG_CORE -#define DPRINTF(fmt, ...) \ -if (LIBVIRT_G(debug)) \ -do { fprintf(stderr, "[%s ", get_datetime()); fprintf(stderr, "libvirt-php/core ]: " fmt , ## __VA_ARGS__); fflush(stderr); } while (0) -#else -#define DPRINTF(fmt, ...) \ -do {} while(0) -#endif +DEBUG_INIT("core"); /* PHP functions are prefixed with `zif_` so strip it */ #define PHPFUNC (__FUNCTION__ + 4) @@ -819,7 +808,7 @@ PHP_INI_END() void change_debug(int val TSRMLS_DC) { LIBVIRT_G(debug) = val; - gdebug = val; + setDebug(val); } /* PHP requires to have this function defined */ diff --git a/src/sockets.c b/src/sockets.c index 6620e17..0a3e3c2 100644 --- a/src/sockets.c +++ b/src/sockets.c @@ -26,14 +26,7 @@ #include "sockets.h" #include "util.h" -#ifdef DEBUG_SOCKETS -#define DPRINTF(fmt, ...) \ -if (gdebug) \ -do { fprintf(stderr, "[%s ", get_datetime()); fprintf(stderr, "libvirt-php/sockets]: " fmt , ## __VA_ARGS__); fflush(stderr); } while (0) -#else -#define DPRINTF(fmt, ...) \ -do {} while(0) -#endif +DEBUG_INIT("sockets"); /* Function macro */ #define PHPFUNC __FUNCTION__ diff --git a/src/util.c b/src/util.c index 7d2b457..53096ae 100644 --- a/src/util.c +++ b/src/util.c @@ -9,11 +9,15 @@ #include <config.h> +#include <stdarg.h> +#include <stdio.h> #include <stdlib.h> #include <time.h> #include "util.h" +int gdebug; + /* * Private function name: get_datetime * Since version: 0.4.2 @@ -21,7 +25,8 @@ * Arguments: None * Returns: Date/time string in `YYYY-mm-dd HH:mm:ss` format */ -char *get_datetime(void) +static char * +get_datetime(void) { /* Caution: Function cannot use DPRINTF() macro otherwise the neverending loop will be met! */ char *outstr = NULL; @@ -39,3 +44,30 @@ char *get_datetime(void) return outstr; } + +void debugPrint(const char *source, + const char *fmt, ...) +{ + char *datetime; + va_list args; + + if (!gdebug) + return; + + datetime = get_datetime(); + fprintf(stderr, "[%s libvirt-php/%s ]: ", datetime, source); + free(datetime); + + if (fmt) { + va_start(args, fmt); + vfprintf(stderr, fmt, args); + va_end(args); + } + fprintf(stderr, "\n"); + fflush(stderr); +} + +void setDebug(int level) +{ + gdebug = level; +} diff --git a/src/util.h b/src/util.h index e907a49..c2b7324 100644 --- a/src/util.h +++ b/src/util.h @@ -17,9 +17,14 @@ # ifdef DEBUG_SUPPORT # define DEBUG_CORE # define DEBUG_VNC -extern int gdebug; # endif +# define DEBUG_INIT(source) \ + static const char *debugSource = "" source "" + +# define DPRINTF(fmt, ...) \ + debugPrint(debugSource, fmt, __VA_ARGS__) + # define ARRAY_CARDINALITY(array) (sizeof(array) / sizeof(array[0])) # define IS_BIGENDIAN (*(uint16_t *)"\0\xff" < 0x100) @@ -60,6 +65,9 @@ extern int gdebug; ((uint32_t)var[2] << 8) + \ ((uint32_t)var[3])) -char *get_datetime(void); +void debugPrint(const char *source, + const char *fmt, ...); + +void setDebug(int level); #endif /* __UTIL_H__ */ diff --git a/src/vncfunc.c b/src/vncfunc.c index cb98341..45f4007 100644 --- a/src/vncfunc.c +++ b/src/vncfunc.c @@ -23,14 +23,7 @@ #include "util.h" #include "sockets.h" -#ifdef DEBUG_VNC -#define DPRINTF(fmt, ...) \ -if (gdebug) \ -do { fprintf(stderr, "[%s ", get_datetime()); fprintf(stderr, "libvirt-php/vnc ]: " fmt , ## __VA_ARGS__); fflush(stderr); } while (0) -#else -#define DPRINTF(fmt, ...) \ -do {} while(0) -#endif +DEBUG_INIT("vncfunc"); /* Function macro */ #define PHPFUNC __FUNCTION__ -- 2.8.4

2016-09-27 16:11 GMT+03:00 Michal Privoznik <mprivozn@redhat.com>:
These are not pushed yet as they might be somewhat controversial. I'll wait if somebody wants to review them.
The ultimate goal is to, well drop libvirt-php.h completely. It is needless. But before going there we must distribute the interesting parts from it around. Therefore I'm introducing some modules (like it should have been done from the start).
I like this idea to move extension helper to external files. -- Vasiliy Tolstov, e-mail: v.tolstov@selfip.ru

On 28.09.2016 14:00, Vasiliy Tolstov wrote:
2016-09-27 16:11 GMT+03:00 Michal Privoznik <mprivozn@redhat.com>:
These are not pushed yet as they might be somewhat controversial. I'll wait if somebody wants to review them.
The ultimate goal is to, well drop libvirt-php.h completely. It is needless. But before going there we must distribute the interesting parts from it around. Therefore I'm introducing some modules (like it should have been done from the start).
I like this idea to move extension helper to external files.
Yeah, thank you. I've pushed the patches now. I know this wasn't an explicit ACK, but things work slightly different here compared to libvirt :-) Michal
participants (2)
-
Michal Privoznik
-
Vasiliy Tolstov