Skip to content

Commit 4810163

Browse files
committed
cas: clang-tidy sweep, remaining checks (arm_tidy, T13 batch 2)
CI report: https://altinity-build-artifacts.s3.amazonaws.com/json.html?PR=2073&sha=aeb13b24394023fa8cd9d310d4cbcbc308380af1&name_0=PR&name_1=Build+(arm_tidy) PR: #2073 Semantics-preserving conformance for the remaining flagged classes: readability-container-contains, readability-isolate-declaration, google-runtime-int (AWS SDK retry-API overrides keep `long` with targeted NOLINT — the override contract owns the type), readability-duplicate-include, cppcoreguidelines-init-variables, cert-msc, modernize-raw-string-literal, modernize-use-starts-ends-with, bugprone-empty-catch (comments only — no new behavior), googletest naming, bugprone-argument-comment, bugprone-optional-value-conversion, bugprone-misplaced-widening-cast (CasTypes.h site audited: not a real precision bug — the value is range-validated to 0-5; cast made explicit without value change). CasRefCowMap's own `contains` keeps its `find` with NOLINT (self-recursion). Bulk edits by codex (gpt-5.6-luna) per the T13 brief (.superpowers/sdd/task-13-batch2-report.md); one over-removed include (PartFolderAccess.h) restored and verification by Claude. Battery 919/919. Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
1 parent 24bd437 commit 4810163

50 files changed

Lines changed: 177 additions & 126 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Backend/CasInMemoryBackend.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ HeadResult InMemoryBackend::head(const String & key)
131131
PutResult InMemoryBackend::putIfAbsent(const String & key, const String & bytes, const ObjectMeta & meta)
132132
{
133133
std::lock_guard lock(mutex_);
134-
if (store_.count(key))
134+
if (store_.contains(key))
135135
return {PutOutcome::PreconditionFailed, {}};
136136

137137
Token t = mintToken();
@@ -276,7 +276,7 @@ PutResult InMemoryBackend::promoteStaged(const String & staging_key, const Strin
276276
"InMemoryBackend::promoteStaged: staging object {} is absent", staging_key);
277277

278278
/// Write-once: a present destination is the "lost the race" signal, not an overwrite.
279-
if (store_.count(blob_key))
279+
if (store_.contains(blob_key))
280280
return {PutOutcome::PreconditionFailed, {}};
281281

282282
/// Server-side copy: the destination bytes ARE the staging bytes; a fresh monotone token stands in
@@ -332,7 +332,7 @@ ListPage InMemoryBackend::list(const String & prefix, const String & cursor, siz
332332
size_t count = 0;
333333
while (it != store_.end() && count < limit)
334334
{
335-
if (it->first.substr(0, prefix.size()) != prefix)
335+
if (!it->first.starts_with(prefix))
336336
break;
337337

338338
ListedKey lk;
@@ -345,7 +345,7 @@ ListPage InMemoryBackend::list(const String & prefix, const String & cursor, siz
345345
}
346346

347347
// Set next_cursor if there are more keys in this prefix
348-
if (!page.keys.empty() && it != store_.end() && it->first.substr(0, prefix.size()) == prefix)
348+
if (!page.keys.empty() && it != store_.end() && it->first.starts_with(prefix))
349349
page.next_cursor = page.keys.back().key;
350350

351351
return page;

src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Backend/CasObjectStorageBackend.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,13 @@ namespace DB::Cas
4545
/// not this SDK loop. Every consultation is counted because it proves that the SDK considered the
4646
/// first attempt inconclusive or failed; otherwise the retry-consultation metric would remain zero
4747
/// even when the SDK reached this decision point.
48-
bool detail::SingleAttemptRetryStrategy::ShouldRetry(const Aws::Client::AWSError<Aws::Client::CoreErrors> &, long) const
48+
bool detail::SingleAttemptRetryStrategy::ShouldRetry(const Aws::Client::AWSError<Aws::Client::CoreErrors> &, long) const // NOLINT(google-runtime-int): AWS SDK virtual API requires `long`.
4949
{
5050
recordConditionalWriteSdkRetryConsidered();
5151
return false;
5252
}
5353

54-
long detail::SingleAttemptRetryStrategy::CalculateDelayBeforeNextRetry(const Aws::Client::AWSError<Aws::Client::CoreErrors> &, long) const
54+
long detail::SingleAttemptRetryStrategy::CalculateDelayBeforeNextRetry(const Aws::Client::AWSError<Aws::Client::CoreErrors> &, long) const // NOLINT(google-runtime-int): AWS SDK virtual API requires `long`.
5555
{
5656
/// AWSClient prepares the delay before calling `ShouldRetry`, so this method is reached even
5757
/// though no retry will be made. Returning zero avoids needless backoff computation; it is never
@@ -1027,7 +1027,7 @@ ListPage ObjectStorageBackend::list(const String & prefix, const String & cursor
10271027
all.reserve(children.size());
10281028
for (const auto & child : children)
10291029
{
1030-
if (child->relative_path.substr(0, physical_prefix.size()) != physical_prefix)
1030+
if (!child->relative_path.starts_with(physical_prefix))
10311031
continue;
10321032
ListedKey lk;
10331033
lk.key = child->relative_path.substr(strip.size());
@@ -1064,7 +1064,7 @@ ListPage ObjectStorageBackend::list(const String & prefix, const String & cursor
10641064
for (; it->isValid(); it->next())
10651065
{
10661066
const auto child = it->current();
1067-
if (child->relative_path.substr(0, physical_prefix.size()) != physical_prefix)
1067+
if (!child->relative_path.starts_with(physical_prefix))
10681068
continue;
10691069

10701070
ListedKey lk;

src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Backend/CasObjectStorageBackend.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,10 @@ class SingleAttemptRetryStrategy final : public Aws::Client::RetryStrategy
4040
{
4141
public:
4242
/// Record that the SDK considered retrying, then refuse the retry.
43-
bool ShouldRetry(const Aws::Client::AWSError<Aws::Client::CoreErrors> &, long) const override;
43+
bool ShouldRetry(const Aws::Client::AWSError<Aws::Client::CoreErrors> &, long) const override; // NOLINT(google-runtime-int): AWS SDK virtual API requires `long`.
4444
/// Return no delay; `ShouldRetry` rejects the attempt immediately afterward.
45-
long CalculateDelayBeforeNextRetry(const Aws::Client::AWSError<Aws::Client::CoreErrors> &, long) const override;
46-
long GetMaxAttempts() const override { return 1; }
45+
long CalculateDelayBeforeNextRetry(const Aws::Client::AWSError<Aws::Client::CoreErrors> &, long) const override; // NOLINT(google-runtime-int): AWS SDK virtual API requires `long`.
46+
long GetMaxAttempts() const override { return 1; } // NOLINT(google-runtime-int): AWS SDK virtual API requires `long`.
4747
};
4848
}
4949
#endif

src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Backend/CasRequestControl.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,7 @@ CasWriteOutcome CasRequestController::putIfAbsentControlled(
330330

331331
/// The committed incarnation's token, filled by whichever leg proves Committed below.
332332
Token committed_token;
333-
CasWriteOutcome attempt_outcome;
333+
CasWriteOutcome attempt_outcome{};
334334
try
335335
{
336336
const PutResult put = backend->putIfAbsent(key_s, bytes_s);

src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/ContentAddressedMetadataStorage.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ void ContentAddressedMetadataStorage::runOneGcRoundForTest()
225225
/// scheduler per call would acquire the lease on the first call and then back off forever
226226
/// ("incumbent alive" - its own previous incarnation). Recreating the scheduler for every call
227227
/// would therefore make every round after the first a silent no-op.
228-
Cas::CasGcScheduler * sched;
228+
Cas::CasGcScheduler * sched = nullptr;
229229
{
230230
std::lock_guard lock(gc_scheduler_mutex);
231231
if (!gc_scheduler)
@@ -356,7 +356,7 @@ Cas::RoundReport ContentAddressedMetadataStorage::runGarbageCollectionRoundNow()
356356
"Garbage collection is not enabled on this content-addressed disk");
357357
/// Mirror runOneGcRoundForTest: a STABLE scheduler instance across calls (the lease's
358358
/// observation-window steal protocol compares consecutive observations of the same gc_id).
359-
Cas::CasGcScheduler * sched;
359+
Cas::CasGcScheduler * sched = nullptr;
360360
{
361361
std::lock_guard lock(gc_scheduler_mutex);
362362
if (!gc_scheduler)

src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/ContentAddressedTransaction.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ void ContentAddressedTransaction::uploadPendingBlobs(PartStaging & st)
246246

247247
for (const auto & pb : st.pending_blobs) /// pool writes — uploads + 412/HEAD/resurrect
248248
{
249-
if (!referenced_hashes.count(pb.ref))
249+
if (!referenced_hashes.contains(pb.ref))
250250
continue; /// The entry was removed by unlinkFile/replaceFile; skip this orphan.
251251
/// Each pending blob
252252
/// is promoted through the SAME condemn/resurrect gate in `PartWriteTxn::putBlob`. Only the upload

src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Formats/CasLayout.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ std::optional<BlobRef> Layout::parseBlobKey(std::string_view key) const
2525
rest.remove_suffix(kMetaSuffix.size());
2626

2727
const String blobs_root = blobsPrefix(); /// "<prefix>/blobs/"
28-
if (rest.size() <= blobs_root.size() || rest.substr(0, blobs_root.size()) != blobs_root)
28+
if (rest.size() <= blobs_root.size() || !rest.starts_with(blobs_root))
2929
return std::nullopt;
3030
rest.remove_prefix(blobs_root.size());
3131

src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Formats/CasLayout.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ class Layout
166166
if (ns_part.empty())
167167
return std::nullopt;
168168

169-
RefObjectKind kind;
169+
RefObjectKind kind{};
170170
if (kind_seg == "_cleanup")
171171
kind = RefObjectKind::Cleanup;
172172
else if (kind_seg == "_log")

src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Formats/CasPartManifestFormat.cpp

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,9 @@ PartManifest decodePartManifest(std::string_view data)
139139
const String meta = readLine(in, line_cap, "cas_part_manifest");
140140
ReadBufferFromMemory mm(meta.data(), meta.size());
141141
JsonObjectReader r(mm, KeyStrictness::Tolerant, "cas_part_manifest");
142-
std::optional<uint64_t> me, mb, mo;
142+
std::optional<uint64_t> me;
143+
std::optional<uint64_t> mb;
144+
std::optional<uint64_t> mo;
143145
std::optional<String> ns;
144146
std::optional<UInt128> pd;
145147
String key;
@@ -195,8 +197,11 @@ PartManifest decodePartManifest(std::string_view data)
195197
throw Exception(ErrorCodes::CORRUPTED_DATA, "PartManifest: record must start with \"p\"");
196198
ManifestEntry e;
197199
e.path = r.readString();
198-
std::optional<String> pm, ha, h;
199-
std::optional<uint64_t> sz, il;
200+
std::optional<String> pm;
201+
std::optional<String> ha;
202+
std::optional<String> h;
203+
std::optional<uint64_t> sz;
204+
std::optional<uint64_t> il;
200205
while (r.nextKey(key))
201206
{
202207
if (key == "pm") pm = r.readString();

src/Disks/DiskObjectStorage/MetadataStorages/ContentAddressed/Formats/CasRecordStreamFormat.cpp

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -252,9 +252,17 @@ bool SourceEdgeRunReader::next(SourceEdgeRecord & rec)
252252
}
253253

254254
SourceEdgeRecord out;
255-
String b, tv;
256-
bool have_b = false, have_s = false, have_m = false;
257-
bool have_pend = false, have_tt = false, have_tv = false, have_sz = false, have_cr = false, have_mc = false;
255+
String b;
256+
String tv;
257+
bool have_b = false;
258+
bool have_s = false;
259+
bool have_m = false;
260+
bool have_pend = false;
261+
bool have_tt = false;
262+
bool have_tv = false;
263+
bool have_sz = false;
264+
bool have_cr = false;
265+
bool have_mc = false;
258266
TokenType tt{};
259267
do
260268
{

0 commit comments

Comments
 (0)