tree-wide: fix a pile of lints

This:
- Converts a bunch of C style casts into C++ casts.
- Removes some very silly pointer subtraction code (which is no more or
  less busted on i686 than it began)
- Fixes some "technically UB" that never had to be UB in the first
  place.
- Makes finally follow the noexcept status of the inner function. Maybe
  in the future we should ban the function from not being noexcept, but
  that is not today.
- Makes various locally-used exceptions inherit from std::exception.

Change-Id: I22e66972602604989b5e494fd940b93e0e6e9297
This commit is contained in:
jade 2024-08-04 20:19:58 -07:00
parent 370ac940dd
commit e34833c025
27 changed files with 74 additions and 69 deletions

View file

@ -96,7 +96,7 @@ static int listPossibleCallback(char * s, char *** avp)
return p;
};
vp = check((char **) malloc(possible.size() * sizeof(char *)));
vp = check(static_cast<char **>(malloc(possible.size() * sizeof(char *))));
for (auto & p : possible)
vp[ac++] = check(strdup(p.c_str()));

View file

@ -21,7 +21,6 @@
#include "flake/flakeref.hh"
#include <algorithm>
#include <chrono>
#include <iostream>
#include <sstream>
#include <cstring>
@ -146,7 +145,7 @@ bool Value::isTrivial() const
&& internalType != tPrimOpApp
&& (internalType != tThunk
|| (dynamic_cast<ExprAttrs *>(thunk.expr)
&& ((ExprAttrs *) thunk.expr)->dynamicAttrs.empty())
&& (static_cast<ExprAttrs *>(thunk.expr))->dynamicAttrs.empty())
|| dynamic_cast<ExprLambda *>(thunk.expr)
|| dynamic_cast<ExprList *>(thunk.expr));
}

View file

@ -346,7 +346,7 @@ void prim_importNative(EvalState & state, const PosIdx pos, Value * * args, Valu
state.error<EvalError>("could not open '%1%': %2%", path, dlerror()).debugThrow();
dlerror();
ValueInitializer func = (ValueInitializer) dlsym(handle, sym.c_str());
ValueInitializer func = reinterpret_cast<ValueInitializer>(dlsym(handle, sym.c_str()));
if(!func) {
char *message = dlerror();
if (message)
@ -2439,6 +2439,8 @@ static void prim_attrValues(EvalState & state, const PosIdx pos, Value * * args,
state.mkList(v, args[0]->attrs->size());
// FIXME: this is incredibly evil, *why*
// NOLINTBEGIN(cppcoreguidelines-pro-type-cstyle-cast)
unsigned int n = 0;
for (auto & i : *args[0]->attrs)
v.listElems()[n++] = (Value *) &i;
@ -2452,6 +2454,7 @@ static void prim_attrValues(EvalState & state, const PosIdx pos, Value * * args,
for (unsigned int i = 0; i < n; ++i)
v.listElems()[i] = ((Attr *) v.listElems()[i])->value;
// NOLINTEND(cppcoreguidelines-pro-type-cstyle-cast)
}
static RegisterPrimOp primop_attrValues({

View file

@ -2,8 +2,6 @@
#include "shared.hh"
#include <cstring>
#include <cstddef>
#include <cstdlib>
#include <unistd.h>
#include <signal.h>
@ -17,17 +15,17 @@ static void sigsegvHandler(int signo, siginfo_t * info, void * ctx)
the stack pointer. Unfortunately, getting the stack pointer is
not portable. */
bool haveSP = true;
char * sp = 0;
int64_t sp = 0;
#if defined(__x86_64__) && defined(REG_RSP)
sp = (char *) ((ucontext_t *) ctx)->uc_mcontext.gregs[REG_RSP];
sp = static_cast<ucontext_t *>(ctx)->uc_mcontext.gregs[REG_RSP];
#elif defined(REG_ESP)
sp = (char *) ((ucontext_t *) ctx)->uc_mcontext.gregs[REG_ESP];
sp = static_cast<ucontext_t *>(ctx)->uc_mcontext.gregs[REG_ESP];
#else
haveSP = false;
#endif
if (haveSP) {
ptrdiff_t diff = (char *) info->si_addr - sp;
int64_t diff = int64_t(info->si_addr) - sp;
if (diff < 0) diff = -diff;
if (diff < 4096) {
nix::stackOverflowHandler(info, ctx);

View file

@ -3,17 +3,15 @@
#include "compression.hh"
#include "derivations.hh"
#include "fs-accessor.hh"
#include "globals.hh"
#include "nar-info.hh"
#include "sync.hh"
#include "remote-fs-accessor.hh"
#include "nar-info-disk-cache.hh"
#include "nar-info-disk-cache.hh" // IWYU pragma: keep
#include "nar-accessor.hh"
#include "thread-pool.hh"
#include "signals.hh"
#include <chrono>
#include <future>
#include <regex>
#include <fstream>
#include <sstream>
@ -480,7 +478,8 @@ void BinaryCacheStore::addSignatures(const StorePath & storePath, const StringSe
when addSignatures() is called sequentially on a path, because
S3 might return an outdated cached version. */
auto narInfo = make_ref<NarInfo>((NarInfo &) *queryPathInfo(storePath));
// downcast: BinaryCacheStore always returns NarInfo from queryPathInfoUncached, making it sound
auto narInfo = make_ref<NarInfo>(dynamic_cast<NarInfo const &>(*queryPathInfo(storePath)));
narInfo->sigs.insert(sigs.begin(), sigs.end());

View file

@ -1226,7 +1226,7 @@ void LocalDerivationGoal::startDaemon()
socklen_t remoteAddrLen = sizeof(remoteAddr);
AutoCloseFD remote{accept(daemonSocket.get(),
(struct sockaddr *) &remoteAddr, &remoteAddrLen)};
reinterpret_cast<struct sockaddr *>(&remoteAddr), &remoteAddrLen)};
if (!remote) {
if (errno == EINTR || errno == EAGAIN) continue;
if (errno == EINVAL || errno == ECONNABORTED) break;

View file

@ -4,7 +4,7 @@
#include "drv-output-substitution-goal.hh"
#include "local-derivation-goal.hh"
#include "signals.hh"
#include "hook-instance.hh"
#include "hook-instance.hh" // IWYU pragma: keep
#include <poll.h>
@ -529,7 +529,7 @@ void Worker::waitForInput()
} else {
printMsg(lvlVomit, "%1%: read %2% bytes",
goal->getName(), rd);
std::string_view data((char *) buffer.data(), rd);
std::string_view data(reinterpret_cast<char *>(buffer.data()), rd);
j->lastOutput = after;
handleWorkResult(goal, goal->handleChildOutput(k, data));
}

View file

@ -44,16 +44,16 @@ std::string SecretKey::signDetached(std::string_view data) const
{
unsigned char sig[crypto_sign_BYTES];
unsigned long long sigLen;
crypto_sign_detached(sig, &sigLen, (unsigned char *) data.data(), data.size(),
(unsigned char *) key.data());
return name + ":" + base64Encode(std::string((char *) sig, sigLen));
crypto_sign_detached(sig, &sigLen, reinterpret_cast<const unsigned char *>(data.data()), data.size(),
reinterpret_cast<const unsigned char *>(key.data()));
return name + ":" + base64Encode(std::string(reinterpret_cast<char *>(sig), sigLen));
}
PublicKey SecretKey::toPublicKey() const
{
unsigned char pk[crypto_sign_PUBLICKEYBYTES];
crypto_sign_ed25519_sk_to_pk(pk, (unsigned char *) key.data());
return PublicKey(name, std::string((char *) pk, crypto_sign_PUBLICKEYBYTES));
crypto_sign_ed25519_sk_to_pk(pk, reinterpret_cast<const unsigned char *>(key.data()));
return PublicKey(name, std::string(reinterpret_cast<char *>(pk), crypto_sign_PUBLICKEYBYTES));
}
SecretKey SecretKey::generate(std::string_view name)
@ -63,7 +63,7 @@ SecretKey SecretKey::generate(std::string_view name)
if (crypto_sign_keypair(pk, sk) != 0)
throw Error("key generation failed");
return SecretKey(name, std::string((char *) sk, crypto_sign_SECRETKEYBYTES));
return SecretKey(name, std::string(reinterpret_cast<char *>(sk), crypto_sign_SECRETKEYBYTES));
}
PublicKey::PublicKey(std::string_view s)
@ -85,9 +85,9 @@ bool verifyDetached(const std::string & data, const std::string & sig,
if (sig2.size() != crypto_sign_BYTES)
throw Error("signature is not valid");
return crypto_sign_verify_detached((unsigned char *) sig2.data(),
(unsigned char *) data.data(), data.size(),
(unsigned char *) key->second.key.data()) == 0;
return crypto_sign_verify_detached(reinterpret_cast<unsigned char *>(sig2.data()),
reinterpret_cast<const unsigned char *>(data.data()), data.size(),
reinterpret_cast<const unsigned char *>(key->second.key.data())) == 0;
}
PublicKeys getDefaultPublicKeys()

View file

@ -453,7 +453,11 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
hashAlgo = parseHashType(hashAlgoRaw);
}
GeneratorSource dumpSource{[&]() -> WireFormatGenerator {
// Note to future maintainers: do *not* inline this into the
// generator statement as the lambda itself needs to live to the
// end of the generator's lifetime and is otherwise a UAF.
// NOLINTNEXTLINE(cppcoreguidelines-avoid-capturing-lambda-coroutines): does not outlive the outer function
auto g = [&]() -> WireFormatGenerator {
if (method == FileIngestionMethod::Recursive) {
/* We parse the NAR dump through into `saved` unmodified,
so why all this extra work? We still parse the NAR so
@ -489,7 +493,8 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
}
co_yield std::move(file->contents);
}
}()};
};
GeneratorSource dumpSource{g()};
logger->startWork();
auto path = store->addToStoreFromDump(dumpSource, baseName, method, hashAlgo);
logger->stopWork();

View file

@ -5,7 +5,6 @@
#include "s3.hh"
#include "signals.hh"
#include "compression.hh"
#include "finally.hh"
#if ENABLE_S3
#include <aws/core/client/ClientConfiguration.h>
@ -143,9 +142,9 @@ struct curlFileTransfer : public FileTransfer
if (successfulStatuses.count(getHTTPStatus()) && this->dataCallback) {
writtenToSink += realSize;
dataCallback(*this, {(const char *) contents, realSize});
dataCallback(*this, {static_cast<const char *>(contents), realSize});
} else {
this->result.data.append((const char *) contents, realSize);
this->result.data.append(static_cast<const char *>(contents), realSize);
}
return realSize;
@ -157,13 +156,13 @@ struct curlFileTransfer : public FileTransfer
static size_t writeCallbackWrapper(void * contents, size_t size, size_t nmemb, void * userp)
{
return ((TransferItem *) userp)->writeCallback(contents, size, nmemb);
return static_cast<TransferItem *>(userp)->writeCallback(contents, size, nmemb);
}
size_t headerCallback(void * contents, size_t size, size_t nmemb)
{
size_t realSize = size * nmemb;
std::string line((char *) contents, realSize);
std::string line(static_cast<char *>(contents), realSize);
printMsg(lvlVomit, "got header for '%s': %s", request.uri, trim(line));
static std::regex statusLine("HTTP/[^ ]+ +[0-9]+(.*)", std::regex::extended | std::regex::icase);
@ -204,7 +203,7 @@ struct curlFileTransfer : public FileTransfer
static size_t headerCallbackWrapper(void * contents, size_t size, size_t nmemb, void * userp)
{
return ((TransferItem *) userp)->headerCallback(contents, size, nmemb);
return static_cast<TransferItem *>(userp)->headerCallback(contents, size, nmemb);
}
int progressCallback(double dltotal, double dlnow)
@ -219,7 +218,7 @@ struct curlFileTransfer : public FileTransfer
static int progressCallbackWrapper(void * userp, double dltotal, double dlnow, double ultotal, double ulnow)
{
return ((TransferItem *) userp)->progressCallback(dltotal, dlnow);
return static_cast<TransferItem *>(userp)->progressCallback(dltotal, dlnow);
}
static int debugCallback(CURL * handle, curl_infotype type, char * data, size_t size, void * userptr)
@ -246,7 +245,7 @@ struct curlFileTransfer : public FileTransfer
static size_t readCallbackWrapper(char *buffer, size_t size, size_t nitems, void * userp)
{
return ((TransferItem *) userp)->readCallback(buffer, size, nitems);
return static_cast<TransferItem *>(userp)->readCallback(buffer, size, nitems);
}
void init()

View file

@ -1318,7 +1318,7 @@ StorePath LocalStore::addToStoreFromDump(Source & source0, std::string_view name
auto *toRealloc = dumpBuffer.release();
if (auto realloced = realloc(toRealloc, oldSize + want)) {
dumpBuffer.reset((char*) realloced);
dumpBuffer.reset(static_cast<char *>(realloced));
} else {
free(toRealloc);
throw std::bad_alloc();

View file

@ -119,8 +119,8 @@ private:
static ssize_t callback_write(struct archive * archive, void * _self, const void * buffer, size_t length)
{
auto self = (ArchiveCompressionSink *) _self;
self->nextSink({(const char *) buffer, length});
auto self = static_cast<ArchiveCompressionSink *>(_self);
self->nextSink({static_cast<const char *>(buffer), length});
return length;
}
};
@ -160,7 +160,7 @@ struct BrotliDecompressionSource : Source
size_t read(char * data, size_t len) override
{
uint8_t * out = (uint8_t *) data;
uint8_t * out = reinterpret_cast<uint8_t *>(data);
const auto * begin = out;
while (len && !BrotliDecoderIsFinished(state.get())) {
@ -172,7 +172,7 @@ struct BrotliDecompressionSource : Source
} catch (EndOfFile &) {
break;
}
next_in = (const uint8_t *) buf.get();
next_in = reinterpret_cast<const uint8_t *>(buf.get());
}
if (!BrotliDecoderDecompressStream(
@ -238,7 +238,7 @@ struct BrotliCompressionSink : ChunkedCompressionSink
void writeInternal(std::string_view data) override
{
auto next_in = (const uint8_t *) data.data();
auto next_in = reinterpret_cast<const uint8_t *>(data.data());
size_t avail_in = data.size();
uint8_t * next_out = outbuf;
size_t avail_out = sizeof(outbuf);
@ -254,7 +254,7 @@ struct BrotliCompressionSink : ChunkedCompressionSink
throw CompressionError("error while compressing brotli compression");
if (avail_out < sizeof(outbuf) || avail_in == 0) {
nextSink({(const char *) outbuf, sizeof(outbuf) - avail_out});
nextSink({reinterpret_cast<const char *>(outbuf), sizeof(outbuf) - avail_out});
next_out = outbuf;
avail_out = sizeof(outbuf);
}

View file

@ -115,7 +115,7 @@ Generator<Bytes> drainFDSource(int fd, bool block)
throw SysError("reading from file");
}
else if (rd == 0) break;
else co_yield std::span{(char *) buf.data(), (size_t) rd};
else co_yield std::span{reinterpret_cast<char *>(buf.data()), (size_t) rd};
}
}

View file

@ -567,9 +567,8 @@ Path createTempDir(const Path & tmpRoot, const Path & prefix,
std::pair<AutoCloseFD, Path> createTempFile(const Path & prefix)
{
Path tmpl(defaultTempDir() + "/" + prefix + ".XXXXXX");
// Strictly speaking, this is UB, but who cares...
// FIXME: use O_TMPFILE.
AutoCloseFD fd(mkstemp((char *) tmpl.c_str()));
AutoCloseFD fd(mkstemp(tmpl.data()));
if (!fd)
throw SysError("creating temporary file '%s'", tmpl);
closeOnExec(fd.get());

View file

@ -22,7 +22,7 @@ public:
Finally(Finally &&other) : fun(std::move(other.fun)) {
other.movedFrom = true;
}
~Finally() noexcept(false)
~Finally() noexcept(noexcept(fun()))
{
try {
if (!movedFrom)

View file

@ -129,7 +129,7 @@ std::string Hash::to_string(Base base, bool includeType) const
break;
case Base::Base64:
case Base::SRI:
s += base64Encode(std::string_view((const char *) hash, hashSize));
s += base64Encode(std::string_view(reinterpret_cast<const char *>(hash), hashSize));
break;
}
return s;

View file

@ -9,9 +9,7 @@
#include <cerrno>
#include <cstdlib>
#include <cstring>
#include <future>
#include <iostream>
#include <thread>
#include <grp.h>
#include <sys/types.h>
@ -176,7 +174,7 @@ static pid_t doFork(std::function<void()> fun)
#if __linux__
static int childEntry(void * arg)
{
auto main = (std::function<void()> *) arg;
auto main = static_cast<std::function<void()> *>(arg);
(*main)();
return 1;
}
@ -212,8 +210,8 @@ Pid startProcess(std::function<void()> fun, const ProcessOptions & options)
assert(!(options.cloneFlags & CLONE_VM));
size_t stackSize = 1 * 1024 * 1024;
auto stack = (char *) mmap(0, stackSize,
PROT_WRITE | PROT_READ, MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK, -1, 0);
auto stack = static_cast<char *>(mmap(0, stackSize,
PROT_WRITE | PROT_READ, MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK, -1, 0));
if (stack == MAP_FAILED) throw SysError("allocating stack");
Finally freeStack([&]() { munmap(stack, stackSize); });

View file

@ -8,7 +8,8 @@ namespace nix {
std::vector<char *> stringsToCharPtrs(const Strings & ss)
{
std::vector<char *> res;
for (auto & s : ss) res.push_back((char *) s.c_str());
// This is const cast since this exists for OS APIs that want char *
for (auto & s : ss) res.push_back(const_cast<char *>(s.data()));
res.push_back(0);
return res;
}

View file

@ -20,6 +20,8 @@ constexpr char treeNull[] = " ";
* Convert a list of strings to a null-terminated vector of `char
* *`s. The result must not be accessed beyond the lifetime of the
* list of strings.
*
* Modifying the resulting array elements violates the constness of ss.
*/
std::vector<char *> stringsToCharPtrs(const Strings & ss);

View file

@ -15,11 +15,11 @@ static int callback_open(struct archive *, void * self)
static ssize_t callback_read(struct archive * archive, void * _self, const void * * buffer)
{
auto self = (TarArchive *) _self;
auto self = static_cast<TarArchive *>(_self);
*buffer = self->buffer.data();
try {
return self->source->read((char *) self->buffer.data(), self->buffer.size());
return self->source->read(reinterpret_cast<char *>(self->buffer.data()), self->buffer.size());
} catch (EndOfFile &) {
return 0;
} catch (std::exception & err) {

View file

@ -316,7 +316,7 @@ static void daemonLoop(std::optional<TrustedFlag> forceTrustClientOpt)
socklen_t remoteAddrLen = sizeof(remoteAddr);
AutoCloseFD remote{accept(fdSocket.get(),
(struct sockaddr *) &remoteAddr, &remoteAddrLen)};
reinterpret_cast<struct sockaddr *>(&remoteAddr), &remoteAddrLen)};
checkInterrupt();
if (!remote) {
if (errno == EINTR) continue;

View file

@ -1,5 +1,3 @@
#include <algorithm>
#include "args/root.hh"
#include "command.hh"
#include "common-args.hh"
@ -62,12 +60,12 @@ static bool haveInternet()
for (auto i = addrs; i; i = i->ifa_next) {
if (!i->ifa_addr) continue;
if (i->ifa_addr->sa_family == AF_INET) {
if (ntohl(((sockaddr_in *) i->ifa_addr)->sin_addr.s_addr) != INADDR_LOOPBACK) {
if (ntohl(reinterpret_cast<sockaddr_in *>(i->ifa_addr)->sin_addr.s_addr) != INADDR_LOOPBACK) {
return true;
}
} else if (i->ifa_addr->sa_family == AF_INET6) {
if (!IN6_IS_ADDR_LOOPBACK(&((sockaddr_in6 *) i->ifa_addr)->sin6_addr) &&
!IN6_IS_ADDR_LINKLOCAL(&((sockaddr_in6 *) i->ifa_addr)->sin6_addr))
if (!IN6_IS_ADDR_LOOPBACK(&reinterpret_cast<sockaddr_in6 *>(i->ifa_addr)->sin6_addr) &&
!IN6_IS_ADDR_LINKLOCAL(&reinterpret_cast<sockaddr_in6 *>(i->ifa_addr)->sin6_addr))
return true;
}
}

View file

@ -171,7 +171,7 @@ struct CmdWhyDepends : SourceExprCommand, MixOperateOnOptions
and `dependency`. */
std::function<void(Node &, const std::string &, const std::string &)> printNode;
struct BailOut { };
struct BailOut : std::exception { };
printNode = [&](Node & node, const std::string & firstPad, const std::string & tailPad) {
auto pathS = store->printStorePath(node.path);

View file

@ -6,7 +6,6 @@
#include <future>
#include <gtest/gtest.h>
#include <netinet/in.h>
#include <stdexcept>
#include <string>
#include <string_view>
#include <sys/poll.h>
@ -130,7 +129,7 @@ serveHTTP(std::string status, std::string headers, std::function<std::string()>
TEST(FileTransfer, exceptionAbortsDownload)
{
struct Done
struct Done : std::exception
{};
auto ft = makeFileTransfer();

View file

@ -28,7 +28,7 @@ TEST(closure, correctClosure) {
}
TEST(closure, properlyHandlesDirectExceptions) {
struct TestExn {};
struct TestExn : std::exception {};
EXPECT_THROW(
computeClosure<string>(
{"A"},

View file

@ -85,6 +85,7 @@ TEST(Generator, nestsExceptions)
co_yield 1;
co_yield []() -> Generator<int> {
co_yield 9;
// NOLINTNEXTLINE(hicpp-exception-baseclass)
throw 1;
co_yield 10;
}();
@ -101,6 +102,7 @@ TEST(Generator, exception)
{
auto g = []() -> Generator<int> {
co_yield 1;
// NOLINTNEXTLINE(hicpp-exception-baseclass)
throw 1;
}();
@ -110,6 +112,7 @@ TEST(Generator, exception)
}
{
auto g = []() -> Generator<int> {
// NOLINTNEXTLINE(hicpp-exception-baseclass)
throw 1;
co_return;
}();
@ -173,11 +176,13 @@ struct ThrowTransform
int operator()(bool)
{
// NOLINTNEXTLINE(hicpp-exception-baseclass)
throw 2;
}
Generator<int, void> operator()(Generator<int> && inner)
{
// NOLINTNEXTLINE(hicpp-exception-baseclass)
throw false;
}
};

View file

@ -29,12 +29,12 @@ namespace nix {
char cwd[PATH_MAX+1];
auto p = absPath("");
ASSERT_EQ(p, getcwd((char*)&cwd, PATH_MAX));
ASSERT_EQ(p, getcwd(cwd, PATH_MAX));
}
TEST(absPath, usesOptionalBasePathWhenGiven) {
char _cwd[PATH_MAX+1];
char* cwd = getcwd((char*)&_cwd, PATH_MAX);
char* cwd = getcwd(_cwd, PATH_MAX);
auto p = absPath("", cwd);
@ -43,7 +43,7 @@ namespace nix {
TEST(absPath, isIdempotent) {
char _cwd[PATH_MAX+1];
char* cwd = getcwd((char*)&_cwd, PATH_MAX);
char* cwd = getcwd(_cwd, PATH_MAX);
auto p1 = absPath(cwd);
auto p2 = absPath(p1);