Skip to content
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

fit: Skip array fields on pre read messages #878

Merged
merged 1 commit into from
Feb 12, 2024

Conversation

mlofjard
Copy link
Contributor

I also used your type->size idea for the expected sizes which simplified the switch statements alot.

func readUint(d *decode.D, fDef mappers.LocalFieldDef, valMap valueMap) {
expectedSize := expectedSizeMap[fDef.Type]
if fDef.Size != expectedSize {
d.SeekRel(int64(fDef.Size) * 8) // skip over array types since they cannot be referenced by subfields
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How come this seek and the a seek in fitDecodeDataMessage does not cause "gaps", some other field cover that bit range?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seek and the one on line 360 are just to keep the pos in sync since we only care about uint fields on the first pass. On line 363 we seek back to before the pre-read and read all the data with d.Field-methods.

@wader
Copy link
Owner

wader commented Feb 12, 2024

+54 -44 😢 thought it would save lines 😬

@wader wader merged commit e1da76d into wader:master Feb 12, 2024
5 checks passed
@mlofjard
Copy link
Contributor Author

+54 -44 😢 thought it would save lines 😬

You win some, you lose some =) At least it's less repeating code.

@mlofjard mlofjard deleted the fit-peek-array-support branch February 12, 2024 15:56
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