-
Notifications
You must be signed in to change notification settings - Fork 9
Offset pack #34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Offset pack #34
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #34 +/- ##
==========================================
- Coverage 94.53% 92.93% -1.60%
==========================================
Files 1 1
Lines 128 184 +56
==========================================
+ Hits 121 171 +50
- Misses 7 13 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Sorry, I don't think anyone has had a chance to look at this. I'm a little skeptical of just entering the raw byte offset for each field — seems error prone (what if the user gives invalid offsets that produce overlapping fields?), and confusing to be able to specify the fields out of order. An alternative would be to just support @io struct RawData
A::UInt32
_::UInt16 # 2 bytes of padding
B::UInt16
_::UInt32 # 4 bytes of padding
C::UInt128
_::NTuple{6,UInt8} # 6 bytes of padding
D::UInt8
end align_packed(Strictly speaking this would be a breaking change, as Julia currently allows you to have fields named @io struct RawData
A::UInt32
_::2
B::UInt16
_::4
C::UInt128
_::6
D::UInt8
end align_packedin which (One question that occurs to me is whether the offset should be in bytes, or bits? But I guess we don't support bit-packing right now anyway.) |
That produces assert error in
Easy to remove that. I actually took extra steps to implement the functionality.
That seems to me clearer, indeed, compared to the dummy field name with an existing type. The question is, how to do packing of this structure with gaps. The easiest to implement approach is just to ignore gaps. Ability to write to only a few needed offsets in existing binary file could be useful, however. Perhaps, the gap specification can be added to all strategies, and additional
The default may be in bytes, and if there's a need for bits offset, additional syntax can be added later: @io struct RawData
A::UInt32
_::2
B::UInt16
_::4
C::UInt128
_::_6bits
D::UInt8
end align_gappedBut with no way to |
I thought that the point is that @io struct RawData
A::UInt32
_::2
B::UInt16
_::4
C::UInt128
_::6
D::UInt8
end align_packedwould be exactly the same on-disk format as @io struct RawData
A::UInt32
_dummy1::NTuple{2,UInt8}
B::UInt16
_dummy2::NTuple{4,UInt8}
C::UInt128
_dummy3::NTuple{6,UInt8}
D::UInt8
end align_packedfor I/O purposes, except that on the Julia side the |
Someone may want to pull just the needed fields from structure and then write them out in more compact file: "_packed" sort of conveys the idea of compacting the structure. So struct RawData
A::UInt32
B::UInt16
C::UInt128
D::UInt8
endwhereas |
I think in that case you would want to copy to a different |
Actually, more interesting question is what to do when encountering a node with gap specification like
The syntax currently proposed by this PR sidesteps those questions, because an offset is linked to the field. |
I think it would be safe to restrict the support to this case.
The whole point is that the |
|
I implemented the proposed syntax in #35 |
|
Closed because #35 has more intuitive syntax |
This PR adds the ability to unpack a binary buffer while skipping some parts of it. The usage may be illustrated by this fragment from
runtests.jl:The
pack()method foralign_offsetpacking strategy is not yet implemented, because there are several ways to go about it, and I could use some discussion. If there's an interest in merging this PR, I'll fill the gaps.