Use smart pointers (std::unique_ptr) in API then ownership is transferred

[lieser]

I think it would be good if Botan 3.x would start using std::unique_ptr in its API there ownership is transferred, instead of raw pointers. In some cases it should also be possible to just add it as an overload, if you prefer to still allow passing a raw pointer too.

I did not look at the complete code there raw pointer are currently used to transfer ownership. Below are only the instances there we currently need to disable the using new warnings in the static analysis of our code:

• Serialized_RNG ctor
• return value of Certificate_Extension::copy()
• Extensions::add(...) (add_new/replace too)

/cc @securitykernel

[randombit]

I general fine with this approach. For Serialized_RNG we might still want a (deprecated) constructor that takes a raw pointer in addition to one consuming a unique_ptr, but the others you mention are relatively obscure and I'd be fine with just using only unique_ptr for those.

[randombit]

A good goal to shoot for IMO is to remove all uses of explicit new (now that we have make_unique available) in the same way that (almost) all uses of explicit delete were removed a few years back in preference to RAII.

[securitykernel]

Definitely a good target for 3.x. Thanks for all the effort @randombit!

[lieser]

Thanks for the quick work on this. And nice to see that you took the additional effort on also getting rid of new/delete in Botan's internal code.

[lieser]

I had a quick look what is still missing, and found the following APIs there currently raw pointers with transferred ownership is used, and now newer API seems to be available:

• [ ] Filter API, e.g. Cipher_Mode_Filter
  • Note that I did not look closer at the filter code
• [x] PBKDF2_Family ctor
• [x] PKCS5_PBKDF2 ctor
• [x] OpenPGP_S2K ctor
• [x] RFC4880_S2K ctor
• [x] RFC4880_S2K_Family ctor
• [x] X509::load_key()
• [x] X509::copy_key()
• [x] DLIES_Encryptor ctor
• [x] DLIES_Decryptor ctor
• [x] DLIES_Decryptor ctor
• [x] PKCS10_Request::subject_public_key()
  • X509_Certificate has std::unique_ptr<Public_Key> load_subject_public_key() as an alternative, but this is missing here

@randombit I think the constructors are not that important, as a new overload can be easily added. But if for X509::load_key() and X509::copy_key() the return value should be changed instead of adding new alternative functions this needs to happen before the final 3.0.0.

[reneme]

We should probably remove:

PublicKey* X509_Certificate::subject_public_key()

... it returns a raw pointer and documentation claims that the caller is responsible for the object. New code shall use load_subject_public_key() instead. Probably, the old method can go with 3.0.

• [x] Remove X509_Certificate::subject_public_key()

[randombit]

I believe all of the above mentioned APIs have now been updated to using smart pointers, with the exception of the filters code. For the filters, I am inclined to just leave things exactly as they are. Already the documentation states that no further development is being done there, and recommends avoiding it in new code.