tls: Restructure gnutls_datum move operator, adding construstor #2214
Conversation
Refs c224fe0 Refs scylladb#2208 Makes the type more RAII complete, and following usual pattern of destroy + construct in place -> old eyes can read easier.
| ::gnutls_free(data); | ||
| if (this != &other) { | ||
| this->~gnutls_datum(); | ||
| new (this) gnutls_datum(std::move(other)); |
There was a problem hiding this comment.
yeah, i understand the motivation. but, personally, i am not in favor of calling destructor manually and using placement new unless really necessary. they hurt the readability. and the new move-ctor is never used. i'd defer the review to seastar maintainers.
There was a problem hiding this comment.
Only motivation is that we tend to use this pattern elsewhere in seastar etc. Plus it is delegating operations. :-)
There was a problem hiding this comment.
i see. i just don't really like this pattern, despite that it is repeating itself in seastar =( . and yeah, it reuses the dtor in a very obvious way.. but still.. i just cannot convince myself that i like it.
There was a problem hiding this comment.
I'm at fault for introducing it but I now regret it, it's likely not totally safe wrt the standard.
Refs c224fe0
Refs #2208
Makes the type more RAII complete, and following usual pattern of destroy + construct in place -> old eyes can read easier.