sqlite3: remove nonsensical return_value dtor #20360
Conversation
| if (!ZSTR_LEN(sql)) { | ||
| RETURN_FALSE; | ||
| } |
There was a problem hiding this comment.
This seems wrong? As this is a constructor and the return value is never used normally?
There was a problem hiding this comment.
You can call __construct manually, which is why this has return values
There was a problem hiding this comment.
You can yes, but the expectation is that there is no meaningful return value and you just call parent::__construct(). I don't think any other internal classes (even those that you can subclass) have those semantics.
There was a problem hiding this comment.
I think I've seen this somewhere else before too
| if (errcode != SQLITE_OK) { | ||
| php_sqlite3_error(db_obj, errcode, "Unable to prepare statement: %s", sqlite3_errmsg(db_obj->db)); | ||
| zval_ptr_dtor(return_value); | ||
| RETURN_FALSE; | ||
| } |
There was a problem hiding this comment.
Same comment as above, this is a constructor so it should throw on error rather than creating a non-usable object.
There was a problem hiding this comment.
That would break people using a combination of reflection+manual __construct calls
|
In any case I merged this as this is correct as-is. |
3 participants
No description provided.