Skip to content

Serialize long as 32 or 64 bit integer depending on platform#204

Closed
jakubpawlewicz wants to merge 1 commit into
ultimatepp:masterfrom
jakubpawlewicz:serialize_long
Closed

Serialize long as 32 or 64 bit integer depending on platform#204
jakubpawlewicz wants to merge 1 commit into
ultimatepp:masterfrom
jakubpawlewicz:serialize_long

Conversation

@jakubpawlewicz
Copy link
Copy Markdown

No description provided.

@mirek-fidler
Copy link
Copy Markdown
Contributor

This would break exchanging data between platforms...

I guess any code that expects long to be 64 bit is sort of broken by default anyway. That said I agree that it would be better to always store long as 64 bits (with slightly different code), but a bit late for that now...

@jakubpawlewicz
Copy link
Copy Markdown
Author

Thank you for a comment. The problem is that some external libraries work on int64_t from <cstdint> which is in fact defined as long. In U++ int64 is long long. Those types are incompatible if you work on templates. Sometimes I am enforced to use int64_t. It took me substantial amount of time why transferring data through U++ serialization resulted in serious problems. But it is ok. I will use my fork of ultimatepp in my applications. If you want you can close that PR.

@mirek-fidler
Copy link
Copy Markdown
Contributor

mirek-fidler commented Sep 7, 2024 via email

@mirek-fidler
Copy link
Copy Markdown
Contributor

mirek-fidler commented Oct 19, 2024

Well, I think I have sort of solid solution evolving for this after all...

The plan is

  • actually remove operator%(long &) - this will make sure that no legacy code breaks - developer will just be forced to use the new method. At the same time, it is also very unlikely that any 'normal' longs were ever serialised

  • replace with method SerializeLong64(long&). I guess should not be a problem for you to call that instead.

Also add Serialize8/16/32/64 for all intXX_t types, esp. int32_t and int64_t

Yep, a bit trivial solution compared to the time that has taken me to consider it, but I guess the only sound one.

What do you think?

Mirek

@jakubpawlewicz
Copy link
Copy Markdown
Author

Huh, removing operator% breaks some implementation from Core for instance StreamContainer which is used in Vector::Serialize().

Vector<int64_t> should be serializable.

@mirek-fidler
Copy link
Copy Markdown
Contributor

mirek-fidler commented Oct 21, 2024 via email

@mirek-fidler
Copy link
Copy Markdown
Contributor

Well, all things considered, I think this is the only safe way:

9cc3711

@jakubpawlewicz
Copy link
Copy Markdown
Author

So Vector<int64_t> will not serialize correctly since this relies on operator%. I will stay with my fork for my purposes.

@mirek-fidler
Copy link
Copy Markdown
Contributor

Well, I really see no way around - dword really is used to store persistent data that has to be correctly saved on windows and loaded in linux. And it basically has to be defined as unsigned long in win32....

@jakubpawlewicz
Copy link
Copy Markdown
Author

For me no problem. I have some other changes and I will keep use my fork of U++ anyway.

I understand that serialization is broken for long in Linux. I encountered some other projects that suffer with portability because of this incompatibility of size of 'long' between Linux and Windows.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants