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

[GO] Verifier #8030

Open
wants to merge 38 commits into
base: master
Choose a base branch
from
Open

[GO] Verifier #8030

wants to merge 38 commits into from

Conversation

tira-misu
Copy link
Contributor

Here a PR for verifier for GO.

I have added

  • some documentation
  • tests
  • the verifier itself

tira-misu and others added 30 commits October 1, 2019 07:51
Pull from google/flatbuffers
If a struct has a key the vector has to be sorted. To sort the vector
you can't use "const".
* option indent_step is supported
@github-actions github-actions bot added golang c++ grpc codegen Involving generating code from schema documentation Documentation labels Jul 10, 2023
go/verify.go Outdated
//
// /* Verify Monster table. Buffer name is 'MONS' and contains data length prefix */
// isValid := verifier.VerifyBuffer("MONS", true, MonsterVerify)
func (verifier *Verifier) VerifyBuffer(identifier []byte, sizePrefixed bool, verifyObjFunc VerifyTableFunc) bool {
Copy link
Contributor

@jdemeyer jdemeyer Jul 10, 2023

Choose a reason for hiding this comment

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

In other places, I've used the type string for the identifier, see for example the code in go/lib.go and the FooIdentifier constants in the generated code. So I suggest to use string also in this code, for consistency.

Interestingly, your last example line

isValid := verifier.VerifyBuffer("MONS", true, MonsterVerify)

already assumes a string.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

For a more detailed control of verification `MonsterVerify` for `Monster`
type can be used:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~{.go}
# Sequence of calls
Copy link
Contributor

@jdemeyer jdemeyer Jul 10, 2023

Choose a reason for hiding this comment

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

This looks like a mixture of Go and Python syntax somehow...

  • To fix b'MONS', I suggest simply using strings, so that would become "MONS".
  • The trailing \ is invalid and can be removed as the line end with a .
  • Comments start with //

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To many different languages at the same time. fixed.

Each root type will have a verification function generated for it,
e.g. `VerifyMonster`. This can be called as shown:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~{.go}
ok := VerifyMonster(buf);
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove trailing ;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

switch Any(typeId) {
case AnyMonster:
result = MonsterVerify(verifier, tablePos)
break
Copy link
Contributor

Choose a reason for hiding this comment

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

In Go, these break statements are not needed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@@ -47,6 +47,11 @@ func FinishReferrableBuffer(builder *flatbuffers.Builder, offset flatbuffers.UOf
builder.Finish(offset)
}

func VerifyReferrable(buf []byte) bool {
return flatbuffers.NewVerifier(buf).VerifyBuffer(nil, false, ReferrableVerify)

Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid generating an empty line here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@tira-misu
Copy link
Contributor Author

What about the build step buildkite/flatbuffers/pr? I don't see the problem.

@visanalexandru
Copy link

Any updates?

Copy link

This pull request is stale because it has been open 6 months with no activity. Please comment or label not-stale, or this will be closed in 14 days.

@github-actions github-actions bot added stale and removed stale labels Mar 19, 2024
Copy link

This pull request is stale because it has been open 6 months with no activity. Please comment or label not-stale, or this will be closed in 14 days.

@github-actions github-actions bot added the stale label Sep 24, 2024
@xpoh
Copy link

xpoh commented Oct 1, 2024

not-stale

@github-actions github-actions bot removed the stale label Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ codegen Involving generating code from schema documentation Documentation golang grpc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants