Try #2 at connect amendment fixup

bretambrose · bretambrose · commit 922002abbb78 · 2025-08-05T17:28:58.000-07:00
diff --git a/eventstream_rpc/include/aws/eventstreamrpc/EventStreamClient.h b/eventstream_rpc/include/aws/eventstreamrpc/EventStreamClient.h
@@ -61,7 +61,6 @@ namespace Aws
           public:
             EventStreamHeader(const EventStreamHeader &lhs) noexcept;
             EventStreamHeader(EventStreamHeader &&rhs) noexcept;
-            EventStreamHeader &operator=(const EventStreamHeader &lhs) noexcept;
             explicit EventStreamHeader(
                 const struct aws_event_stream_header_value_pair &header,
                 Crt::Allocator *allocator = Crt::g_allocator);
@@ -72,6 +71,8 @@ namespace Aws
 
             ~EventStreamHeader() noexcept;
 
+            EventStreamHeader &operator=(const EventStreamHeader &lhs) noexcept;
+
             Crt::String GetHeaderName() const noexcept;
 
             bool GetValueAsString(Crt::String &) const noexcept;
@@ -92,12 +93,9 @@ namespace Aws
         class AWS_EVENTSTREAMRPC_API MessageAmendment final
         {
           public:
+            explicit MessageAmendment(Crt::Allocator *allocator = Crt::g_allocator) noexcept;
             MessageAmendment(const MessageAmendment &lhs);
             MessageAmendment(MessageAmendment &&rhs) noexcept;
-            MessageAmendment &operator=(const MessageAmendment &lhs);
-            MessageAmendment &operator=(MessageAmendment &&rhs) noexcept;
-            ~MessageAmendment() noexcept;
-            explicit MessageAmendment(Crt::Allocator *allocator = Crt::g_allocator) noexcept;
             MessageAmendment(
                 const Crt::List<EventStreamHeader> &headers,
                 Crt::Optional<Crt::ByteBuf> &payload,
@@ -112,6 +110,11 @@ namespace Aws
                 const Crt::ByteBuf &payload,
                 Crt::Allocator *allocator = Crt::g_allocator) noexcept;
 
+            ~MessageAmendment() noexcept;
+
+            MessageAmendment &operator=(const MessageAmendment &lhs);
+            MessageAmendment &operator=(MessageAmendment &&rhs) noexcept;
+
             /**
              * Add a given header to the end of the header list.
              */
@@ -378,7 +381,7 @@ namespace Aws
         {
           public:
             EventstreamResultVariantType();
-            explicit EventstreamResultVariantType(Crt::ScopedResource<AbstractShapeBase> &&modeledResult) noexcept;
+            explicit EventstreamResultVariantType(Crt::ScopedResource<AbstractShapeBase> &&modeledResponse) noexcept;
             explicit EventstreamResultVariantType(Crt::ScopedResource<OperationError> &&modeledError) noexcept;
             explicit EventstreamResultVariantType(RpcError rpcError) noexcept;
             EventstreamResultVariantType(EventstreamResultVariantType &&rhs) noexcept;
@@ -388,7 +391,7 @@ namespace Aws
 
             ResultType GetType() const { return m_type; }
 
-            AbstractShapeBase *GetModeledResult() const;
+            AbstractShapeBase *GetModeledResponse() const;
 
             OperationError *GetModeledError() const;
 
@@ -397,7 +400,7 @@ namespace Aws
           private:
             ResultType m_type;
 
-            Crt::ScopedResource<AbstractShapeBase> m_modeledResult;
+            Crt::ScopedResource<AbstractShapeBase> m_modeledResponse;
             Crt::ScopedResource<OperationError> m_modeledError;
             RpcError m_rpcError;
         };
diff --git a/eventstream_rpc/source/EventStreamClient.cpp b/eventstream_rpc/source/EventStreamClient.cpp
@@ -1025,14 +1025,14 @@ namespace Aws
                 MessageAmendment connectAmendment(m_connectionConfig.GetConnectAmendment().value());
                 amendmentStorage.PrependHeaders(std::move(connectAmendment).GetHeaders());
 
-                // optional of a C structure is a terrible idea because it doesn't have a move which means
-                // a correct-looking move out of the optional does not actually erase the C struct, leading to a
-                // double free.  For now, work around by copying the buffer and then erasing the source optional
-                // by hand.
-                amendmentStorage.SetPayload(connectAmendment.GetPayload());
+                if (connectAmendment.GetPayload().has_value())
+                {
+                    const auto &payload = connectAmendment.GetPayload().value();
 
-                Crt::Optional<Crt::ByteBuf> noBuffer;
-                connectAmendment.SetPayload(noBuffer);
+                    Crt::Optional<Crt::ByteBuf> copiedPayload =
+                        Crt::ByteBufNewCopy(payload.allocator, payload.buffer, payload.len);
+                    amendmentStorage.SetPayload(copiedPayload);
+                }
             }
 
             s_fillNativeHeadersArray(amendmentStorage.GetHeaders(), headersArray, m_allocator);
@@ -2392,33 +2392,33 @@ namespace Aws
         }
 
         EventstreamResultVariantType::EventstreamResultVariantType()
-            : m_type(ResultType::NONE), m_modeledResult(nullptr), m_modeledError(nullptr),
+            : m_type(ResultType::NONE), m_modeledResponse(nullptr), m_modeledError(nullptr),
               m_rpcError({EventStreamRpcStatusCode::EVENT_STREAM_RPC_SUCCESS, AWS_ERROR_SUCCESS})
         {
         }
 
         EventstreamResultVariantType::EventstreamResultVariantType(
             Crt::ScopedResource<AbstractShapeBase> &&modeledResult) noexcept
-            : m_type(ResultType::OPERATION_RESPONSE), m_modeledResult(std::move(modeledResult)),
+            : m_type(ResultType::OPERATION_RESPONSE), m_modeledResponse(std::move(modeledResult)),
               m_modeledError(nullptr),
               m_rpcError({EventStreamRpcStatusCode::EVENT_STREAM_RPC_SUCCESS, AWS_ERROR_SUCCESS})
         {
         }
 
         EventstreamResultVariantType::EventstreamResultVariantType(
             Crt::ScopedResource<OperationError> &&modeledError) noexcept
-            : m_type(ResultType::OPERATION_ERROR), m_modeledResult(nullptr), m_modeledError(std::move(modeledError)),
+            : m_type(ResultType::OPERATION_ERROR), m_modeledResponse(nullptr), m_modeledError(std::move(modeledError)),
               m_rpcError({EventStreamRpcStatusCode::EVENT_STREAM_RPC_SUCCESS, AWS_ERROR_SUCCESS})
         {
         }
 
         EventstreamResultVariantType::EventstreamResultVariantType(RpcError rpcError) noexcept
-            : m_type(ResultType::RPC_ERROR), m_modeledResult(nullptr), m_modeledError(nullptr), m_rpcError(rpcError)
+            : m_type(ResultType::RPC_ERROR), m_modeledResponse(nullptr), m_modeledError(nullptr), m_rpcError(rpcError)
         {
         }
 
         EventstreamResultVariantType::EventstreamResultVariantType(EventstreamResultVariantType &&rhs) noexcept
-            : m_type(rhs.m_type), m_modeledResult(std::move(rhs.m_modeledResult)),
+            : m_type(rhs.m_type), m_modeledResponse(std::move(rhs.m_modeledResponse)),
               m_modeledError(std::move(rhs.m_modeledError)), m_rpcError(rhs.m_rpcError)
         {
         }
@@ -2429,17 +2429,17 @@ namespace Aws
             if (this != &rhs)
             {
                 m_type = rhs.m_type;
-                m_modeledResult = std::move(rhs.m_modeledResult);
+                m_modeledResponse = std::move(rhs.m_modeledResponse);
                 m_modeledError = std::move(rhs.m_modeledError);
                 m_rpcError = rhs.m_rpcError;
             }
 
             return *this;
         }
 
-        AbstractShapeBase *EventstreamResultVariantType::GetModeledResult() const
+        AbstractShapeBase *EventstreamResultVariantType::GetModeledResponse() const
         {
-            return m_modeledResult.get();
+            return m_modeledResponse.get();
         }
 
         OperationError *EventstreamResultVariantType::GetModeledError() const
diff --git a/eventstream_rpc/tests/include/awstest/EchoTestRpcModel.h b/eventstream_rpc/tests/include/awstest/EchoTestRpcModel.h
@@ -686,7 +686,7 @@ namespace Awstest
         GetAllProductsResult(EventstreamResultVariantType &&result) noexcept : m_result(std::move(result)) {}
         GetAllProductsResponse *GetOperationResponse() const noexcept
         {
-            return static_cast<GetAllProductsResponse *>(m_result.GetModeledResult());
+            return static_cast<GetAllProductsResponse *>(m_result.GetModeledResponse());
         }
 
         /**
@@ -736,7 +736,7 @@ namespace Awstest
         CauseServiceErrorResult(EventstreamResultVariantType &&result) noexcept : m_result(std::move(result)) {}
         CauseServiceErrorResponse *GetOperationResponse() const noexcept
         {
-            return static_cast<CauseServiceErrorResponse *>(m_result.GetModeledResult());
+            return static_cast<CauseServiceErrorResponse *>(m_result.GetModeledResponse());
         }
 
         /**
@@ -835,7 +835,7 @@ namespace Awstest
         CauseStreamServiceToErrorResult(EventstreamResultVariantType &&result) noexcept : m_result(std::move(result)) {}
         EchoStreamingResponse *GetOperationResponse() const noexcept
         {
-            return static_cast<EchoStreamingResponse *>(m_result.GetModeledResult());
+            return static_cast<EchoStreamingResponse *>(m_result.GetModeledResponse());
         }
 
         /**
@@ -937,7 +937,7 @@ namespace Awstest
         EchoStreamMessagesResult(EventstreamResultVariantType &&result) noexcept : m_result(std::move(result)) {}
         EchoStreamingResponse *GetOperationResponse() const noexcept
         {
-            return static_cast<EchoStreamingResponse *>(m_result.GetModeledResult());
+            return static_cast<EchoStreamingResponse *>(m_result.GetModeledResponse());
         }
 
         /**
@@ -1000,7 +1000,7 @@ namespace Awstest
         EchoMessageResult(EventstreamResultVariantType &&result) noexcept : m_result(std::move(result)) {}
         EchoMessageResponse *GetOperationResponse() const noexcept
         {
-            return static_cast<EchoMessageResponse *>(m_result.GetModeledResult());
+            return static_cast<EchoMessageResponse *>(m_result.GetModeledResponse());
         }
 
         /**
@@ -1050,7 +1050,7 @@ namespace Awstest
         GetAllCustomersResult(EventstreamResultVariantType &&result) noexcept : m_result(std::move(result)) {}
         GetAllCustomersResponse *GetOperationResponse() const noexcept
         {
-            return static_cast<GetAllCustomersResponse *>(m_result.GetModeledResult());
+            return static_cast<GetAllCustomersResponse *>(m_result.GetModeledResponse());
         }
 
         /**
diff --git a/greengrass_ipc/include/aws/greengrass/GreengrassCoreIpcModel.h b/greengrass_ipc/include/aws/greengrass/GreengrassCoreIpcModel.h

Original file line number	Diff line number	Diff line change
`@@ -1025,14 +1025,14 @@ namespace Aws`
`1025`	`1025`	`MessageAmendment connectAmendment(m_connectionConfig.GetConnectAmendment().value());`
`1026`	`1026`	`amendmentStorage.PrependHeaders(std::move(connectAmendment).GetHeaders());`
`1027`	`1027`
`1028`		`- // optional of a C structure is a terrible idea because it doesn't have a move which means`
`1029`		`- // a correct-looking move out of the optional does not actually erase the C struct, leading to a`
`1030`		`- // double free. For now, work around by copying the buffer and then erasing the source optional`
`1031`		`- // by hand.`
`1032`		`- amendmentStorage.SetPayload(connectAmendment.GetPayload());`
	`1028`	`+ if (connectAmendment.GetPayload().has_value())`
	`1029`	`+ {`
	`1030`	`+ const auto &payload = connectAmendment.GetPayload().value();`
`1033`	`1031`
`1034`		`- Crt::Optional<Crt::ByteBuf> noBuffer;`
`1035`		`- connectAmendment.SetPayload(noBuffer);`
	`1032`	`+ Crt::Optional<Crt::ByteBuf> copiedPayload =`
	`1033`	`+ Crt::ByteBufNewCopy(payload.allocator, payload.buffer, payload.len);`
	`1034`	`+ amendmentStorage.SetPayload(copiedPayload);`
	`1035`	`+ }`
`1036`	`1036`	`}`
`1037`	`1037`
`1038`	`1038`	`s_fillNativeHeadersArray(amendmentStorage.GetHeaders(), headersArray, m_allocator);`
`@@ -2392,33 +2392,33 @@ namespace Aws`
`2392`	`2392`	`}`
`2393`	`2393`
`2394`	`2394`	`EventstreamResultVariantType::EventstreamResultVariantType()`
`2395`		`- : m_type(ResultType::NONE), m_modeledResult(nullptr), m_modeledError(nullptr),`
	`2395`	`+ : m_type(ResultType::NONE), m_modeledResponse(nullptr), m_modeledError(nullptr),`
`2396`	`2396`	`m_rpcError({EventStreamRpcStatusCode::EVENT_STREAM_RPC_SUCCESS, AWS_ERROR_SUCCESS})`
`2397`	`2397`	`{`
`2398`	`2398`	`}`
`2399`	`2399`
`2400`	`2400`	`EventstreamResultVariantType::EventstreamResultVariantType(`
`2401`	`2401`	`Crt::ScopedResource<AbstractShapeBase> &&modeledResult) noexcept`
`2402`		`- : m_type(ResultType::OPERATION_RESPONSE), m_modeledResult(std::move(modeledResult)),`
	`2402`	`+ : m_type(ResultType::OPERATION_RESPONSE), m_modeledResponse(std::move(modeledResult)),`
`2403`	`2403`	`m_modeledError(nullptr),`
`2404`	`2404`	`m_rpcError({EventStreamRpcStatusCode::EVENT_STREAM_RPC_SUCCESS, AWS_ERROR_SUCCESS})`
`2405`	`2405`	`{`
`2406`	`2406`	`}`
`2407`	`2407`
`2408`	`2408`	`EventstreamResultVariantType::EventstreamResultVariantType(`
`2409`	`2409`	`Crt::ScopedResource<OperationError> &&modeledError) noexcept`
`2410`		`- : m_type(ResultType::OPERATION_ERROR), m_modeledResult(nullptr), m_modeledError(std::move(modeledError)),`
	`2410`	`+ : m_type(ResultType::OPERATION_ERROR), m_modeledResponse(nullptr), m_modeledError(std::move(modeledError)),`
`2411`	`2411`	`m_rpcError({EventStreamRpcStatusCode::EVENT_STREAM_RPC_SUCCESS, AWS_ERROR_SUCCESS})`
`2412`	`2412`	`{`
`2413`	`2413`	`}`
`2414`	`2414`
`2415`	`2415`	`EventstreamResultVariantType::EventstreamResultVariantType(RpcError rpcError) noexcept`
`2416`		`- : m_type(ResultType::RPC_ERROR), m_modeledResult(nullptr), m_modeledError(nullptr), m_rpcError(rpcError)`
	`2416`	`+ : m_type(ResultType::RPC_ERROR), m_modeledResponse(nullptr), m_modeledError(nullptr), m_rpcError(rpcError)`
`2417`	`2417`	`{`
`2418`	`2418`	`}`
`2419`	`2419`
`2420`	`2420`	`EventstreamResultVariantType::EventstreamResultVariantType(EventstreamResultVariantType &&rhs) noexcept`
`2421`		`- : m_type(rhs.m_type), m_modeledResult(std::move(rhs.m_modeledResult)),`
	`2421`	`+ : m_type(rhs.m_type), m_modeledResponse(std::move(rhs.m_modeledResponse)),`
`2422`	`2422`	`m_modeledError(std::move(rhs.m_modeledError)), m_rpcError(rhs.m_rpcError)`
`2423`	`2423`	`{`
`2424`	`2424`	`}`
`@@ -2429,17 +2429,17 @@ namespace Aws`
`2429`	`2429`	`if (this != &rhs)`
`2430`	`2430`	`{`
`2431`	`2431`	`m_type = rhs.m_type;`
`2432`		`- m_modeledResult = std::move(rhs.m_modeledResult);`
	`2432`	`+ m_modeledResponse = std::move(rhs.m_modeledResponse);`
`2433`	`2433`	`m_modeledError = std::move(rhs.m_modeledError);`
`2434`	`2434`	`m_rpcError = rhs.m_rpcError;`
`2435`	`2435`	`}`
`2436`	`2436`
`2437`	`2437`	`return *this;`
`2438`	`2438`	`}`
`2439`	`2439`
`2440`		`- AbstractShapeBase *EventstreamResultVariantType::GetModeledResult() const`
	`2440`	`+ AbstractShapeBase *EventstreamResultVariantType::GetModeledResponse() const`
`2441`	`2441`	`{`
`2442`		`- return m_modeledResult.get();`
	`2442`	`+ return m_modeledResponse.get();`
`2443`	`2443`	`}`
`2444`	`2444`
`2445`	`2445`	`OperationError *EventstreamResultVariantType::GetModeledError() const`
Original file line number	Diff line number	Diff line change
`@@ -686,7 +686,7 @@ namespace Awstest`
`686`	`686`	`GetAllProductsResult(EventstreamResultVariantType &&result) noexcept : m_result(std::move(result)) {}`
`687`	`687`	`GetAllProductsResponse *GetOperationResponse() const noexcept`
`688`	`688`	`{`
`689`		`- return static_cast<GetAllProductsResponse *>(m_result.GetModeledResult());`
	`689`	`+ return static_cast<GetAllProductsResponse *>(m_result.GetModeledResponse());`
`690`	`690`	`}`
`691`	`691`
`692`	`692`	`/**`
`@@ -736,7 +736,7 @@ namespace Awstest`
`736`	`736`	`CauseServiceErrorResult(EventstreamResultVariantType &&result) noexcept : m_result(std::move(result)) {}`
`737`	`737`	`CauseServiceErrorResponse *GetOperationResponse() const noexcept`
`738`	`738`	`{`
`739`		`- return static_cast<CauseServiceErrorResponse *>(m_result.GetModeledResult());`
	`739`	`+ return static_cast<CauseServiceErrorResponse *>(m_result.GetModeledResponse());`
`740`	`740`	`}`
`741`	`741`
`742`	`742`	`/**`
`@@ -835,7 +835,7 @@ namespace Awstest`
`835`	`835`	`CauseStreamServiceToErrorResult(EventstreamResultVariantType &&result) noexcept : m_result(std::move(result)) {}`
`836`	`836`	`EchoStreamingResponse *GetOperationResponse() const noexcept`
`837`	`837`	`{`
`838`		`- return static_cast<EchoStreamingResponse *>(m_result.GetModeledResult());`
	`838`	`+ return static_cast<EchoStreamingResponse *>(m_result.GetModeledResponse());`
`839`	`839`	`}`
`840`	`840`
`841`	`841`	`/**`
`@@ -937,7 +937,7 @@ namespace Awstest`
`937`	`937`	`EchoStreamMessagesResult(EventstreamResultVariantType &&result) noexcept : m_result(std::move(result)) {}`
`938`	`938`	`EchoStreamingResponse *GetOperationResponse() const noexcept`
`939`	`939`	`{`
`940`		`- return static_cast<EchoStreamingResponse *>(m_result.GetModeledResult());`
	`940`	`+ return static_cast<EchoStreamingResponse *>(m_result.GetModeledResponse());`
`941`	`941`	`}`
`942`	`942`
`943`	`943`	`/**`
`@@ -1000,7 +1000,7 @@ namespace Awstest`
`1000`	`1000`	`EchoMessageResult(EventstreamResultVariantType &&result) noexcept : m_result(std::move(result)) {}`
`1001`	`1001`	`EchoMessageResponse *GetOperationResponse() const noexcept`
`1002`	`1002`	`{`
`1003`		`- return static_cast<EchoMessageResponse *>(m_result.GetModeledResult());`
	`1003`	`+ return static_cast<EchoMessageResponse *>(m_result.GetModeledResponse());`
`1004`	`1004`	`}`
`1005`	`1005`
`1006`	`1006`	`/**`
`@@ -1050,7 +1050,7 @@ namespace Awstest`
`1050`	`1050`	`GetAllCustomersResult(EventstreamResultVariantType &&result) noexcept : m_result(std::move(result)) {}`
`1051`	`1051`	`GetAllCustomersResponse *GetOperationResponse() const noexcept`
`1052`	`1052`	`{`
`1053`		`- return static_cast<GetAllCustomersResponse *>(m_result.GetModeledResult());`
	`1053`	`+ return static_cast<GetAllCustomersResponse *>(m_result.GetModeledResponse());`
`1054`	`1054`	`}`
`1055`	`1055`
`1056`	`1056`	`/**`