6
Your speed-up claim:
avoid loading the whole file at once, instead parsing a file line by line, which seems to be 2x as fast according to testing
seems highly unlikely, and I would like to see proof of this. A compiled-and-tuned built-in JSON parser that operates on an in-memory buffer is nearly certain to outperform a non-compiled, non-built-in, line-by-...
5
Excellent comment in the header. It's very important to be clear who owns the returned memory and how to release it. It ought to mention that both newline and \0 are considered as end-of-line characters.
Good realloc() usage - we don't leak when allocation fails.
Some small improvements:
#define EXPAND_RATIO 2
Why do we need a preprocessor macro for this?...
5
Overview
Templated classes "normally" should provide all the code at the point of usage. As a result you should probably not have a separate shared.cpp. Though some people put the definitions in a separate shared.tpp file that is included by the header file.
Code Review:
You are missing a constructor for nullptr.
Though nullptr can be converted to ...
4
I am not sure how it works at all as it shouldn't compile. Generic templates require the whole implementation in .hpp/.h files and nothing hidden in .cpp files aside from some possible instantiations.
You have a ton of memory leaking due to ref_cnt{new RefCount()} in lots of places.
Those are incorrect ways to write copy assignment/operator:
...
4
Trivial: the declaration is not complete - you're missing the final ; after the class definition.
Missing includes:
#include <algorithm>
#include <cstddef>
using std::size_t; // don't do this in a header!
As noted in the comment, don't put standard library identifiers into the global namespace - just write the type in full throughout the ...
4
There are some bugs
There are several bugs in your code. In the constructor that takes a C string, you compute the length of cstr and set pSize accordingly, but you allocate pCapacity bytes. You should allocate at least pSize bytes. I recommend you get rid of pCapacity and try to make the class work with just pSize first.
You make the same mistake in ...
4
That is a lot of code to review, so this review will only touch on more superficial things.
I'm not familiar with the pattern of modifying __all__, but it seems like using an underscore at the start of names which should not be available from the outside would be a less "magic" way of achieving the same thing.
black and isort can help make the ...
4
Overall, a good and thoughtful effort.
Bug: Treats character as EOF
When EOF == -1 and code reads a character with the value of 255, the c = getc(fp); likely converts that 255 to -1. Later, the loop incorrectly exits.
signed char c;
c = getc(fp);
if (c == '\n' || c == EOF) return NULL; // Maybe true for wrong reason
Instead use an int c to well ...
3
There's absolutely no reason to make this code platform-specific.
Instead of guessing the size of the fundamental types, just use the standard types from the compiler-provided <cstdint> (we should only need std::size_t, I think). The whole reason that this header is provided is so we have a portable means of using information that the compiler knows ...
3
Overview
Couple of small bugs.
You don't always seem to initialize the appropriate objects correctly. Overall I find it a bit confusing when objects are initialized and when they are not.
The major missing part is that a lot of important functionality is another library and it I can check it is correct.
A lot of functionality you use pass a pointer and a ...
3
Overall this isn't a bad effort. I can't see any problems, aside from clobbering the EOF, mostly it's stylistic choices. So good job 👍.
Comments go above
It's much more common for the comment in a header to be ABOVE the function, rather than below it. This is ok for now as there's only one function, but when you have more it will confuse people.
If you're ...
3
The mb_strlen($configFileNameWithPath) > 0 checks don't need the > 0 part at the end. mb_strlen() will always return an integer-type value, so you only need to check if it is truthy. As I think about it, I don't see the value in mb_strlen() -- it will be potentially more accurate in non-zero cases, but you only intend to differentiate between zero ...
2
#include only the headers you need where you need them. MyImageSize.h needs no includes. MonoPixelOperation.h needs only <memory>.
template<class T = unsigned int>
class MyImageSize
{
private:
T xsize;
T ysize;
public:
MyImageSize(T new_xsize, T new_ysize)
{
this->xsize = new_xsize;
this->ysize = new_ysize;...
1
General review about pow.h and pow.c.
When overflow does not occur, pow_umax() can only return UINTMAX_MAX with pow_umax(UINTMAX_MAX, 1).
There is no other base, expo such that pow_umax(base, expo) returns UINTMAX_MAX, regardless of the bit width of uintmax_t due to Mihailescu's Theorem. [Just found that out today.]
So setting errno = ERANGE is only ...
1
As a discussion point (and not strictly a recommendation), I wonder how your overflow detection algorithm would compare with the following:
Assuming a 64-bit integer (substituting as necessary): We know that if \$y \ge 2^{64} \$, \$y\$ has overflowed. If \$ y = x^a \$,
$$ x^a \lt 2^{64} $$
$$ a \log_2 x < 64 $$
\$ \log_2 \$ is cheap and can be done with ...
1
Advanced Encryption Standard.
Please simply include a well defined reference to the standard (it's FIPS 197 by NIST). If you do use any of the standard notations in there, indicate that as well, it will make your code much easier to understand.
'MODE_ECB', 'MODE_CBC', 'MODE_CFB', 'MODE_OFB', 'MODE_CTR'
Modes of operation are not part of the block cipher ...
Only top voted, non community-wiki answers of a minimum length are eligible
