New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use const references to extend lifetime of temporaries #847

Closed
Bu11etmagnet opened this Issue Apr 19, 2018 · 3 comments

Comments

Projects
None yet
4 participants
@Bu11etmagnet
Copy link

Bu11etmagnet commented Apr 19, 2018

What platform/OS are you using?

Linux Ubuntu 17.10

What compiler are you using? what version?

gcc version 7.2.0 (Ubuntu 7.2.0-8ubuntu3.2)

aws-cpp-sdk-core/source/utils/crypto/openssl/CryptoImpl.cpp:639:24: error: 
      local variable 'finalBuffer' will be copied despite being returned by name [-Werror,-Wreturn-std-move]
                return finalBuffer;
                       ^~~~~~~~~~~
aws-cpp-sdk-core/source/utils/crypto/openssl/CryptoImpl.cpp:639:24: note: 
      call 'std::move' explicitly to avoid copying
                return finalBuffer;
                       ^~~~~~~~~~~

aws-cpp-sdk-core/source/utils/crypto/Cipher.cpp:143:24: error: 
      local variable 'key' will be copied despite being returned by name [-Werror,-Wreturn-std-move]
                return key;
                       ^~~
aws-cpp-sdk-core/source/utils/crypto/Cipher.cpp:143:24: note: 
      call 'std::move' explicitly to avoid copying
                return key;
                       ^~~
                       std::move(key)

Despite the suggestion to use std::move, I believe the fix is:

diff --git a/aws-cpp-sdk-core/source/utils/crypto/Cipher.cpp b/aws-cpp-sdk-core/source/utils/crypto/Cipher.cpp
index 5cc13916ec..9fea0870f7 100644
--- a/aws-cpp-sdk-core/source/utils/crypto/Cipher.cpp
+++ b/aws-cpp-sdk-core/source/utils/crypto/Cipher.cpp
@@ -133,7 +133,7 @@ namespace Aws
 
             CryptoBuffer SymmetricCipher::GenerateKey(size_t keyLengthBytes)
             {
-                CryptoBuffer&& key = GenerateXRandomBytes(keyLengthBytes, false);
+                CryptoBuffer const& key = GenerateXRandomBytes(keyLengthBytes, false);
 
                 if(key.GetLength() == 0)
                 {
diff --git a/aws-cpp-sdk-core/source/utils/crypto/openssl/CryptoImpl.cpp b/aws-cpp-sdk-core/source/utils/crypto/openssl/CryptoImpl.cpp
index 363a06b5b3..fe4ecac671 100644
--- a/aws-cpp-sdk-core/source/utils/crypto/openssl/CryptoImpl.cpp
+++ b/aws-cpp-sdk-core/source/utils/crypto/openssl/CryptoImpl.cpp
@@ -626,7 +626,7 @@ namespace Aws
 
             CryptoBuffer AES_GCM_Cipher_OpenSSL::FinalizeEncryption()
             {
-                CryptoBuffer&& finalBuffer = OpenSSLCipher::FinalizeEncryption();
+                CryptoBuffer const& finalBuffer = OpenSSLCipher::FinalizeEncryption();
                 m_tag = CryptoBuffer(TagLengthBytes);
                 if (!EVP_CIPHER_CTX_ctrl(m_ctx, EVP_CTRL_CCM_GET_TAG, static_cast<int>(m_tag.GetLength()),
                                          m_tag.GetUnderlyingData()))
@marcomagdy

This comment has been minimized.

Copy link
Contributor

marcomagdy commented Apr 23, 2018

Agreed. Will make the change. Thank you.

@marcomagdy

This comment has been minimized.

Copy link
Contributor

marcomagdy commented Apr 25, 2018

Fixed in v1.4.38

@muggenhor

This comment has been minimized.

Copy link

muggenhor commented Nov 6, 2018

Note that using auto&& key = .... Would have achieved the best of both worlds: it performs both lifetime extension and allows you to move from it later on to avoid a copy.

I.e. auto&& has the same lifetime extension semantics as const T&, unlike T&&, which doesn't perform lifetime extension.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment