Rudimentary validator for binary files #94
Conversation
| // it basically loads up the header and checks | ||
|
|
||
| // This struct and the getter are copied from the marian source, because it's located | ||
| // inside src/common/binary.cpp:15 and we can't include it. |
There was a problem hiding this comment.
/// to be picked up by doxygen, consistent with what exists in source.
There was a problem hiding this comment.
I don't want this to be picked up by doxygen? It's an internal function, not exposed by the headers and only really meant to be read by the developers.
There was a problem hiding this comment.
If it's not exposed by headers, it usually belongs in an anonymous namespace. Particularly for generic names like Header and get.
The standard for documentation is higher in this repo than it is elsewhere, sorry @XapaJIaMnu.
There was a problem hiding this comment.
Sorry, I genuinely do not understand how I should document this better. I'm went through the repo and looked at various files: (this one comes to mind) https://github.com/browsermt/bergamot-translator/blob/main/src/translator/request.h
Do I need to add the defines bit on top?
There was a problem hiding this comment.
There was a problem hiding this comment.
Ok that should be fixed now.
|
|
||
| bool validateBinaryModel(AlignedMemory& model, uint64_t fileSize) { | ||
| const void * current = &model[0]; | ||
| uint64_t memoryNeeded = sizeof(uint64_t)*2; // We keep track of how much memory we would need if we have a complete file |
There was a problem hiding this comment.
wrap lines with comments to 80 characters is standard? My config overwrites this autoformat and I always end up bearing the brunt.
There was a problem hiding this comment.
Dude, we're not 1998, we have 4k monitors. Just. No. All modern projects (including marian) have abandoned that.
src/translator/byte_array_util.cpp
Outdated
| } | ||
| } | ||
|
|
||
| } // Annonymous namespace |
Validate that the binary file you are trying to load is not truncated.