Check for zero SplPriorityQueue extract flags earlier

Generate an exception during the SplPriorityQueue::setExtracFlags()
call instead of generating E_RECOVERABLE_FATAL all over the place
later.

diff --git a/UPGRADING b/UPGRADING
index 2d9299f883b6033a686ea0c54bb193ccfee2a7fc..2e49c7d64108c69776a72b45186ce83dbe0ae5d2 100644 (file)
--- a/UPGRADING
+++ b/UPGRADING
@@ -46,6 +46,9 @@ PHP 7.4 UPGRADE NOTES
     (array) casts are *not* affected. They will continue to return either the
     wrapped array, or the ArrayObject properties, depending on whether the
     STD_PROP_LIST flag is used.
+  . SplPriorityQueue::setExtractFlags() will throw an exception if zero is
+    passed. Previously this would generate a recoverable fatal error on the
+    next extraction operation.
 
 ========================================
 2. New Features

diff --git a/ext/spl/spl_heap.c b/ext/spl/spl_heap.c
index 1050b732bf9502d79f0b0b4e22d310bdd7d735a6..8e644df76e3235f2b5cce73cef369fb16e03dc78 100644 (file)
--- a/ext/spl/spl_heap.c
+++ b/ext/spl/spl_heap.c
@@ -137,27 +137,27 @@ static int spl_ptr_heap_cmp_cb_helper(zval *object, spl_heap_object *heap_object
 }
 /* }}} */
 
-static int spl_pqueue_extract_helper(zval *result, zval *value, int flags) /* {{{ */
+static void spl_pqueue_extract_helper(zval *result, zval *value, int flags) /* {{{ */
 {
        spl_pqueue_elem *elem = Z_PTR_P(value);
        if ((flags & SPL_PQUEUE_EXTR_BOTH) == SPL_PQUEUE_EXTR_BOTH) {
                array_init(result);
                add_assoc_zval_ex(result, "data", sizeof("data") - 1, &elem->data);
                add_assoc_zval_ex(result, "priority", sizeof("priority") - 1, &elem->priority);
-               return SUCCESS;
+               return;
        }
 
        if (flags & SPL_PQUEUE_EXTR_DATA) {
                ZVAL_COPY(result, &elem->data);
-               return SUCCESS;
+               return;
        }
 
        if (flags & SPL_PQUEUE_EXTR_PRIORITY) {
                ZVAL_COPY(result, &elem->priority);
-               return SUCCESS;
+               return;
        }
 
-       return FAILURE;
+       ZEND_ASSERT(0);
 }
 /* }}} */
 
@@ -701,11 +701,7 @@ SPL_METHOD(SplPriorityQueue, extract)
                return;
        }
 
-       if (spl_pqueue_extract_helper(return_value, &value, intern->flags) == FAILURE) {
-               zend_error(E_RECOVERABLE_ERROR, "Unable to extract from the PriorityQueue node");
-               spl_ptr_heap_pqueue_elem_dtor(&value);
-               return;
-       }
+       spl_pqueue_extract_helper(return_value, &value, intern->flags);
 }
 /* }}} */
 
@@ -734,9 +730,7 @@ SPL_METHOD(SplPriorityQueue, top)
                return;
        }
 
-       if (spl_pqueue_extract_helper(return_value, value, intern->flags) == FAILURE) {
-               zend_error(E_RECOVERABLE_ERROR, "Unable to extract from the PriorityQueue node");
-       }
+       spl_pqueue_extract_helper(return_value, value, intern->flags);
 }
 /* }}} */
 
@@ -752,10 +746,14 @@ SPL_METHOD(SplPriorityQueue, setExtractFlags)
                return;
        }
 
-       intern = Z_SPLHEAP_P(ZEND_THIS);
-
-       intern->flags = value & SPL_PQUEUE_EXTR_MASK;
+       value &= SPL_PQUEUE_EXTR_MASK;
+       if (!value) {
+               zend_throw_exception(spl_ce_RuntimeException, "Must specify at least one extract flag", 0);
+               return;
+       }
 
+       intern = Z_SPLHEAP_P(ZEND_THIS);
+       intern->flags = value;
        RETURN_LONG(intern->flags);
 }
 /* }}} */
@@ -935,9 +933,7 @@ static zval *spl_pqueue_it_get_current_data(zend_object_iterator *iter) /* {{{ *
                return NULL;
        }
        if (Z_ISUNDEF(user_it->value)) {
-               if (spl_pqueue_extract_helper(&user_it->value, element, object->flags) == FAILURE) {
-                       zend_error(E_RECOVERABLE_ERROR, "Unable to extract from the PriorityQueue node");
-               }
+               spl_pqueue_extract_helper(&user_it->value, element, object->flags);
        }
        return &user_it->value;
 }
@@ -1057,10 +1053,7 @@ SPL_METHOD(SplPriorityQueue, current)
        if (!intern->heap->count || Z_ISUNDEF_P(element)) {
                RETURN_NULL();
        } else {
-               if (spl_pqueue_extract_helper(return_value, element, intern->flags) == FAILURE) {
-                       zend_error(E_RECOVERABLE_ERROR, "Unable to extract from the PriorityQueue node");
-                       RETURN_NULL();
-               }
+               spl_pqueue_extract_helper(return_value, element, intern->flags);
        }
 }
 /* }}} */

diff --git a/ext/spl/tests/SplPriorityQueue_setExtractFlags_zero.phpt b/ext/spl/tests/SplPriorityQueue_setExtractFlags_zero.phpt
new file mode 100644 (file)
index 0000000..cf2d339
--- /dev/null
+++ b/ext/spl/tests/SplPriorityQueue_setExtractFlags_zero.phpt
@@ -0,0 +1,15 @@
+--TEST--
+Setting SplPriorityQueue extract flags to zero generates an exception
+--FILE--
+<?php
+
+$queue = new SplPriorityQueue();
+$queue->setExtractFlags(0);
+
+?>
+--EXPECTF--
+Fatal error: Uncaught RuntimeException: Must specify at least one extract flag in %s:%d
+Stack trace:
+#0 %s(%d): SplPriorityQueue->setExtractFlags(0)
+#1 {main}
+  thrown in %s on line %d