diff options
author | Emiel Bruijntjes <emiel.bruijntjes@copernica.com> | 2013-09-10 05:21:18 -0700 |
---|---|---|
committer | Emiel Bruijntjes <emiel.bruijntjes@copernica.com> | 2013-09-10 05:21:18 -0700 |
commit | 85507088051bdfabf8bc71346290949b78c3c914 (patch) | |
tree | d3503b8d28ccac78c5b209bea97a094b4a54aeb7 | |
parent | e220af8dc07d845efb81082f3159460406ece9ca (diff) |
Fixed various crashes because hidden pointers were not persistently stored
Copying the result value of a function has been fixed
New C++ nullptr type is supported for Php::Value
-rw-r--r-- | include/extension.h | 12 | ||||
-rw-r--r-- | include/function.h | 73 | ||||
-rw-r--r-- | include/hiddenpointer.h | 56 | ||||
-rw-r--r-- | include/value.h | 5 | ||||
-rw-r--r-- | phpcpp.h | 1 | ||||
-rw-r--r-- | src/extension.cpp | 34 | ||||
-rw-r--r-- | src/function.cpp | 20 | ||||
-rw-r--r-- | src/includes.h | 1 | ||||
-rw-r--r-- | src/nativefunction.h | 21 | ||||
-rw-r--r-- | src/value.cpp | 50 | ||||
-rw-r--r-- | tests/simple/simple.cpp | 19 | ||||
-rw-r--r-- | tests/simple/simple.php | 1 |
12 files changed, 207 insertions, 86 deletions
diff --git a/include/extension.h b/include/extension.h index ad00593..fb33c99 100644 --- a/include/extension.h +++ b/include/extension.h @@ -166,11 +166,15 @@ public: * It is only possible to create functions during the initialization of * the library, before the Extension::module() method is called. * - * @param name Name of the function + * Note that the function must have been allocated on the HEAP (using + * "new") and that the object will be destructed (using "delete") + * by the extension object (you thus do not have to destruct it + * yourself!) + * * @param function The function to add * @return Function The added function */ - Function &add(const char *name, const Function &function); + Function &add(Function *function); /** * Add a native function directly to the extension @@ -200,10 +204,10 @@ public: private: /** - * Map of function objects defined in the library + * Set of function objects defined in the library * @var map */ - std::map<const char *,Function> _functions; + std::set<std::unique_ptr<Function>> _functions; /** * Hidden pointer to self diff --git a/include/function.h b/include/function.h index daf8572..c2dd9b8 100644 --- a/include/function.h +++ b/include/function.h @@ -26,24 +26,13 @@ namespace Php { class Function { public: - -// Function(std::function<Value()> &function); -// Function(std::function<Value(Value&)> &function); -// Function(std::function<Value(Value&,Value&)> &function); -// Function(std::function<Value(Value&,Value&,Value&)> &function); -// Function(std::function<Value(Value&,Value&,Value&,Value&)> &function); -// Function(std::function<void()> &function); -// Function(std::function<void(Value&)> &function); -// Function(std::function<void(Value&,Value&)> &function); -// Function(std::function<void(Value&,Value&,Value&)> &function); -// Function(std::function<void(Value&,Value&,Value&,Value&)> &function); - /** * Constructor + * @param name Name of the function * @param min Min number of arguments * @param max Max number of arguments */ - Function(int min = 0, int max = 0) + Function(const char *name, int min = 0, int max = 0) : _ptr(this, name) { // construct the arguments _arguments = std::shared_ptr<Arguments>(new Arguments(min, max)); @@ -53,7 +42,7 @@ public: * No copy constructor * @param function The other function */ - Function(const Function &function) + Function(const Function &function) : _ptr(this, function.name()) { // copy members _arguments = function._arguments; @@ -64,7 +53,7 @@ public: * Move constructor * @param function The other function */ - Function(Function &&function) + Function(Function &&function) : _ptr(this, function.name()) { // copy arguments _arguments = function._arguments; @@ -90,12 +79,52 @@ public: if (this == &function) return *this; // copy members + _ptr.setText(function.name()); _arguments = function._arguments; _type = function._type; // done return *this; } + + /** + * Comparison + * @param function The other function + * @return bool + */ + bool operator<(const Function &function) const + { + return strcmp(name(), function.name()) < 0; + } + + /** + * Comparison + * @param function The other function + * @return bool + */ + bool operator>(const Function &function) const + { + return strcmp(name(), function.name()) > 0; + } + + /** + * Comparison + * @param function The other function + * @return bool + */ + bool operator==(const Function &function) const + { + return strcmp(name(), function.name()) == 0; + } + + /** + * Function name + * @return const char * + */ + const char *name() const + { + return _ptr.text(); + } /** * Method that gets called every time the function is executed @@ -105,7 +134,7 @@ public: */ virtual Value invoke(Request &request, Parameters ¶ms) { - return 0; + return nullptr; } protected: @@ -121,20 +150,24 @@ protected: */ std::shared_ptr<Arguments> _arguments; + /** + * The name is stored in a hidden pointer, so that we have access to the function + * @var HiddenPointer + */ + HiddenPointer<Function> _ptr; + private: /** * Fill a function entry - * @param name Name of the function * @param entry Entry to be filled */ - void fill(const char *name, struct _zend_function_entry *entry); + void fill(struct _zend_function_entry *entry) const; /** * Fill function info - * @param name Name of the function * @param info Info object to be filled */ - void fill(const char *name, struct _zend_internal_function_info *info); + void fill(struct _zend_internal_function_info *info) const; /** * Extension has access to the private members diff --git a/include/hiddenpointer.h b/include/hiddenpointer.h index f29082f..3502bc9 100644 --- a/include/hiddenpointer.h +++ b/include/hiddenpointer.h @@ -106,25 +106,64 @@ public: * Retrieve the pointer * @return Type* */ - Type *pointer() + Type *pointer() const { return _pointer; } /** + * Change the pointer + * @param Type* + */ + void setPointer(Type *pointer) + { + // store pointer + _pointer = pointer; + + // overwrite in data + _data.replace(0, sizeof(Type *), (const char *)&_pointer, sizeof(Type *)); + + // for safety reasons, we recalculate text pointer + _text = _data.c_str() + sizeof(Type *); + } + + /** * Retrieve the text * @return const char * */ - const char *text() + const char *text() const { return _text; } /** + * Change the text + * @param text + * @param size + */ + void setText(const char *text, int size=-1) + { + // check if size was set + if (size < 0) size = strlen(text); + + // reserve enough room for the text and the pointer + _data.reserve(size + sizeof(Type *)); + + // store the pointer + _data.assign(std::string((const char *)&_pointer, sizeof(Type *))); + + // append the text + _data.append(text, size); + + // store new text + _text = _data.c_str() + sizeof(Type *); + } + + /** * Cast to the pointer * @return Type* */ - operator Type* () + operator Type* () const { return _pointer; } @@ -133,10 +172,19 @@ public: * Cast to text * @return const char * */ - operator const char * () + operator const char * () const { return _text; } + + /** + * Length of the text + * @return int + */ + int length() const + { + return _data.size() - sizeof(Type *); + } private: /** diff --git a/include/value.h b/include/value.h index 64f92df..e4af1b7 100644 --- a/include/value.h +++ b/include/value.h @@ -45,6 +45,11 @@ public: Value(); /** + * Constructor for null ptr + */ + Value(std::nullptr_t value); + + /** * Constructor based on integer value * @param value */ @@ -16,6 +16,7 @@ #include <vector> #include <map> #include <memory> +#include <set> /** * Include all headers files that are related to this library diff --git a/src/extension.cpp b/src/extension.cpp index 1fd4b65..a084763 100644 --- a/src/extension.cpp +++ b/src/extension.cpp @@ -108,8 +108,6 @@ static int extension_startup(INIT_FUNC_ARGS) */ static int extension_shutdown(SHUTDOWN_FUNC_ARGS) { - std::cout << "extension_shutdown" << std::endl; - // @todo the get_extension() call crashes, we need a different way to find the extension return 0; @@ -137,8 +135,6 @@ static int request_startup(INIT_FUNC_ARGS) */ static int request_shutdown(INIT_FUNC_ARGS) { - std::cout << "request_shutdown" << std::endl; - // @todo the get_extension() call crashes, we need a different way to find the extension return 0; @@ -200,8 +196,6 @@ Extension::Extension(const char *name, const char *version) : _ptr(this, name) */ Extension::~Extension() { - std::cout << "destruct extension" << std::endl; - // deallocate functions if (_entry->functions) delete[] _entry->functions; @@ -211,14 +205,16 @@ Extension::~Extension() /** * Add a function to the library - * @param name Function name * @param function Function object * @return Function */ -Function &Extension::add(const char *name, const Function &function) +Function &Extension::add(Function *function) { // add the function to the map - return _functions[name] = function; + _functions.insert(std::unique_ptr<Function>(function)); + + // the result is a pair with an iterator + return *function; } /** @@ -227,14 +223,14 @@ Function &Extension::add(const char *name, const Function &function) * @param function The function to add * @return Function The added function */ -Function &Extension::add(const char *name, native_callback_0 function) { add(name, NativeFunction(function)); } -Function &Extension::add(const char *name, native_callback_1 function) { add(name, NativeFunction(function)); } -Function &Extension::add(const char *name, native_callback_2 function) { add(name, NativeFunction(function)); } -Function &Extension::add(const char *name, native_callback_3 function) { add(name, NativeFunction(function)); } -Function &Extension::add(const char *name, native_callback_4 function) { add(name, NativeFunction(function)); } -Function &Extension::add(const char *name, native_callback_5 function) { add(name, NativeFunction(function)); } -Function &Extension::add(const char *name, native_callback_6 function) { add(name, NativeFunction(function)); } -Function &Extension::add(const char *name, native_callback_7 function) { add(name, NativeFunction(function)); } +Function &Extension::add(const char *name, native_callback_0 function) { return add(new NativeFunction(name, function)); } +Function &Extension::add(const char *name, native_callback_1 function) { return add(new NativeFunction(name, function)); } +Function &Extension::add(const char *name, native_callback_2 function) { return add(new NativeFunction(name, function)); } +Function &Extension::add(const char *name, native_callback_3 function) { return add(new NativeFunction(name, function)); } +Function &Extension::add(const char *name, native_callback_4 function) { return add(new NativeFunction(name, function)); } +Function &Extension::add(const char *name, native_callback_5 function) { return add(new NativeFunction(name, function)); } +Function &Extension::add(const char *name, native_callback_6 function) { return add(new NativeFunction(name, function)); } +Function &Extension::add(const char *name, native_callback_7 function) { return add(new NativeFunction(name, function)); } /** * Retrieve the module entry @@ -255,10 +251,10 @@ zend_module_entry *Extension::module() for (auto it = begin(_functions); it != _functions.end(); it++) { // retrieve entry - zend_function_entry *entry = &functions[i]; + zend_function_entry *entry = &functions[i++]; // let the function fill the entry - it->second.fill(it->first, entry); + (*it)->fill(entry); } // last entry should be set to all zeros diff --git a/src/function.cpp b/src/function.cpp index 13d4d27..f15bffd 100644 --- a/src/function.cpp +++ b/src/function.cpp @@ -32,7 +32,7 @@ void invoke_function(INTERNAL_FUNCTION_PARAMETERS) Function *function = HiddenPointer<Function>(name); // wrap the return value - Value ret(return_value, true); + Value result(return_value, true); // construct parameters Parameters params(ZEND_NUM_ARGS()); @@ -40,8 +40,8 @@ void invoke_function(INTERNAL_FUNCTION_PARAMETERS) // @todo get the appropriate request (or environment) Request request(NULL); - // call the appropriate object - ret = function->invoke(request, params); + // get the result + result = function->invoke(request, params); } /** @@ -50,13 +50,12 @@ void invoke_function(INTERNAL_FUNCTION_PARAMETERS) * This method is called when the extension is registering itself, when the * function or method introces himself * - * @param name Name of the function * @param entry Entry to be filled */ -void Function::fill(const char *name, zend_function_entry *entry) +void Function::fill(zend_function_entry *entry) const { // fill the members of the entity, and hide a pointer to the current object in the name - entry->fname = HiddenPointer<Function>(this, name); + entry->fname = _ptr; entry->handler = invoke_function; entry->arg_info = _arguments->internal(); entry->num_args = _arguments->argc(); @@ -65,21 +64,20 @@ void Function::fill(const char *name, zend_function_entry *entry) entry->flags = 0; // we should fill the first argument as well - fill(name, (zend_internal_function_info *)entry->arg_info); + fill((zend_internal_function_info *)entry->arg_info); } /** * Fill a function entry - * @param name Name of the function * @param info Info to be filled */ -void Function::fill(const char *name, zend_internal_function_info *info) +void Function::fill(zend_internal_function_info *info) const { // fill in all the members, note that return reference is false by default, // because we do want to return references, inside the name we hide a pointer // to the current object - info->_name = HiddenPointer<Function>(this, name); - info->_name_len = strlen(name); + info->_name = _ptr; + info->_name_len = _ptr.length(); info->_class_name = NULL; // number of required arguments, and the expected return type diff --git a/src/includes.h b/src/includes.h index 8d83752..3c1f26f 100644 --- a/src/includes.h +++ b/src/includes.h @@ -16,6 +16,7 @@ #include <vector> #include <map> #include <memory> +#include <set> // for debugging #include <iostream> diff --git a/src/nativefunction.h b/src/nativefunction.h index 7f36608..40b8539 100644 --- a/src/nativefunction.h +++ b/src/nativefunction.h @@ -21,16 +21,17 @@ class NativeFunction : public Function public: /** * Constructor - * @param function + * @param name Function name + * @param function The native C function */ - NativeFunction(native_callback_0 function) : _type(0) { _function.f0 = function; } - NativeFunction(native_callback_1 function) : _type(1) { _function.f1 = function; } - NativeFunction(native_callback_2 function) : _type(2) { _function.f2 = function; } - NativeFunction(native_callback_3 function) : _type(3) { _function.f3 = function; } - NativeFunction(native_callback_4 function) : _type(4) { _function.f4 = function; } - NativeFunction(native_callback_5 function) : _type(5) { _function.f5 = function; } - NativeFunction(native_callback_6 function) : _type(6) { _function.f6 = function; } - NativeFunction(native_callback_7 function) : _type(7) { _function.f7 = function; } + NativeFunction(const char *name, native_callback_0 function) : Function(name), _type(0) { _function.f0 = function; _ptr.setPointer(this); } + NativeFunction(const char *name, native_callback_1 function) : Function(name), _type(1) { _function.f1 = function; _ptr.setPointer(this); } + NativeFunction(const char *name, native_callback_2 function) : Function(name), _type(2) { _function.f2 = function; _ptr.setPointer(this); } + NativeFunction(const char *name, native_callback_3 function) : Function(name), _type(3) { _function.f3 = function; _ptr.setPointer(this); } + NativeFunction(const char *name, native_callback_4 function) : Function(name), _type(4) { _function.f4 = function; _ptr.setPointer(this); } + NativeFunction(const char *name, native_callback_5 function) : Function(name), _type(5) { _function.f5 = function; _ptr.setPointer(this); } + NativeFunction(const char *name, native_callback_6 function) : Function(name), _type(6) { _function.f6 = function; _ptr.setPointer(this); } + NativeFunction(const char *name, native_callback_7 function) : Function(name), _type(7) { _function.f7 = function; _ptr.setPointer(this); } /** * Destructor @@ -43,7 +44,7 @@ public: * @param params The parameters that were passed * @return Variable Return value */ - virtual Value invoke(Request &request, Parameters ¶ms) + virtual Value invoke(Request &request, Parameters ¶ms) override { switch (_type) { case 0: _function.f0(); return Value(); diff --git a/src/value.cpp b/src/value.cpp index f386822..7f43724 100644 --- a/src/value.cpp +++ b/src/value.cpp @@ -42,6 +42,16 @@ Value::Value() } /** + * Constructor for null ptr + */ +Value::Value(std::nullptr_t value) +{ + // create a null zval + MAKE_STD_ZVAL(_val); + ZVAL_NULL(_val); +} + +/** * Constructor based on integer value * @param value */ @@ -197,15 +207,37 @@ Value &Value::operator=(const Value &value) // skip self assignment if (this == &value) return *this; - // destruct the zval (this function will decrement the reference counter, - // and only destruct if there are no other references left) - zval_ptr_dtor(&_val); - - // just copy the zval, and the refcounter - _val = value._val; - - // and we have one more reference - Z_ADDREF_P(_val); + // is the object a reference? + if (Z_ISREF_P(_val)) + { + // the current object is a reference, this means that we should + // keep the zval object, and copy the other value into it, get + // the current refcount + int refcount = Z_REFCOUNT_P(_val); + + // clean up the current zval (but keep the zval structure) + zval_dtor(_val); + + // make the copy + *_val = *value._val; + zval_copy_ctor(_val); + + // restore refcount and reference setting + Z_SET_ISREF_TO_P(_val, true); + Z_SET_REFCOUNT_P(_val, refcount); + } + else + { + // destruct the zval (this function will decrement the reference counter, + // and only destruct if there are no other references left) + zval_ptr_dtor(&_val); + + // just copy the zval, and the refcounter + _val = value._val; + + // and we have one more reference + Z_ADDREF_P(_val); + } // done return *this; diff --git a/tests/simple/simple.cpp b/tests/simple/simple.cpp index 311ce4c..954bfa9 100644 --- a/tests/simple/simple.cpp +++ b/tests/simple/simple.cpp @@ -17,14 +17,23 @@ using namespace std; static Php::Value my_plus(Php::Parameters ¶ms) { + cout << "my_plus called" << endl; + + cout << "params: " << params.size() << endl; + string p1 = params[0]; string p2 = params[1]; + cout << "p1: " << p1 << endl; + cout << "p2: " << p2 << endl; + return p1 + p2; } class MyCustomFunction : public Php::Function { +public: + MyCustomFunction(const char *name) : Function(name) {} }; @@ -35,20 +44,14 @@ extern "C" // export the "get_module" function that will be called by the Zend engine PHPCPP_EXPORT void *get_module() { - cout << "a" << endl; - // create extension static Php::Extension extension("simple","1.0"); - cout << "b" << endl; - // define the functions extension.add("my_plus", my_plus); -// extension.add("my_concat", my_concat); - extension.add("my_custom", MyCustomFunction()); + extension.add("my_concat", my_concat); + extension.add(new MyCustomFunction("my_custom")); - cout << "c" << endl; - // define classes // extension.add("my_class", MyCustomClass()); diff --git a/tests/simple/simple.php b/tests/simple/simple.php index fc552fd..a6bf7f5 100644 --- a/tests/simple/simple.php +++ b/tests/simple/simple.php @@ -18,4 +18,3 @@ echo("resultaat: $result\n"); print_r($result); -?> |