Skip to content

fix(storage): avoid crashes after move#7045

Merged
coryan merged 1 commit intogoogleapis:mainfrom
coryan:fix-storage-avoid-crashes-after-move-from
Jul 22, 2021
Merged

fix(storage): avoid crashes after move#7045
coryan merged 1 commit intogoogleapis:mainfrom
coryan:fix-storage-avoid-crashes-after-move-from

Conversation

@coryan
Copy link
Copy Markdown
Contributor

@coryan coryan commented Jul 22, 2021

The storage::Object*Stream classes should not crash when used after
moved-from. They do not guarantee what value they hold, but crashing on
functions that just query their state is unexpected. Note that we have
not defined what pre-conditions are required by each function, so we
could have decided to make it UB to call these functions. That seems too
hostile when "not crash but returns some undefined value" is less
hostile and about as easy to implement.

Fixes #7044


This change is Reviewable

The `storage::Object*Stream` classes should not crash when used after
moved-from. They do not guarantee what value they hold, but crashing on
functions that just query their state is unexpected. Note that we have
not defined what pre-conditions are required by each function, so we
could have decided to make it UB to call these functions. That seems too
hostile when "not crash but returns some undefined value" is less
hostile and about as easy to implement.
@product-auto-label product-auto-label Bot added the api: storage Issues related to the Cloud Storage API. label Jul 22, 2021
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 22, 2021
@google-cloud-cpp-bot
Copy link
Copy Markdown
Contributor

Google Cloud Build Logs
For commit: 3bff109b08c065eddae20f0503b9eb66c2f7718a

ℹ️ NOTE: Kokoro logs are linked from "Details" below.

@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 22, 2021

Codecov Report

Merging #7045 (3bff109) into main (942c723) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #7045   +/-   ##
=======================================
  Coverage   94.47%   94.48%           
=======================================
  Files        1304     1304           
  Lines      112300   112319   +19     
=======================================
+ Hits       106100   106124   +24     
+ Misses       6200     6195    -5     
Impacted Files Coverage Δ
google/cloud/storage/object_read_stream.cc 75.00% <100.00%> (+8.33%) ⬆️
google/cloud/storage/object_stream_test.cc 100.00% <100.00%> (ø)
google/cloud/storage/object_write_stream.cc 98.07% <100.00%> (+8.94%) ⬆️
google/cloud/pubsub/subscriber_connection_test.cc 97.20% <0.00%> (-0.70%) ⬇️
...sub/internal/batching_publisher_connection_test.cc 97.60% <0.00%> (-0.21%) ⬇️
google/cloud/pubsub/samples/samples.cc 91.67% <0.00%> (-0.08%) ⬇️
...bigtable/examples/bigtable_hello_instance_admin.cc 83.51% <0.00%> (+2.19%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 942c723...3bff109. Read the comment docs.

@coryan coryan marked this pull request as ready for review July 22, 2021 22:12
@coryan coryan requested a review from a team July 22, 2021 22:12
@coryan coryan merged commit 4f94bc1 into googleapis:main Jul 22, 2021
@coryan coryan deleted the fix-storage-avoid-crashes-after-move-from branch July 22, 2021 23:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: storage Issues related to the Cloud Storage API. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

storage::ObjectWriteStream left in unusable state after Suspend() or moves

4 participants