Fix various clang-tidy lints

* some things that can throw are marked noexcept
  yet the linter seems to think not. Maybe they can't throw in practice.
  I would rather not have the UB possibility in pretty obvious cold
  paths.
* various default-case-missing complaints
* a fair pile of casts from integer to character, which are in fact
  deliberate.
* an instance of <https://clang.llvm.org/extra/clang-tidy/checks/bugprone/move-forwarding-reference.html>
* bugprone-not-null-terminated-result on handing a string to curl in
  chunks of bytes. our usage is fine.
* reassigning a unique_ptr by CRIMES instead of using release(), then
  using release() and ignoring the result. wild. let's use release() for
  its intended purpose.

Change-Id: Ic3e7affef12383576213a8a7c8145c27e662513d
This commit is contained in:
jade 2024-03-29 20:26:38 -07:00
parent 99f159c536
commit 1fa6a3e335
9 changed files with 36 additions and 27 deletions

View file

@ -157,7 +157,7 @@ size_t StoreReferences::size() const
return (self ? 1 : 0) + others.size(); return (self ? 1 : 0) + others.size();
} }
ContentAddressWithReferences ContentAddressWithReferences::withoutRefs(const ContentAddress & ca) noexcept ContentAddressWithReferences ContentAddressWithReferences::withoutRefs(const ContentAddress & ca)
{ {
return std::visit(overloaded { return std::visit(overloaded {
[&](const TextIngestionMethod &) -> ContentAddressWithReferences { [&](const TextIngestionMethod &) -> ContentAddressWithReferences {
@ -177,7 +177,7 @@ ContentAddressWithReferences ContentAddressWithReferences::withoutRefs(const Con
} }
std::optional<ContentAddressWithReferences> ContentAddressWithReferences::fromPartsOpt( std::optional<ContentAddressWithReferences> ContentAddressWithReferences::fromPartsOpt(
ContentAddressMethod method, Hash hash, StoreReferences refs) noexcept ContentAddressMethod method, Hash hash, StoreReferences refs)
{ {
return std::visit(overloaded { return std::visit(overloaded {
[&](TextIngestionMethod _) -> std::optional<ContentAddressWithReferences> { [&](TextIngestionMethod _) -> std::optional<ContentAddressWithReferences> {

View file

@ -255,7 +255,7 @@ struct ContentAddressWithReferences
* Create a `ContentAddressWithReferences` from a mere * Create a `ContentAddressWithReferences` from a mere
* `ContentAddress`, by claiming no references. * `ContentAddress`, by claiming no references.
*/ */
static ContentAddressWithReferences withoutRefs(const ContentAddress &) noexcept; static ContentAddressWithReferences withoutRefs(const ContentAddress &);
/** /**
* Create a `ContentAddressWithReferences` from 3 parts: * Create a `ContentAddressWithReferences` from 3 parts:
@ -270,7 +270,7 @@ struct ContentAddressWithReferences
* returns for invalid combinations. * returns for invalid combinations.
*/ */
static std::optional<ContentAddressWithReferences> fromPartsOpt( static std::optional<ContentAddressWithReferences> fromPartsOpt(
ContentAddressMethod method, Hash hash, StoreReferences refs) noexcept; ContentAddressMethod method, Hash hash, StoreReferences refs);
ContentAddressMethod getMethod() const; ContentAddressMethod getMethod() const;

View file

@ -142,7 +142,7 @@ struct curlFileTransfer : public FileTransfer
template<class T> template<class T>
void fail(T && e) void fail(T && e)
{ {
failEx(std::make_exception_ptr(std::move(e))); failEx(std::make_exception_ptr(std::forward<T>(e)));
} }
LambdaSink finalSink; LambdaSink finalSink;
@ -270,6 +270,9 @@ struct curlFileTransfer : public FileTransfer
return 0; return 0;
auto count = std::min(size * nitems, request.data->length() - readOffset); auto count = std::min(size * nitems, request.data->length() - readOffset);
assert(count); assert(count);
// Lint: this is turning a string into a byte array to hand to
// curl, which is fine.
// NOLINTNEXTLINE(bugprone-not-null-terminated-result)
memcpy(buffer, request.data->data() + readOffset, count); memcpy(buffer, request.data->data() + readOffset, count);
readOffset += count; readOffset += count;
return count; return count;

View file

@ -1315,10 +1315,12 @@ StorePath LocalStore::addToStoreFromDump(Source & source0, std::string_view name
auto oldSize = dump.size(); auto oldSize = dump.size();
constexpr size_t chunkSize = 65536; constexpr size_t chunkSize = 65536;
auto want = std::min(chunkSize, settings.narBufferSize - oldSize); auto want = std::min(chunkSize, settings.narBufferSize - oldSize);
if (auto tmp = realloc(dumpBuffer.get(), oldSize + want)) {
dumpBuffer.release(); auto *toRealloc = dumpBuffer.release();
dumpBuffer.reset((char*) tmp); if (auto realloced = realloc(toRealloc, oldSize + want)) {
dumpBuffer.reset((char*) realloced);
} else { } else {
free(toRealloc);
throw std::bad_alloc(); throw std::bad_alloc();
} }
auto got = 0; auto got = 0;

View file

@ -69,8 +69,8 @@ public:
case lvlWarn: c = '4'; break; case lvlWarn: c = '4'; break;
case lvlNotice: case lvlInfo: c = '5'; break; case lvlNotice: case lvlInfo: c = '5'; break;
case lvlTalkative: case lvlChatty: c = '6'; break; case lvlTalkative: case lvlChatty: c = '6'; break;
case lvlDebug: case lvlVomit: c = '7'; break; case lvlDebug: case lvlVomit:
default: c = '7'; break; // should not happen, and missing enum case is reported by -Werror=switch-enum default: c = '7'; break; // default case should not happen, and missing enum case is reported by -Werror=switch-enum
} }
prefix = std::string("<") + c + ">"; prefix = std::string("<") + c + ">";
} }

View file

@ -50,6 +50,7 @@ InputAccessor::DirEntries SourcePath::readDirectory() const
case DT_REG: type = InputAccessor::Type::tRegular; break; case DT_REG: type = InputAccessor::Type::tRegular; break;
case DT_LNK: type = InputAccessor::Type::tSymlink; break; case DT_LNK: type = InputAccessor::Type::tSymlink; break;
case DT_DIR: type = InputAccessor::Type::tDirectory; break; case DT_DIR: type = InputAccessor::Type::tDirectory; break;
default: break; // unknown type
} }
res.emplace(entry.name, type); res.emplace(entry.name, type);
} }

View file

@ -32,13 +32,13 @@ Gen<StorePathName> Arbitrary<StorePathName>::arbitrary()
for (size_t c = 0; c < len; ++c) { for (size_t c = 0; c < len; ++c) {
switch (auto i = *gen::inRange<uint8_t>(0, 10 + 2 * 26 + 6)) { switch (auto i = *gen::inRange<uint8_t>(0, 10 + 2 * 26 + 6)) {
case 0 ... 9: case 0 ... 9:
pre += '0' + i; pre += static_cast<uint8_t>('0' + i);
break; break;
case 10 ... 35: case 10 ... 35:
pre += 'A' + (i - 10); pre += static_cast<uint8_t>('A' + (i - 10));
break; break;
case 36 ... 61: case 36 ... 61:
pre += 'a' + (i - 36); pre += static_cast<uint8_t>('a' + (i - 36));
break; break;
case 62: case 62:
pre += '+'; pre += '+';

View file

@ -170,20 +170,22 @@ TEST(ExtendedOutputsSpec, many_carrot) {
} }
#define TEST_JSON(TYPE, NAME, STR, VAL) \ #define TEST_JSON(TYPE, NAME, STR, VAL) \
\ \
TEST(TYPE, NAME ## _to_json) { \ TEST(TYPE, NAME ## _to_json) { \
using nlohmann::literals::operator "" _json; \ using nlohmann::literals::operator "" _json; \
ASSERT_EQ( \ ASSERT_EQ( \
STR ## _json, \ STR ## _json, \
((nlohmann::json) TYPE { VAL })); \ /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \
} \ ((nlohmann::json) TYPE { VAL })); \
\ } \
TEST(TYPE, NAME ## _from_json) { \ \
using nlohmann::literals::operator "" _json; \ TEST(TYPE, NAME ## _from_json) { \
ASSERT_EQ( \ using nlohmann::literals::operator "" _json; \
TYPE { VAL }, \ ASSERT_EQ( \
(STR ## _json).get<TYPE>()); \ /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \
TYPE { VAL }, \
(STR ## _json).get<TYPE>()); \
} }
TEST_JSON(OutputsSpec, all, R"(["*"])", OutputsSpec::All { }) TEST_JSON(OutputsSpec, all, R"(["*"])", OutputsSpec::All { })

View file

@ -26,6 +26,7 @@ void TerminalCodeEater::feed(char c, std::function<void(char)> on_char)
// Just eat \r, since it is part of clearing a line // Just eat \r, since it is part of clearing a line
case '\r': case '\r':
return; return;
default: break;
} }
if constexpr (DEBUG_EATER) { if constexpr (DEBUG_EATER) {
std::cerr << "eater uneat" << MaybeHexEscapedChar{c} << "\n"; std::cerr << "eater uneat" << MaybeHexEscapedChar{c} << "\n";