From 6134deed3865de373f438e2a34fea53bba2c93ba Mon Sep 17 00:00:00 2001 From: ITHelpDec <34002836+ITHelpDec@users.noreply.github.com> Date: Wed, 31 May 2023 16:19:15 +0100 Subject: [PATCH 1/2] add early exit to `.pop()` - seems odd to perform work on a node that could potentially be a nullptr from the moment the function is called (regardless of CAS) - same thing applies to `void pop(T &val)` (early exit allows the code the run without throwing for bad access to `old_head->next`) --- listings/listing_7.3.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/listings/listing_7.3.cpp b/listings/listing_7.3.cpp index 29a85de..dea3efe 100644 --- a/listings/listing_7.3.cpp +++ b/listings/listing_7.3.cpp @@ -23,9 +23,9 @@ class lock_free_stack } std::shared_ptr pop() { + if (!head_) { return std::shared_ptr(); } node* old_head=head.load(); - while(old_head && - !head.compare_exchange_weak(old_head,old_head->next)); - return old_head ? old_head->data : std::shared_ptr(); + while(!head.compare_exchange_weak(old_head,old_head->next)); + return old_head->data; } }; From 7fb92f69420ae14b40d51a19f809c4c836db9907 Mon Sep 17 00:00:00 2001 From: ITHelpDec <34002836+ITHelpDec@users.noreply.github.com> Date: Wed, 31 May 2023 16:43:41 +0100 Subject: [PATCH 2/2] consider renaming `old_head` to `original_head` - optional, but maybe more useful in understanding the intent of `.compare_exchange_weak(...)` --- listings/listing_7.3.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/listings/listing_7.3.cpp b/listings/listing_7.3.cpp index dea3efe..228e2e3 100644 --- a/listings/listing_7.3.cpp +++ b/listings/listing_7.3.cpp @@ -24,8 +24,8 @@ class lock_free_stack std::shared_ptr pop() { if (!head_) { return std::shared_ptr(); } - node* old_head=head.load(); - while(!head.compare_exchange_weak(old_head,old_head->next)); - return old_head->data; + node* original_head=head.load(); + while(!head.compare_exchange_weak(original_head,original_head->next)); + return original_head->data; } };