Time  Nick      Message
16:45 Krock     p_gimeno: https://github.com/minetest/minetest/pull/7768#discussion_r222977729 how about the other code below? That's only checking whether our serialization function works correctly. So that means the check could simply be skipped (i.e. don't return)?
16:45 Krock     -- end of Wall Of Text --
17:16 Krock     also the writeU32 and readU32 functions already convert to the host encoding; so swapping would no longer be required?
19:00 p_gimeno  Krock: sorry, I've been AFK for most of the evening. The only one that should stay (IMO) is the test for endianness.
19:02 p_gimeno  Krock: as written, the functions compare the endianness of an integer with that of a float. In the big majority of cases, there will be equal. I remember having read somewhere in SO that they can differ in some exotic machines.
19:04 p_gimeno  ah, here: https://stackoverflow.com/questions/2945174/floating-point-endianness - second answer
19:07 p_gimeno  So the gist is, only if the endianness of an integer and of a float differ, and that of an integer is not big-endian, is there a possibility for having to do more than one swap. I don't expect that to be a concern.
19:15 p_gimeno  "However, on modern standard computers (i.e., implementing IEEE 754), one may in practice safely assume that the endianness is the same for floating-point numbers as for integers, making the conversion straightforward regardless of data type" https://en.wikipedia.org/wiki/Endianness#Floating_point
19:16 p_gimeno  indeed, the reference in Wikipedia that says there are architectures where that's not the case, only mentions PDP-11 and VAX as known cases
20:15 Krock     rare cases. not our problem
20:16 p_gimeno  I'm still wondering why to add support for non-IEEE
20:17 p_gimeno  I'd only bother if we get reports about it failing and where
20:18 p_gimeno  similarly for endianness differences between integer and float, even though I've implemented all 4 variations
20:18 Krock     yes, but special platforms, may it even just be Android are very slow in reporting issues
20:19 Krock     that means the reports roll in when the next stable is released
20:20 Krock     I'd also like to just copy&send the floats as-is with some rudimentary checking whether the floats are in the right format.. but testing such stuff is hard
20:22 p_gimeno  there's something I'm uneasy about - using a union for the conversion, that's undefined behaviour
20:24 Krock     in C++; yes.
20:24 Krock     memcpy might solve that
20:24 p_gimeno  memcpy to an array of char is well-defined in the spec, but then you have to deal with the endianness of float independently of that of integer
20:29 p_gimeno  I'm a little bit concerned about the extern global, that will probably prevent optimizing out the switch statement
20:32 p_gimeno  the inline code I wrote handled constants everywhere, so it was easily optimizable (if that word exists), but having code depend on an extern global means that another module must be able to modify that global, therefore all cases must be included
20:33 p_gimeno  I may be worrying about something unimportant, though
20:40 ssieb     I don't think there's any common platform where integer and float have a different endian.
20:40 ssieb     big endian itself is very rare now
21:45 p_gimeno  in GCC the union trick is supported and documented: https://gcc.gnu.org/onlinedocs/gcc/Structures-unions-enumerations-and-bit-fields-implementation.html (first bullet point)
21:47 p_gimeno  oops, no Krock
22:50 paramat   no problem they'll see it
22:52 paramat   i might make a PR that applies the good bits of #6880 and #7726 as those PRs are annoying and the author uncooperative. it will be faster to do this than further argument
22:52 ShadowBot https://github.com/minetest/minetest/issues/6880 -- Spelling: ": Bigversal" by comradekingu
22:52 ShadowBot https://github.com/minetest/minetest/issues/7726 -- Language rework by comradekingu
22:52 paramat   funny how the later PR went as badly as the earlier
22:54 paramat   both PRs are a mess too
23:53 paramat   merging #7461
23:53 ShadowBot https://github.com/minetest/minetest/issues/7461 -- F5 debug info: Use full words for NSEW directions for readability by paramat
23:54 paramat   merged