From 9f6aceaa8ce8250d9e36225180c218035bd49fe9 Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Tue, 12 Dec 2017 12:06:51 -0800 Subject: [PATCH 1/4] Add PROTOBUF_ENABLE_TIMESTAMP to let user decide whether timestamp util can be used at install time. --- php/ext/google/protobuf/message.c | 13 ++++++++++++- php/tests/test.sh | 2 +- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/php/ext/google/protobuf/message.c b/php/ext/google/protobuf/message.c index b14c1f0c5a..70080a3b6e 100644 --- a/php/ext/google/protobuf/message.c +++ b/php/ext/google/protobuf/message.c @@ -29,9 +29,12 @@ // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include -#include #include +#ifdef PROTOBUF_ENABLE_TIMESTAMP +#include +#endif + #include "protobuf.h" #include "utf8.h" @@ -1121,6 +1124,7 @@ PHP_PROTO_FIELD_ACCESSORS(Timestamp, timestamp, Seconds, "seconds") PHP_PROTO_FIELD_ACCESSORS(Timestamp, timestamp, Nanos, "nanos") PHP_METHOD(Timestamp, fromDateTime) { +#ifdef PROTOBUF_ENABLE_TIMESTAMP zval* datetime; zval member; @@ -1149,9 +1153,13 @@ PHP_METHOD(Timestamp, fromDateTime) { storage = message_data(self); memory = slot_memory(self->descriptor->layout, storage, field); *(int32_t*)memory = 0; +#else + zend_error(E_USER_ERROR, "fromDateTime needs date extension."); +#endif } PHP_METHOD(Timestamp, toDateTime) { +#ifdef PROTOBUF_ENABLE_TIMESTAMP zval datetime; php_date_instantiate(php_date_get_date_ce(), &datetime TSRMLS_CC); php_date_obj* dateobj = UNBOX(php_date_obj, &datetime); @@ -1184,6 +1192,9 @@ PHP_METHOD(Timestamp, toDateTime) { zval* datetime_ptr = &datetime; PHP_PROTO_RETVAL_ZVAL(datetime_ptr); +#else + zend_error(E_USER_ERROR, "toDateTime needs date extension."); +#endif } // ----------------------------------------------------------------------------- From 88102eae8f86045307e9d46ad900f91158227f2b Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Tue, 12 Dec 2017 13:57:49 -0800 Subject: [PATCH 2/4] Replace private timelib_update_ts with public date_timestamp_get --- php/ext/google/protobuf/message.c | 24 +++++++++++++++++++----- php/tests/memory_leak_test.php | 12 ++++++++++++ 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/php/ext/google/protobuf/message.c b/php/ext/google/protobuf/message.c index 70080a3b6e..5b654a78d6 100644 --- a/php/ext/google/protobuf/message.c +++ b/php/ext/google/protobuf/message.c @@ -1133,12 +1133,24 @@ PHP_METHOD(Timestamp, fromDateTime) { return; } - php_date_obj* dateobj = UNBOX(php_date_obj, datetime); - if (!dateobj->time->sse_uptodate) { - timelib_update_ts(dateobj->time, NULL); + // Get timestamp from Datetime object. + zval* retval_ptr; + zval* function_name; + int64_t timestamp; + + MAKE_STD_ZVAL(retval_ptr); + MAKE_STD_ZVAL(function_name); + + ZVAL_STRING(function_name, "date_timestamp_get", 1); + + if (call_user_function(EG(function_table), NULL, + function_name, retval_ptr, 1, + &datetime TSRMLS_CC) == SUCCESS) { + protobuf_convert_to_int64(retval_ptr, ×tamp); } - int64_t timestamp = dateobj->time->sse; + zval_ptr_dtor(&retval_ptr); + zval_ptr_dtor(&function_name); // Set seconds MessageHeader* self = UNBOX(MessageHeader, getThis()); @@ -1146,13 +1158,15 @@ PHP_METHOD(Timestamp, fromDateTime) { upb_msgdef_ntofz(self->descriptor->msgdef, "seconds"); void* storage = message_data(self); void* memory = slot_memory(self->descriptor->layout, storage, field); - *(int64_t*)memory = dateobj->time->sse; + *(int64_t*)memory = timestamp; // Set nanos field = upb_msgdef_ntofz(self->descriptor->msgdef, "nanos"); storage = message_data(self); memory = slot_memory(self->descriptor->layout, storage, field); *(int32_t*)memory = 0; + + RETURN_NULL(); #else zend_error(E_USER_ERROR, "fromDateTime needs date extension."); #endif From fffe8d39f810d147c6db65f90ae4f71f4e0f0116 Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Tue, 12 Dec 2017 17:47:04 -0800 Subject: [PATCH 3/4] Call php method via function name instead of calling directly. This changes the linking error if php extension is not statically linked to a runtime error. In this way, users who don't need Timestamp can still use protobuf even if date extension is not statically linked in php. --- php/ext/google/protobuf/message.c | 70 ++++++++++++++++++++++---------------- php/ext/google/protobuf/protobuf.h | 4 +++ php/tests/memory_leak_test.php | 8 ++--- php/tests/test.sh | 2 +- 4 files changed, 50 insertions(+), 34 deletions(-) diff --git a/php/ext/google/protobuf/message.c b/php/ext/google/protobuf/message.c index 5b654a78d6..7d7d86517a 100644 --- a/php/ext/google/protobuf/message.c +++ b/php/ext/google/protobuf/message.c @@ -31,10 +31,6 @@ #include #include -#ifdef PROTOBUF_ENABLE_TIMESTAMP -#include -#endif - #include "protobuf.h" #include "utf8.h" @@ -1124,7 +1120,6 @@ PHP_PROTO_FIELD_ACCESSORS(Timestamp, timestamp, Seconds, "seconds") PHP_PROTO_FIELD_ACCESSORS(Timestamp, timestamp, Nanos, "nanos") PHP_METHOD(Timestamp, fromDateTime) { -#ifdef PROTOBUF_ENABLE_TIMESTAMP zval* datetime; zval member; @@ -1134,23 +1129,27 @@ PHP_METHOD(Timestamp, fromDateTime) { } // Get timestamp from Datetime object. - zval* retval_ptr; - zval* function_name; + zval retval; + zval function_name; int64_t timestamp; - MAKE_STD_ZVAL(retval_ptr); - MAKE_STD_ZVAL(function_name); +#if PHP_MAJOR_VERSION < 7 + INIT_ZVAL(retval); + INIT_ZVAL(function_name); +#endif - ZVAL_STRING(function_name, "date_timestamp_get", 1); + PHP_PROTO_ZVAL_STRING(&function_name, "date_timestamp_get", 1); - if (call_user_function(EG(function_table), NULL, - function_name, retval_ptr, 1, - &datetime TSRMLS_CC) == SUCCESS) { - protobuf_convert_to_int64(retval_ptr, ×tamp); + if (call_user_function(EG(function_table), NULL, &function_name, &retval, 1, + ZVAL_PTR_TO_CACHED_PTR(datetime) TSRMLS_CC) == FAILURE) { + zend_error(E_ERROR, "Cannot get timestamp from DateTime."); + return; } - zval_ptr_dtor(&retval_ptr); - zval_ptr_dtor(&function_name); + protobuf_convert_to_int64(&retval, ×tamp); + + zval_dtor(&retval); + zval_dtor(&function_name); // Set seconds MessageHeader* self = UNBOX(MessageHeader, getThis()); @@ -1167,17 +1166,9 @@ PHP_METHOD(Timestamp, fromDateTime) { *(int32_t*)memory = 0; RETURN_NULL(); -#else - zend_error(E_USER_ERROR, "fromDateTime needs date extension."); -#endif } PHP_METHOD(Timestamp, toDateTime) { -#ifdef PROTOBUF_ENABLE_TIMESTAMP - zval datetime; - php_date_instantiate(php_date_get_date_ce(), &datetime TSRMLS_CC); - php_date_obj* dateobj = UNBOX(php_date_obj, &datetime); - // Get seconds MessageHeader* self = UNBOX(MessageHeader, getThis()); const upb_fielddef* field = @@ -1198,16 +1189,37 @@ PHP_METHOD(Timestamp, toDateTime) { strftime(formated_time, sizeof(formated_time), "%Y-%m-%dT%H:%M:%SUTC", utc_time); - if (!php_date_initialize(dateobj, formated_time, strlen(formated_time), NULL, - NULL, 0 TSRMLS_CC)) { - zval_dtor(&datetime); - RETURN_NULL(); + // Create Datetime object. + zval datetime; + zval formated_time_php; + zval function_name; + int64_t timestamp = 0; + +#if PHP_MAJOR_VERSION < 7 + INIT_ZVAL(function_name); + INIT_ZVAL(formated_time_php); +#endif + + PHP_PROTO_ZVAL_STRING(&function_name, "date_create", 1); + PHP_PROTO_ZVAL_STRING(&formated_time_php, formated_time, 1); + + CACHED_VALUE params[1] = {ZVAL_TO_CACHED_VALUE(formated_time_php)}; + + if (call_user_function(EG(function_table), NULL, + &function_name, &datetime, 1, + params TSRMLS_CC) == FAILURE) { + zend_error(E_ERROR, "Cannot create DateTime."); + return; } + zval_dtor(&formated_time_php); + zval_dtor(&function_name); + +#if PHP_MAJOR_VERSION < 7 zval* datetime_ptr = &datetime; PHP_PROTO_RETVAL_ZVAL(datetime_ptr); #else - zend_error(E_USER_ERROR, "toDateTime needs date extension."); + ZVAL_OBJ(return_value, Z_OBJ(datetime)); #endif } diff --git a/php/ext/google/protobuf/protobuf.h b/php/ext/google/protobuf/protobuf.h index f299b4150d..6ab0f13478 100644 --- a/php/ext/google/protobuf/protobuf.h +++ b/php/ext/google/protobuf/protobuf.h @@ -182,6 +182,8 @@ #define CACHED_TO_ZVAL_PTR(VALUE) (VALUE) #define CACHED_PTR_TO_ZVAL_PTR(VALUE) (*VALUE) #define ZVAL_PTR_TO_CACHED_PTR(VALUE) (&VALUE) +#define ZVAL_PTR_TO_CACHED_VALUE(VALUE) (VALUE) +#define ZVAL_TO_CACHED_VALUE(VALUE) (&VALUE) #define CREATE_OBJ_ON_ALLOCATED_ZVAL_PTR(zval_ptr, class_type) \ ZVAL_OBJ(zval_ptr, class_type->create_object(class_type TSRMLS_CC)); @@ -452,6 +454,8 @@ static inline int php_proto_zend_hash_get_current_data_ex(HashTable* ht, #define CACHED_TO_ZVAL_PTR(VALUE) (&VALUE) #define CACHED_PTR_TO_ZVAL_PTR(VALUE) (VALUE) #define ZVAL_PTR_TO_CACHED_PTR(VALUE) (VALUE) +#define ZVAL_PTR_TO_CACHED_VALUE(VALUE) (*VALUE) +#define ZVAL_TO_CACHED_VALUE(VALUE) (VALUE) #define CREATE_OBJ_ON_ALLOCATED_ZVAL_PTR(zval_ptr, class_type) \ ZVAL_OBJ(zval_ptr, class_type->create_object(class_type)); From 1a549d9a902151e980bfa76093b3d82b7589e158 Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Wed, 13 Dec 2017 17:09:55 -0800 Subject: [PATCH 4/4] Avoid using php_date_get_date_ce() in case date extension is not available. --- php/ext/google/protobuf/message.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/php/ext/google/protobuf/message.c b/php/ext/google/protobuf/message.c index 7d7d86517a..df5eb408a1 100644 --- a/php/ext/google/protobuf/message.c +++ b/php/ext/google/protobuf/message.c @@ -1123,8 +1123,21 @@ PHP_METHOD(Timestamp, fromDateTime) { zval* datetime; zval member; - if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "O", &datetime, - php_date_get_date_ce()) == FAILURE) { + if (zend_parse_parameters( + ZEND_NUM_ARGS() TSRMLS_CC, "z", &datetime) == FAILURE) { + return; + } + + zend_class_entry* ce = Z_OBJCE_P(datetime); + PHP_PROTO_CE_DECLARE datetime_ce; + if (php_proto_zend_lookup_class("\\Datetime", 9, &datetime_ce) == + FAILURE) { + zend_error(E_ERROR, "Make sure date extension is enabled."); + return; + } + + if (!instanceof_function(PHP_PROTO_CE_UNREF(datetime_ce), ce TSRMLS_CC)) { + zend_error(E_USER_ERROR, "Expect Datetime."); return; }