Exceptions + mmap + groth16_prover_zkey_file #10
Conversation
# Conflicts: # build/fq.cpp # build/fr.cpp
src/binfile_utils.cpp
Outdated
| if (type != _type) { | ||
| munmap(addr, size); | ||
| close(fd); | ||
| throw new std::invalid_argument("Invalid file type. It should be " + _type + " and it is " + type + " filename: " + fileName); |
There was a problem hiding this comment.
In modern C++, it is not recommended to throw or catch exceptions by pointer; instead, use references. I would recommend removing the new keyword.
Furthermore, from what I observe, we do not free the memory allocated for exceptions caught by a pointer. This results in a memory leak.
src/binfile_utils.cpp
Outdated
| if (version > maxVersion) { | ||
| munmap(addr, size); | ||
| close(fd); | ||
| throw new std::invalid_argument("Invalid version. It should be <=" + std::to_string(maxVersion) + " and it is " + std::to_string(version)); |
There was a problem hiding this comment.
the same for exception pointer
src/binfile_utils.cpp
Outdated
|
|
||
| size = sb.st_size; | ||
|
|
||
| addr = mmap(NULL, sb.st_size, PROT_READ, MAP_PRIVATE, fd, 0); |
There was a problem hiding this comment.
clang-tidy recommends using C++ native nullptr variable instead of C-style NULL.
src/binfile_utils.cpp
Outdated
| @@ -8,8 +12,70 @@ | |||
|
|
|||
| namespace BinFileUtils { | |||
|
|
|||
| BinFile::BinFile(std::string fileName, std::string _type, uint32_t maxVersion) { | |||
There was a problem hiding this comment.
clang-tidy recommends using const std::string& fileName and const std::string& _type types.
src/prover.h
Outdated
| char *proof_buffer, unsigned long *proof_size, | ||
| char *public_buffer, unsigned long *public_size, | ||
| char *error_msg, unsigned long error_msg_maxsize); | ||
| int groth16_prover(const void *zkey_buffer, unsigned long zkey_size, |
There was a problem hiding this comment.
As this is a C header file (not C++), functions are declared in the traditional manner, such as
int
groth16_prover(...
his approach makes it easier to differentiate between function declarations and function usage using old-school grep-like utilities. Sure we do not obligated to support such style, but for me it looks more natural 🤷
src/binfile_utils.cpp
Outdated
| FileLoader fileLoader(filename); | ||
| // FileLoader fileLoader(filename); | ||
| // | ||
| // return std::unique_ptr<BinFile>(new BinFile(fileLoader.dataBuffer(), fileLoader.dataSize(), type, maxVersion)); |
There was a problem hiding this comment.
did you forget to remove those lines?
| close(fd); | ||
| } else { | ||
| free(addr); | ||
| } |
There was a problem hiding this comment.
Looks like this class is not safe for copy. I would delete default copy and assignment constructors to prevent these operations.
BinFile(const BinFile&) = delete;
BinFile& operator=(const BinFile&) = delete;
… default implementation.
No description provided.