added "keeprefcount" parameter to Value::detach()

4 files changed, 22 insertions, 23 deletions

diff --git a/include/value.h b/include/value.h
index 807ad42..532d0c1 100644
--- a/include/value.h
+++ b/include/value.h
@@ -1149,9 +1149,10 @@ protected:
      *  return a zval pointer, that would otherwise be deallocated the moment
      *  the function returns.
      *
+     *  @param  keeprefcount    Keep the same refcount
      *  @return zval
      */
-    struct _zval_struct *detach();
+    struct _zval_struct *detach(bool keeprefcount = true);
 
     /**
      *  Set a certain property without running any checks (you must already know
diff --git a/zend/classimpl.cpp b/zend/classimpl.cpp
index 71b85e5..ad7dab4 100644
--- a/zend/classimpl.cpp
+++ b/zend/classimpl.cpp
@@ -467,19 +467,16 @@ int ClassImpl::cast(zval *val, zval *retval, int type TSRMLS_DC)
 
         // check type
         switch ((Type)type) {
-            case Type::Numeric:     result = meta->callToInteger(object).detach();  break;
-            case Type::Float:       result = meta->callToFloat(object).detach();    break;
-            case Type::Bool:        result = meta->callToBool(object).detach();     break;
-            case Type::String:      result = meta->callToString(object).detach();   break;
-            default:                throw NotImplemented();                         break;
+            case Type::Numeric:     result = meta->callToInteger(object).detach(true);  break;
+            case Type::Float:       result = meta->callToFloat(object).detach(true);    break;
+            case Type::Bool:        result = meta->callToBool(object).detach(true);     break;
+            case Type::String:      result = meta->callToString(object).detach(true);   break;
+            default:                throw NotImplemented();                             break;
         }
 
         // @todo do we turn into endless conversion if the __toString object returns 'this' ??
         // (and if it does: who cares? If the extension programmer is stupid, why do we have to suffer?)
         
-        // increment refcount for "result" (we keep a reference here)
-        Z_ADDREF_P(result);
-
         // is the original parameter overwritten?
         if (val == retval) zval_dtor(val);
 
@@ -653,7 +650,7 @@ zval *ClassImpl::readDimension(zval *object, zval *offset, int type TSRMLS_DC)
             process(exception TSRMLS_CC);
 
             // unreachable
-            return Value(nullptr).detach();
+            return Value(nullptr).detach(false);
         }
     }
     else
@@ -811,17 +808,17 @@ zval *ClassImpl::toZval(Value &&value, int type)
     // because we do not want the value object to destruct the zval when
     // it falls out of scope, we detach the zval from it, if this is a regular
     // read operation we can do this right away
-    if (type == 0) return value.detach();
+    if (type == 0) return value.detach(false);
 
     // this is a more complicated read operation, the scripts wants to get
     // deeper access to the returned value. This, however, is only possible
     // if the value has more than once reference (if it has a refcount of one,
     // the value object that we have here is the only instance of the zval,
     // and it is simply impossible to return a reference or so
-    if (value.refcount() <= 1) return value.detach();
+    if (value.refcount() <= 1) return value.detach(false);
 
     // we're dealing with an editable zval, return a reference variable
-    return Value(value.detach(), true).detach();
+    return Value(value.detach(false), true).detach(false);
 }
 
 /**
@@ -907,7 +904,7 @@ zval *ClassImpl::readProperty(zval *object, zval *name, int type, const zend_lit
         process(exception TSRMLS_CC);
 
         // unreachable
-        return Value(nullptr).detach();
+        return Value(nullptr).detach(false);
     }
 }
 
diff --git a/zend/iteratorimpl.cpp b/zend/iteratorimpl.cpp
index 3bd2ed7..49526b1 100644
--- a/zend/iteratorimpl.cpp
+++ b/zend/iteratorimpl.cpp
@@ -81,10 +81,7 @@ void IteratorImpl::key(zend_object_iterator *iter, zval *key TSRMLS_DC)
     Value retval(self(iter)->key());
 
     // detach the underlying zval
-    zval *val = retval.detach();
-
-    // increment number of references of the zval (we keep a copy too)
-    Z_ADDREF_P(val);
+    zval *val = retval.detach(true);
 
     // copy it to the key
     ZVAL_ZVAL(key, val, 1, 1);
diff --git a/zend/value.cpp b/zend/value.cpp
index e2d18f1..d22830a 100644
--- a/zend/value.cpp
+++ b/zend/value.cpp
@@ -317,10 +317,11 @@ Value::~Value()
  *  deallocate the zval structure. This is used for functions that have to
  *  return a zval pointer, that would otherwise be deallocated the moment
  *  the function returns.
- *
+ * 
+ *  @param  keeprefcount
  *  @return zval
  */
-zval *Value::detach()
+zval *Value::detach(bool keeprefcount)
 {
     // leap out if already detached
     if (!_val) return nullptr;
@@ -328,11 +329,14 @@ zval *Value::detach()
     // copy return value
     zval *result = _val;
 
-    // decrement reference counter
-    Z_DELREF_P(_val);
-
     // reset internal object
     _val = nullptr;
+    
+    // we're ready if we should keep the refcounter
+    if (keeprefcount) return result;
+
+    // decrement reference counter
+    Z_DELREF_P(result);
 
     // done
     return result;