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

[luci/import] Revise importModule method #13902

Merged
merged 1 commit into from
Sep 3, 2024

Conversation

seanshpark
Copy link
Contributor

This will revise importModule method with circle::Module as

  • make it private to prevent future isses with large model
  • add some guards
  • only call reader with file data and size

This will revise importModule method with circle::Module as
- make it private to prevent future isses with large model
- add some guards
- only call reader with file data and size

ONE-DCO-1.0-Signed-off-by: SaeHie Park <[email protected]>
@seanshpark
Copy link
Contributor Author

@seanshpark seanshpark requested a review from a team September 2, 2024 23:10
Copy link
Contributor

@jinevening jinevening left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -268,6 +268,10 @@ Importer::Importer()

std::unique_ptr<Module> Importer::importModule(const circle::Model *model) const
{
assert(model);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is checked in importModule(const uint8_t *data, size_t size)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing this :)

@seanshpark seanshpark merged commit 8f3f1cd into Samsung:master Sep 3, 2024
10 checks passed
@seanshpark seanshpark deleted the luci_imp_rev_importmod branch September 3, 2024 01:18
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