Remove 1000 char xattr limit #76
Conversation
We need to access xattrs much bigger than 1000, up to the size allowed by the filesystem.
|
@crrodriguez please have a look at the failing tests - to my understanding the tests should still all be green. |
|
Please try this code instead: https://github.com/eduardok/libsmbclient-php/tree/edtests |
This patch is the same thing I had before, but avoided posting it do the different paths needed for old PHP versions, should do exacmle the same thing. |
Not sure what you mean by exactly the same since this pull request's code (b8137f6) is quite different from the one I pointed you to (565d0af). It uses no ALLOCA_FLAG/do_alloca, handles different PHP versions, and passes CI (https://travis-ci.org/github/eduardok/libsmbclient-php/builds/748486644). |
Sorry if I was unclear, I mean an early version of this request that isn't online was very similar to yours. |
| if(retsize < 0) | ||
| goto fail; | ||
|
|
||
| RETVAL_STRING(values); |
There was a problem hiding this comment.
This will break support for PHP 5, see deleted line above (and previous comments about duplication)
| RETURN_EMPTY_STRING(); | ||
| } | ||
|
|
||
| values = do_alloca(xattr_size + 1, use_heap); |
There was a problem hiding this comment.
why not a simple emalloc ?
BTW, for PHP 7+, will be better to use a zend_string, which will avoid double allocation (RETURN_STR will use the allocated string, while RETURN_STRING allocate a new one)
(and for PHP 5, RETURN_STRING have a duplicate flag to avoid double allocation)
|
|
||
| values = do_alloca(xattr_size + 1, use_heap); | ||
|
|
||
| retsize = smbc_getxattr(state->ctx, url, name, values, sizeof(values)); |
There was a problem hiding this comment.
sizeof(values) is only the pointer size...
|
PR #96 seems better |
|
Pulled Remi's request, please use release 1.0.7. |
Any idea of what size is needed ? |
64k |
|
@crrodriguez thanks (test/feedback on PR #100 welcome) |
We need to access xattrs much bigger than 1000, up to the size
allowed by the filesystem.