Implemented FR #55716 - Add an option to pass a custom stream context #1204
Conversation
|
|
||
| if (zend_parse_parameters(ZEND_NUM_ARGS(), "s|l", &url, &url_len, &format) == FAILURE) { | ||
| if (zend_parse_parameters(ZEND_NUM_ARGS(), "s|lr", &url, &url_len, &format, &zcontext) == FAILURE) { |
There was a problem hiding this comment.
I'd use s|lr! here to allow passing null. Otherwise the change looks great :)
There was a problem hiding this comment.
thanks, updated the PR with that.
Tyrael
force-pushed
the
get_headers_optional_context
branch
from
d2742a6 to
b9b0d3d
Compare
| php_stream *stream; | ||
| zval *prev_val, *hdr = NULL, *h; | ||
| HashTable *hashT; | ||
| zend_long format = 0; | ||
| zval *zcontext = NULL; | ||
| php_stream_context *context = NULL; |
There was a problem hiding this comment.
It's unnecessary to set it to NULL, since context will be always assigned below..
There was a problem hiding this comment.
yeah, but we do the same in file_get_contents, so I thought I would follow suit:
http://lxr.php.net/xref/PHP_TRUNK/ext/standard/file.c#527
There was a problem hiding this comment.
then it should also be improved..
There was a problem hiding this comment.
"improved", since compiler is smart enough to optimize it anyway..
|
That would have been a welcome improvement for PHP 7.0. Maybe we can get it in for PHP 7.1? :) |
|
yeah. I wasn't sure if it needs an rfc or not then I took the feature freeze seriously. |
the current way of setting the context options through stream_context_set_default is really clunky (and can cause side-effects as it modifies the global context options obviously).