factor
factor copied to clipboard
VM prepare image reserved fields usage
The version 4 image format did not write 0 into reserved_* fields (#2580). Therefore current version 4 images are not forward compatible with image versions that use these fields. By introducing an escape code 0 into the data_size field, three reserved_* fields now become available for redefinition, while keeping image version at 4.
The image.cpp changes can transparently read regular v4 image files, as well as ones with h.data_size=0 and h.escaped_data_size=data_size.
I’m a little unsure about this by itself.
Why not see what changes are needed by compressed images and maybe increment to version 5 instead of this indirection?
On Feb 10, 2022, at 9:37 AM, nomennescio @.***> wrote:
The version 4 image format did not write 0 into reserved_* fields (#2580). Therefore current version 4 images are not forward compatible with image versions that use these fields. By introducing an escape code 0 into the data_size field, three reserved_* fields now become available for redefinition, while keeping image version at 4.
The image.cpp changes can transparently read regular v4 image files, as well as ones with h.data_size=0 and h.escaped_data_size=data_size.
You can view, comment on, or merge this pull request online at:
https://github.com/factor/factor/pull/2591
Commit Summary
97a525a VM prepare image reserved fields usage File Changes (2 files) M vm/image.cpp (11) M vm/image.hpp (6) Patch Links:
https://github.com/factor/factor/pull/2591.patch https://github.com/factor/factor/pull/2591.diff — Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you are subscribed to this thread.
I can imagine your viewpoint. Version 5 version bump might be most appropriate, this is more like a premature contract while keeping all options open and staying at version 4, while only explicitly declaring how fields are to be interpreted which is compatible with current ways. I was hoping for a kind of baseline to build upon, but we can also postpone such a decision.
Made the code change less intrusive, with the same result.
Would you like this merged while you are also considering what other changes are needed?
Would you like this merged while you are also considering what other changes are needed?
Yes please. It doesn't break existing code, and we can still change it later, as we nowhere explicitly save into this format. As a next step for my compression proposal on my other experimental branch, I intend to use reserved_2 and reserved_3 for compressed_data_size
and compressed_code_size
, which requires some non-intrusive image header change like this.
I can also just put everything in an experimental branch, but I like to keep feature/experimental branches small.
You can decide.
Note that in a valid version 4 image file, actual data_size
can never be 0 due to an assert in raw_fread
. We will use this invariant as an escape value.