From 7923dcc034a9f8ac5c671cebbda5989827ca0585 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Mon, 28 Jul 2025 17:15:50 +0200 Subject: [PATCH] libtuil: remove deserializing operator>> they will not work well with async deserialization and are not used consistently anyway. just like the serializing operator<< these are protocol stability hazards: changing the type of a field influences the wire protocol layout and type constraints, which is not amazing Change-Id: I54b20a133048f4ca15a9fb0f4d8b94dc78f62d89 --- lix/legacy/nix-store.cc | 7 ++++++- lix/libstore/daemon.cc | 24 +++++++++++++++--------- lix/libstore/derivations.cc | 4 +++- lix/libstore/legacy-ssh-store.cc | 2 +- lix/libstore/remote-store.cc | 2 +- lix/libstore/serve-protocol.cc | 14 +++++++------- lix/libstore/worker-protocol.cc | 21 +++++++++++---------- lix/libutil/serialise.cc | 7 ------- lix/libutil/serialise.hh | 15 ++------------- 9 files changed, 46 insertions(+), 50 deletions(-) diff --git a/lix/legacy/nix-store.cc b/lix/legacy/nix-store.cc index b6766957a..a0af95c5f 100644 --- a/lix/legacy/nix-store.cc +++ b/lix/legacy/nix-store.cc @@ -17,8 +17,11 @@ #include "graphml.hh" #include "lix/libcmd/legacy.hh" #include "lix/libstore/path-with-outputs.hh" +#include "lix/libutil/serialise.hh" #include "nix-store.hh" +#include +#include #include #include @@ -1067,7 +1070,9 @@ opServe(std::shared_ptr store, AsyncIoRoot & aio, Strings opFlags, String if (deriver != "") info.deriver = store->parseStorePath(deriver); info.references = ServeProto::Serialise::read(rconn); - in >> info.registrationTime >> info.narSize >> info.ultimate; + info.registrationTime = readNum(in); + info.narSize = readNum(in); + info.ultimate = readBool(in); info.sigs = readStrings(in); info.ca = ContentAddress::parseOpt(readString(in)); diff --git a/lix/libstore/daemon.cc b/lix/libstore/daemon.cc index eb4491dae..c2508f3f4 100644 --- a/lix/libstore/daemon.cc +++ b/lix/libstore/daemon.cc @@ -13,10 +13,13 @@ #include "lix/libutil/finally.hh" #include "lix/libutil/archive.hh" #include "lix/libstore/derivations.hh" +#include "lix/libutil/serialise.hh" #include "lix/libutil/strings.hh" #include "lix/libutil/args.hh" #include +#include +#include #include namespace nix::daemon { @@ -364,8 +367,7 @@ static void performOp(AsyncIoRoot & aio, TunnelLogger * logger, ref store auto name = readString(from); auto camStr = readString(from); auto refs = WorkerProto::Serialise::read(rconn); - bool repairBool; - from >> repairBool; + bool repairBool = readBool(from); auto repair = RepairFlag{repairBool}; logger->startWork(); @@ -401,8 +403,8 @@ static void performOp(AsyncIoRoot & aio, TunnelLogger * logger, ref store } case WorkerProto::Op::AddMultipleToStore: { - bool repair, dontCheckSigs; - from >> repair >> dontCheckSigs; + bool repair = readBool(from); + bool dontCheckSigs = readBool(from); if (!trusted && dontCheckSigs) dontCheckSigs = false; @@ -607,7 +609,8 @@ static void performOp(AsyncIoRoot & aio, TunnelLogger * logger, ref store GCOptions options; options.action = (GCOptions::GCAction) readInt(from); options.pathsToDelete = WorkerProto::Serialise::read(rconn); - from >> options.ignoreLiveness >> options.maxFreed; + options.ignoreLiveness = readBool(from); + options.maxFreed = readNum(from); // obsolete fields readInt(from); readInt(from); @@ -722,8 +725,8 @@ static void performOp(AsyncIoRoot & aio, TunnelLogger * logger, ref store break; case WorkerProto::Op::VerifyStore: { - bool checkContents, repair; - from >> checkContents >> repair; + bool checkContents = readBool(from); + bool repair = readBool(from); logger->startWork(); if (repair && !trusted) throw Error("you are not privileged to repair paths"); @@ -760,10 +763,13 @@ static void performOp(AsyncIoRoot & aio, TunnelLogger * logger, ref store if (deriver != "") info.deriver = store->parseStorePath(deriver); info.references = WorkerProto::Serialise::read(rconn); - from >> info.registrationTime >> info.narSize >> info.ultimate; + info.registrationTime = readNum(from); + info.narSize = readNum(from); + info.ultimate = readBool(from); info.sigs = readStrings(from); info.ca = ContentAddress::parseOpt(readString(from)); - from >> repair >> dontCheckSigs; + repair = readBool(from); + dontCheckSigs = readBool(from); if (!trusted && dontCheckSigs) dontCheckSigs = false; if (!trusted) diff --git a/lix/libstore/derivations.cc b/lix/libstore/derivations.cc index 961942149..f7b005450 100644 --- a/lix/libstore/derivations.cc +++ b/lix/libstore/derivations.cc @@ -3,6 +3,7 @@ #include "lix/libstore/globals.hh" #include "lix/libutil/json.hh" #include "lix/libutil/result.hh" +#include "lix/libutil/serialise.hh" #include "lix/libutil/types.hh" #include "lix/libstore/common-protocol.hh" #include "lix/libstore/common-protocol-impl.hh" @@ -678,7 +679,8 @@ Source & readDerivation(Source & in, const Store & store, BasicDerivation & drv, drv.inputSrcs = CommonProto::Serialise::read( CommonProto::ReadConn { .from = in, .store = store }); - in >> drv.platform >> drv.builder; + drv.platform = readString(in); + drv.builder = readString(in); drv.args = readStrings(in); nr = readNum(in); diff --git a/lix/libstore/legacy-ssh-store.cc b/lix/libstore/legacy-ssh-store.cc index 97d3dd724..53fb45888 100644 --- a/lix/libstore/legacy-ssh-store.cc +++ b/lix/libstore/legacy-ssh-store.cc @@ -65,7 +65,7 @@ BuildPathsResult ServeProto::Serialise::read(ServeProto::ReadC result.status = (BuildResult::Status) readInt(conn.from); if (!result.success()) { - conn.from >> result.errorMsg; + result.errorMsg = readString(conn.from); return {result, Error(result.status, result.errorMsg)}; } diff --git a/lix/libstore/remote-store.cc b/lix/libstore/remote-store.cc index 62d6bcd87..bb0a6e367 100644 --- a/lix/libstore/remote-store.cc +++ b/lix/libstore/remote-store.cc @@ -98,7 +98,7 @@ try { if (magic != WORKER_MAGIC_2) throw Error("protocol mismatch"); - from >> conn.daemonVersion; + conn.daemonVersion = readNum(from); if (GET_PROTOCOL_MAJOR(conn.daemonVersion) != GET_PROTOCOL_MAJOR(PROTOCOL_VERSION)) throw Error("Nix daemon protocol version not supported"); if (GET_PROTOCOL_MINOR(conn.daemonVersion) < MIN_SUPPORTED_MINOR_WORKER_PROTO_VERSION) diff --git a/lix/libstore/serve-protocol.cc b/lix/libstore/serve-protocol.cc index 5383b18d0..2fb9b27a2 100644 --- a/lix/libstore/serve-protocol.cc +++ b/lix/libstore/serve-protocol.cc @@ -14,14 +14,14 @@ BuildResult ServeProto::Serialise::read(ServeProto::ReadConn conn) { BuildResult status; status.status = (BuildResult::Status) readInt(conn.from); - conn.from >> status.errorMsg; + status.errorMsg = readString(conn.from); - if (GET_PROTOCOL_MINOR(conn.version) >= 3) - conn.from - >> status.timesBuilt - >> status.isNonDeterministic - >> status.startTime - >> status.stopTime; + if (GET_PROTOCOL_MINOR(conn.version) >= 3) { + status.timesBuilt = readNum(conn.from); + status.isNonDeterministic = readBool(conn.from); + status.startTime = readNum(conn.from); + status.stopTime = readNum(conn.from); + } if (GET_PROTOCOL_MINOR(conn.version) >= 6) { auto builtOutputs = ServeProto::Serialise::read(conn); for (auto && [output, realisation] : builtOutputs) diff --git a/lix/libstore/worker-protocol.cc b/lix/libstore/worker-protocol.cc index bbe5af805..6adfe13c0 100644 --- a/lix/libstore/worker-protocol.cc +++ b/lix/libstore/worker-protocol.cc @@ -6,6 +6,8 @@ #include "lix/libstore/worker-protocol-impl.hh" #include "lix/libutil/archive.hh" #include "lix/libstore/path-info.hh" +#include +#include #include namespace nix { @@ -79,12 +81,11 @@ BuildResult WorkerProto::Serialise::read(WorkerProto::ReadConn conn { BuildResult res; res.status = (BuildResult::Status) readInt(conn.from); - conn.from >> res.errorMsg; - conn.from - >> res.timesBuilt - >> res.isNonDeterministic - >> res.startTime - >> res.stopTime; + res.errorMsg = readString(conn.from); + res.timesBuilt = readNum(conn.from); + res.isNonDeterministic = readBool(conn.from); + res.startTime = readNum(conn.from); + res.stopTime = readNum(conn.from); auto builtOutputs = WorkerProto::Serialise::read(conn); for (auto && [output, realisation] : builtOutputs) res.builtOutputs.insert_or_assign( @@ -131,9 +132,10 @@ UnkeyedValidPathInfo WorkerProto::Serialise::read(ReadConn UnkeyedValidPathInfo info(narHash); if (deriver != "") info.deriver = conn.store.parseStorePath(deriver); info.references = WorkerProto::Serialise::read(conn); - conn.from >> info.registrationTime >> info.narSize; + info.registrationTime = readNum(conn.from); + info.narSize = readNum(conn.from); - conn.from >> info.ultimate; + info.ultimate = readBool(conn.from); info.sigs = readStrings(conn.from); info.ca = ContentAddress::parseOpt(readString(conn.from)); @@ -156,8 +158,7 @@ WireFormatGenerator WorkerProto::Serialise::write(WriteCon std::optional WorkerProto::Serialise>::read(ReadConn conn) { - bool valid; - conn.from >> valid; + bool valid = readBool(conn.from); if (valid) { return WorkerProto::Serialise::read(conn); } else { diff --git a/lix/libutil/serialise.cc b/lix/libutil/serialise.cc index 04426a4bb..1a5b6496c 100644 --- a/lix/libutil/serialise.cc +++ b/lix/libutil/serialise.cc @@ -254,13 +254,6 @@ std::string readString(Source & source, size_t max) return res; } -Source & operator >> (Source & in, std::string & s) -{ - s = readString(in); - return in; -} - - template T readStrings(Source & source) { auto count = readNum(source); diff --git a/lix/libutil/serialise.hh b/lix/libutil/serialise.hh index e89bca191..13617f7c9 100644 --- a/lix/libutil/serialise.hh +++ b/lix/libutil/serialise.hh @@ -395,20 +395,9 @@ void readPadding(size_t len, Source & source); std::string readString(Source & source, size_t max = std::numeric_limits::max()); template T readStrings(Source & source); -Source & operator >> (Source & in, std::string & s); - -template -Source & operator >> (Source & in, T & n) +inline bool readBool(Source & in) { - n = readNum(in); - return in; -} - -template -Source & operator >> (Source & in, bool & b) -{ - b = readNum(in); - return in; + return readNum(in); } Error readError(Source & source);