Implement per-value reader context in abstract reader interface#326
Open
sjlongland wants to merge 15 commits intointel:devfrom
Open
Implement per-value reader context in abstract reader interface#326sjlongland wants to merge 15 commits intointel:devfrom
sjlongland wants to merge 15 commits intointel:devfrom
Conversation
Describe the input parameters for the function and how they are used as best we understand from on-paper analysis of the C code.
What is not known, is what the significance is of `CborEncoderAppendType`. It basically tells the writer the nature of the data being written, but the default implementation ignores this and just blindly appends it no matter what. That raises the question of why it's important enough that the writer function needs to know about it.
Not 100% sure of the syntax for documenting struct-members outside of the struct as I'm too used to doing it inline, hopefully this works as expected. :-)
The `token` parameter is not sufficient since it is effectively shared by all `CborValue` instances. Since `tinycbor` often uses a temporary `CborValue` context to perform some operation, we need to store our context inside that `CborValue` so that we don't pollute the global state of the reader.
In its place, put an arbitrary `void *` pointer for reader context. The reader needs to store some context information which is specific to the `CborParser` instance it is serving. Right now, `CborValue::source::token` serves this purpose, but the problem is that we also need a per-`CborValue` context and have nowhere to put it. Better to spend an extra pointer (4 bytes on 32-bit platforms) in the `CborParser` (which there'll be just one of), then to do it in the `CborValue` (which there may be several of) or to use a `CborReader` object that itself carries two pointers (`ops` and the context, thus we'd need an extra 3 pointers).
We simplify this reader in two ways: 1. we remove the `consumed` member of `struct Input`, and instead use the `CborValue`'s `source.token` member, which we treat as an unsigned integer offset into our `QByteArray`. 2. we replace the reader-specific `struct Input` with the `QByteArray` it was wrapping, since that's the only thing now contained in our `struct Input`. If a `CborValue` gets cloned, the pointer referred to by `source.token` similarly gets cloned, thus when we advance the pointer on the clone, it leaves the original alone, so computing the length of unknown-length entities in the CBOR document can be done safely.
This reads a CBOR file piece-wise, seeking backward and forward through the file if needed. Some seeking can be avoided by tuning the block size used in reads so that the read window shifts by smaller amounts.
Author
|
This follows on from #208 as the changes would clash if they were kept separate. |
sjlongland
force-pushed
the
feature/WSHUB-455-chunked-codec-parser
branch
from
8fec047 to
c32860c
Compare
Instead of testing a bit in a flags bitmap. intel#208 (comment)
sjlongland
force-pushed
the
feature/WSHUB-455-chunked-codec-parser
branch
from
c32860c to
496f0fb
Compare
As per suggestion: intel#208 (comment)
To ensure we don't truncate to 32-bits as suggested here: intel#208 (comment)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The abstract reader interface relies on passing around a
tokenwhich represents the abstract reader context, keeping context information global to the entire parser instance.This leads to significantly more work at the implementation end as it has no concept of what is being read and therefore whether it might already have the information needed. The end result being additional I/O that could have been avoided.
By passing in the
CborValue, we can provide a little more detail to the reader and allow it to store per-value context information so that it is not constantly jumping back and forth within the file unnecessarily.