[RFC] Add support for endianness modifier in pack()/unpack() format codes - #21074
Conversation
8d5f2e4 to
4b714a6
Compare
alexandre-daubois
changed the title
alexandre-daubois
changed the title
032ef28 to
dfb409c
Compare
|
PR updated to include the similar, second RFC. |
|
Just checking - what's the current status of this PR ? Both RFC votes passed, but it looks like this has not been merged yet. |
|
Oh wow, I really forgot about this one. Just asked for new review. Thanks! |
dfb409c to
82fbd91
Compare
9022795 to
af2d0c5
Compare
Girgias
left a comment
Girgias
left a comment
There was a problem hiding this comment.
I think splitting the 64 bit specific tests to another tests is probably a good idea.
The implementation looks reasonable to me otherwise.
af2d0c5 to
fbaaafe
Compare
fbaaafe to
4d729f8
Compare
|
Also added entires in UPGRADING and NEWS, I forgot about that |
|
Merging, if there are issues with it can always be fixed for beta3, but at least end users can test this. |
|
Thank you Gina! |
| if (formatlen > 0) { | ||
| char c = *format; | ||
|
|
||
| if (c == '<') { | ||
| endian = PHP_LITTLE_ENDIAN; | ||
| format++; | ||
| formatlen--; | ||
| } else if (c == '>') { | ||
| endian = PHP_BIG_ENDIAN; | ||
| format++; | ||
| formatlen--; | ||
| } |
There was a problem hiding this comment.
I think there is a bug when reviewing. Seems like unpack silently breaks existing key names beginning with < or >.
unpack('s<value', "\x02\x01"); // ["<value" => 258]
unpack('C>name', "\x01"); // [">name" => 1]
After this fix the first becomes ["value" => 258] as well as the second it becomes ValueError.
Or is this a feature?? Anyways, this should at least be documented as a BC break (UPGRADING) and add tests.
There was a problem hiding this comment.
I think we should document this. Since the feature is already pretty niche, I doubt that discontinuing support for names starting with a chevron will really be a problem. I'll create the PR if you agree
|
@alexandre-daubois Now this is merged, should the RFCs be moved from "Pending implementation" to "Implemented in PHP 8.6" ? |
|
Tests pass on BE, FWIW |
6 participants
RFC (integer format codes): https://wiki.php.net/rfc/pack-unpack-endianness-signed-integers-support
RFC (float format codes): https://wiki.php.net/rfc/pack-unpack-float-endianness-modifier