its still computing result, no? why does this have to change?

CalculateSHA256 returns a byte buf, which according to the CRT API is empty if it "failed". since this utility function hides away cipher state we use the empty/non-empty signal to determine if it was successfully calculated.

can we just add bytebuffer from string constructor?

ByteBuffer is an Array, but yeah that would be a good convenience function let me add that

do we even need object for this anymore? can we just make it a static helper?

so in theory yes, we could make a static helper function but it would require a major re-write of of how hashing works. specifically in Sha256 we separate implementation using pimpl using a unique ptr to a Hash type. because of this pimpl indirection we dont know the underlying type and invoke it using its virtual function. We could of course remove this pimpl indirection but the delta of that change would prove to be much more than and just declaring a temp hash var and using the vtable. IMO it would create a diff that would make this change more risky than it already is.

we could just have a static method in Sha256 that creates an instance and calls hash on it, instead of doing it all over the place? or have oneshot variant that resets state to working after hash? but whatever, not something to lose sleep over.

how does error handling work here? we just return empty cryptoBuffer and nothing else?

yeah thats all from the original API which heavily relied on the virtual operator bool() const { return Good(); } operator to check if the cipher was in a erroronus state or not. which is why i change in this PR to catpure the state of being finished or ready. The error handling here would be to check the ciphers boolean operator before using the return value per the original API.

add a check that other != parent of this
(i.e. static_cast<Crt::ByteBuf*>(this) != &other) or smth like this
otherwise you risk erasing the parent

CryptoBuffer != Crt::ByteBuf we actually want to delete the parent unconditionally as we are taking ownership of the underlying buffer from the bytebuf. since this is a move operation other is invalid after this operation and CryptoBuffer should always take owneship from parent, and it cannot move from self as they are different types.

have you tried to fix perfect forwarding in MakeShared / make_shared?

discussed offline, removing comment, its just a move ctor from the underlying type

why did you have to change this?
it seems to be an API backward incompatible change.

This is because of @DmitriyMusatkin 's requested change for adding a string to array ctor.. UUID is a special class in that it has a
operator Aws::String() const
AND
operator ByteBuffer() const

so when we convert a UUID to byte buf the ctor is ambigious now because there is a ctor with both string and byte buf. Is this a breaking API change? sort of? if a class has a operator defined for both String and ByteBuf and used in a constructor for Array. I think we can rationalize in making this change as we are the only people likely doing that.