Pack/unpack double/float in Little Endian and Big Endian

[sskaje]

The pack/unpack's parameter 'd' and 'f' are machine order in PHP.

The number 236.0 packed to 'd' is: 0000000000806d40.

I was reversing a game save file, which uses 406D800000000000.
Here is how I decode that in PHP.

        public function readDouble()
        {
                $data = substr($this->stream, $this->ptr, 8);
                $this->ptr += 8;

                $data = implode('', array_reverse(str_split($data, 1)));

                $int = unpack('d', $data);
                return (double) ($int[1]);
        }

Since integers are allowed to pack/unpack in both little endian and big endian, I think double/float also need.

In this PR, I choose:
e: little endian double
E: big endian double
g: little endian float
G: big endian float.

BTW, these formats, 'e', 'E', 'g', and 'G' are not taken in Perl.

[bwoebi]

I think this addition is pretty much non-controversial and may be merged as-is. [i.e. without RFC]

@sskaje Have you noticed the tests failing with: ext/standard/tests/strings/unpack_error.phpt being the failing test? At least here on travis? May you please fix that, thanks!

[sskaje]

@bwoebi done this time.

[hikari-no-yume]

This feature gap seemed quite conspicuous when I was writing TypedArrays, so I'm glad to see an attempt to fill it.

[laruence]

you could reuse "WORDS_BIGENDIAN" macro.

[hikari-no-yume]

Hmm, do float and integer endianness always match?

[bwoebi]

If they would not match, you could not use bit-tricks on floats with integers... (which we anyway do)

[laruence]

and it's better also added some notes in UPGRADE, thanks

[sskaje]

Out for beer, I'll fix those later.

2016年5月28日星期六，Xinchen Hui notifications@github.com 写道：

and it's better also added some notes in UPGRADE, thanks

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1905 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAMwNh0ITstc3vizYjpVd7j1HxTusN18ks5qF_0MgaJpZM4IdEOI
.

sskaje@gmail.com
https://sskaje.me/

[sskaje]

@laruence tests passed on debian jessie x86_64(little endian) and mips32(big endian) + gcc, and osx 10.11 + llvm.

[krakjoe]

I'm not satisfied that this does not have BC implications, since you had to change an existing test.

Can I ask that you start an internals discussion to try to gather more consensus on this change please.

Please link to internals discussion here so it may be tracked.

UPGRADING has merge conflict that should be fixed at the last moment. If consensus emerges from discussion, fix this conflict and ping me directly, I will merge it quickly before more conflicts can arise.

[sskaje]

@krakjoe You mean I have to provide PR for every active branch ?

btw, I'll fix UPGRADING later

[krakjoe]

No, a PR to master is fine, and is all that is possible right now because of BC.

[nikic]

@krakjoe The existing test only checks the error message is an unknown modifier is used, I don't think there's a BC concern there.

[krakjoe]

Yep, you're right

[sskaje]

UPGRADING changed and conflict resolved.

[krakjoe]

Merged 7267d00

Thanks.

[nikic]

Linkage fixed in b3889d4.