Merge pull request #472 from davidgiven/proto

Make the internal disk data structures all const.
2025-10-31 11:17:01 -07:00 · 2022-02-24 22:15:58 +00:00
parent 4a0fc3d566 eade2e279e
commit 30957f4105
22 changed files with 331 additions and 276 deletions
--- a/arch/amiga/encoder.cc
+++ b/arch/amiga/encoder.cc
@@ -51,7 +51,7 @@ static void write_bits(std::vector<bool>& bits, unsigned& cursor, const Bytes& b
 	}
 }

-static void write_sector(std::vector<bool>& bits, unsigned& cursor, const std::shared_ptr<Sector>& sector)
+static void write_sector(std::vector<bool>& bits, unsigned& cursor, const std::shared_ptr<const Sector>& sector)
 {
 	if ((sector->data.size() != 512) && (sector->data.size() != 528))
 		Error() << "unsupported sector size --- you must pick 512 or 528";
@@ -105,9 +105,9 @@ public:
 		_config(config.amiga()) {}

 public:
-	std::vector<std::shared_ptr<Sector>> collectSectors(int physicalTrack, int physicalSide, const Image& image) override
+	std::vector<std::shared_ptr<const Sector>> collectSectors(int physicalTrack, int physicalSide, const Image& image) override
 	{
-		std::vector<std::shared_ptr<Sector>> sectors;
+		std::vector<std::shared_ptr<const Sector>> sectors;

 		if ((physicalTrack >= 0) && (physicalTrack < AMIGA_TRACKS_PER_DISK))
 		{
@@ -123,7 +123,7 @@ public:
 	}

    std::unique_ptr<Fluxmap> encode(int physicalTrack, int physicalSide,
-			const std::vector<std::shared_ptr<Sector>>& sectors, const Image& image) override
+			const std::vector<std::shared_ptr<const Sector>>& sectors, const Image& image) override
 	{
 		if ((physicalTrack < 0) || (physicalTrack >= AMIGA_TRACKS_PER_DISK))
 			return std::unique_ptr<Fluxmap>();
--- a/arch/brother/encoder.cc
+++ b/arch/brother/encoder.cc
@@ -109,9 +109,9 @@ public:
 	{}

 public:
-	std::vector<std::shared_ptr<Sector>> collectSectors(int physicalTrack, int physicalSide, const Image& image) override
+	std::vector<std::shared_ptr<const Sector>> collectSectors(int physicalTrack, int physicalSide, const Image& image) override
 	{
-		std::vector<std::shared_ptr<Sector>> sectors;
+		std::vector<std::shared_ptr<const Sector>> sectors;

 		int logicalTrack;
 		if (physicalSide != 0)
@@ -144,7 +144,7 @@ public:
 	}

    std::unique_ptr<Fluxmap> encode(int physicalTrack, int physicalSide,
-			const std::vector<std::shared_ptr<Sector>>& sectors, const Image& image) override
+			const std::vector<std::shared_ptr<const Sector>>& sectors, const Image& image) override
 	{
 		int logicalTrack;
 		if (physicalSide != 0)
--- a/arch/c64/encoder.cc
+++ b/arch/c64/encoder.cc
@@ -211,9 +211,9 @@ public:
 	{}

 public:
-	std::vector<std::shared_ptr<Sector>> collectSectors(int physicalTrack, int physicalSide, const Image& image) override
+	std::vector<std::shared_ptr<const Sector>> collectSectors(int physicalTrack, int physicalSide, const Image& image) override
 	{
-		std::vector<std::shared_ptr<Sector>> sectors;
+		std::vector<std::shared_ptr<const Sector>> sectors;

        if (physicalSide == 0)
        {
@@ -231,7 +231,7 @@ public:
 	}

    std::unique_ptr<Fluxmap> encode(int physicalTrack, int physicalSide,
-            const std::vector<std::shared_ptr<Sector>>& sectors, const Image& image) override
+            const std::vector<std::shared_ptr<const Sector>>& sectors, const Image& image) override
    {
        /* The format ID Character # 1 and # 2 are in the .d64 image only present
         * in track 18 sector zero which contains the BAM info in byte 162 and 163.
@@ -278,7 +278,7 @@ public:
    }

 private:
-	void writeSector(std::vector<bool>& bits, unsigned& cursor, const std::shared_ptr<Sector>& sector) const
+	void writeSector(std::vector<bool>& bits, unsigned& cursor, std::shared_ptr<const Sector> sector) const
    {
        /* Source: http://www.unusedino.de/ec64/technical/formats/g64.html 
         * 1. Header sync       FF FF FF FF FF (40 'on' bits, not GCR)
--- a/arch/ibm/encoder.cc
+++ b/arch/ibm/encoder.cc
@@ -123,9 +123,9 @@ private:
 	}

 public:
-	std::vector<std::shared_ptr<Sector>> collectSectors(int physicalTrack, int physicalSide, const Image& image) override
+	std::vector<std::shared_ptr<const Sector>> collectSectors(int physicalTrack, int physicalSide, const Image& image) override
 	{
-		std::vector<std::shared_ptr<Sector>> sectors;
+		std::vector<std::shared_ptr<const Sector>> sectors;
 		IbmEncoderProto::TrackdataProto trackdata;
 		getTrackFormat(trackdata, physicalTrack, physicalSide);

@@ -141,7 +141,7 @@ public:
 	}

    std::unique_ptr<Fluxmap> encode(int physicalTrack, int physicalSide,
-			const std::vector<std::shared_ptr<Sector>>& sectors, const Image& image) override
+			const std::vector<std::shared_ptr<const Sector>>& sectors, const Image& image) override
 	{
 		IbmEncoderProto::TrackdataProto trackdata;
 		getTrackFormat(trackdata, physicalTrack, physicalSide);
--- a/arch/macintosh/encoder.cc
+++ b/arch/macintosh/encoder.cc
@@ -164,7 +164,7 @@ static uint8_t encode_side(uint8_t track, uint8_t side)
 	return (side ? 0x20 : 0x00) | ((track>0x3f) ? 0x01 : 0x00);
 }

-static void write_sector(std::vector<bool>& bits, unsigned& cursor, const std::shared_ptr<Sector>& sector)
+static void write_sector(std::vector<bool>& bits, unsigned& cursor, const std::shared_ptr<const Sector>& sector)
 {
 	if ((sector->data.size() != 512) && (sector->data.size() != 524))
 		Error() << "unsupported sector size --- you must pick 512 or 524";
@@ -208,9 +208,9 @@ public:
 	{}

 public:
-	std::vector<std::shared_ptr<Sector>> collectSectors(int physicalTrack, int physicalSide, const Image& image) override
+	std::vector<std::shared_ptr<const Sector>> collectSectors(int physicalTrack, int physicalSide, const Image& image) override
 	{
-		std::vector<std::shared_ptr<Sector>> sectors;
+		std::vector<std::shared_ptr<const Sector>> sectors;

 		if ((physicalTrack >= 0) && (physicalTrack < MAC_TRACKS_PER_DISK))
        {
@@ -227,7 +227,7 @@ public:
 	}

    std::unique_ptr<Fluxmap> encode(int physicalTrack, int physicalSide,
-			const std::vector<std::shared_ptr<Sector>>& sectors, const Image& image) override
+			const std::vector<std::shared_ptr<const Sector>>& sectors, const Image& image) override
 	{
 		if ((physicalTrack < 0) || (physicalTrack >= MAC_TRACKS_PER_DISK))
 			return std::unique_ptr<Fluxmap>();
--- a/arch/micropolis/encoder.cc
+++ b/arch/micropolis/encoder.cc
@@ -6,7 +6,7 @@
 #include "image.h"
 #include "lib/encoders/encoders.pb.h"

-static void write_sector(std::vector<bool>& bits, unsigned& cursor, const std::shared_ptr<Sector>& sector)
+static void write_sector(std::vector<bool>& bits, unsigned& cursor, const std::shared_ptr<const Sector>& sector)
 {
 	if ((sector->data.size() != 256) && (sector->data.size() != MICROPOLIS_ENCODED_SECTOR_SIZE))
 		Error() << "unsupported sector size --- you must pick 256 or 275";
@@ -70,9 +70,9 @@ public:
 		_config(config.micropolis())
 	{}

-	std::vector<std::shared_ptr<Sector>> collectSectors(int physicalTrack, int physicalSide, const Image& image) override
+	std::vector<std::shared_ptr<const Sector>> collectSectors(int physicalTrack, int physicalSide, const Image& image) override
 	{
-		std::vector<std::shared_ptr<Sector>> sectors;
+		std::vector<std::shared_ptr<const Sector>> sectors;

 		if ((physicalTrack >= 0) && (physicalTrack < 77))
 		{
@@ -88,7 +88,7 @@ public:
 	}

 	std::unique_ptr<Fluxmap> encode(int physicalTrack, int physicalSide,
-			const std::vector<std::shared_ptr<Sector>>& sectors, const Image& image) override
+			const std::vector<std::shared_ptr<const Sector>>& sectors, const Image& image) override
 	{
 		int bitsPerRevolution = 100000;
 		double clockRateUs = 2.00;
--- a/arch/northstar/encoder.cc
+++ b/arch/northstar/encoder.cc
@@ -17,7 +17,7 @@

 #define TOTAL_SECTOR_BYTES ()

-static void write_sector(std::vector<bool>& bits, unsigned& cursor, const std::shared_ptr<Sector>& sector)
+static void write_sector(std::vector<bool>& bits, unsigned& cursor, const std::shared_ptr<const Sector>& sector)
 {
 	int preambleSize = 0;
 	int encodedSectorSize = 0;
@@ -108,9 +108,9 @@ public:
 		_config(config.northstar())
 	{}

-	std::vector<std::shared_ptr<Sector>> collectSectors(int physicalTrack, int physicalSide, const Image& image) override
+	std::vector<std::shared_ptr<const Sector>> collectSectors(int physicalTrack, int physicalSide, const Image& image) override
 	{
-		std::vector<std::shared_ptr<Sector>> sectors;
+		std::vector<std::shared_ptr<const Sector>> sectors;

 		if ((physicalTrack >= 0) && (physicalTrack < 35))
 		{
@@ -126,7 +126,7 @@ public:
 	}

 	std::unique_ptr<Fluxmap> encode(int physicalTrack, int physicalSide,
-			const std::vector<std::shared_ptr<Sector>>& sectors, const Image& image) override
+			const std::vector<std::shared_ptr<const Sector>>& sectors, const Image& image) override
 	{
 		int bitsPerRevolution = 100000;
 		double clockRateUs = 4.00;
--- a/arch/tids990/encoder.cc
+++ b/arch/tids990/encoder.cc
@@ -59,9 +59,9 @@ private:
 	}

 public:
-	std::vector<std::shared_ptr<Sector>> collectSectors(int physicalTrack, int physicalSide, const Image& image) override
+	std::vector<std::shared_ptr<const Sector>> collectSectors(int physicalTrack, int physicalSide, const Image& image) override
 	{
-		std::vector<std::shared_ptr<Sector>> sectors;
+		std::vector<std::shared_ptr<const Sector>> sectors;

 		for (char sectorChar : _config.sector_skew())
        {
@@ -75,7 +75,7 @@ public:
 	}

    std::unique_ptr<Fluxmap> encode(int physicalTrack, int physicalSide,
-			const std::vector<std::shared_ptr<Sector>>& sectors, const Image& image) override
+			const std::vector<std::shared_ptr<const Sector>>& sectors, const Image& image) override
 	{
 		double clockRateUs = 1e3 / _config.clock_rate_khz() / 2.0;
 		int bitsPerRevolution = (_config.track_length_ms() * 1000.0) / clockRateUs;
--- a/arch/victor9k/encoder.cc
+++ b/arch/victor9k/encoder.cc
@@ -143,9 +143,9 @@ private:
 	}

 public:
-	std::vector<std::shared_ptr<Sector>> collectSectors(int physicalTrack, int physicalSide, const Image& image) override
+	std::vector<std::shared_ptr<const Sector>> collectSectors(int physicalTrack, int physicalSide, const Image& image) override
 	{
-		std::vector<std::shared_ptr<Sector>> sectors;
+		std::vector<std::shared_ptr<const Sector>> sectors;

 		Victor9kEncoderProto::TrackdataProto trackdata;
 		getTrackFormat(trackdata, physicalTrack, physicalSide);
@@ -162,7 +162,7 @@ public:
 	}

    std::unique_ptr<Fluxmap> encode(int physicalTrack, int physicalSide,
-            const std::vector<std::shared_ptr<Sector>>& sectors, const Image& image) override
+            const std::vector<std::shared_ptr<const Sector>>& sectors, const Image& image) override
    {
 		Victor9kEncoderProto::TrackdataProto trackdata;
 		getTrackFormat(trackdata, physicalTrack, physicalSide);
--- a/lib/decoders/decoders.cc
+++ b/lib/decoders/decoders.cc
@@ -57,7 +57,7 @@ std::unique_ptr<AbstractDecoder> AbstractDecoder::create(const DecoderProto& con
 	return (decoder->second)(config);
 }

-std::shared_ptr<TrackDataFlux> AbstractDecoder::decodeToSectors(
+std::shared_ptr<const TrackDataFlux> AbstractDecoder::decodeToSectors(
 		std::shared_ptr<const Fluxmap> fluxmap, unsigned physicalCylinder, unsigned physicalHead)
 {
 	_trackdata = std::make_shared<TrackDataFlux>();
@@ -84,7 +84,7 @@ std::shared_ptr<TrackDataFlux> AbstractDecoder::decodeToSectors(
        Fluxmap::Position recordStart = fmr.tell();
        _sector->clock = advanceToNextRecord();
 		if (fmr.eof() || !_sector->clock)
-            return std::move(_trackdata);
+            return _trackdata;

        /* Read the sector record. */

--- a/lib/decoders/decoders.h
+++ b/lib/decoders/decoders.h
@@ -45,7 +45,7 @@ public:
    };

 public:
-    std::shared_ptr<TrackDataFlux> decodeToSectors(std::shared_ptr<const Fluxmap> fluxmap, unsigned cylinder, unsigned head);
+    std::shared_ptr<const TrackDataFlux> decodeToSectors(std::shared_ptr<const Fluxmap> fluxmap, unsigned cylinder, unsigned head);
    void pushRecord(const Fluxmap::Position& start, const Fluxmap::Position& end);

 	void resetFluxDecoder();
--- a/lib/encoders/encoders.h
+++ b/lib/encoders/encoders.h
@@ -15,12 +15,13 @@ public:
    static std::unique_ptr<AbstractEncoder> create(const EncoderProto& config);

 public:
-	virtual std::vector<std::shared_ptr<Sector>> collectSectors(
+    virtual std::vector<std::shared_ptr<const Sector>> collectSectors(
        int physicalCylinder, int physicalHead, const Image& image) = 0;

-	virtual std::unique_ptr<Fluxmap> encode(
-		int physicalCylinder, int physicalHead, const std::vector<std::shared_ptr<Sector>>& sectors, const Image& image) = 0;
+    virtual std::unique_ptr<Fluxmap> encode(int physicalCylinder,
+        int physicalHead,
+        const std::vector<std::shared_ptr<const Sector>>& sectors,
+        const Image& image) = 0;
 };

 #endif
-
--- a/lib/flux.h
+++ b/lib/flux.h
@@ -17,7 +17,7 @@ struct TrackDataFlux
 	unsigned physicalCylinder;
 	unsigned physicalHead;
 	std::shared_ptr<const Fluxmap> fluxmap;
-	std::vector<std::shared_ptr<Record>> records;
+	std::vector<std::shared_ptr<const Record>> records;
 	std::vector<std::shared_ptr<Sector>> sectors;
 };

@@ -25,14 +25,14 @@ struct TrackFlux
 {
 	unsigned physicalCylinder;
 	unsigned physicalHead;
-	std::vector<std::shared_ptr<TrackDataFlux>> trackDatas;
-	std::set<std::shared_ptr<Sector>> sectors;
+	std::vector<std::shared_ptr<const TrackDataFlux>> trackDatas;
+	std::set<std::shared_ptr<const Sector>> sectors;
 };

 struct DiskFlux
 {
-	std::vector<std::shared_ptr<TrackFlux>> tracks;
-	std::unique_ptr<Image> image;
+	std::vector<std::shared_ptr<const TrackFlux>> tracks;
+	std::unique_ptr<const Image> image;
 };

 #endif
--- a/lib/globals.h
+++ b/lib/globals.h
@@ -3,6 +3,7 @@

 #include <stddef.h>
 #include <functional>
+#include <numeric>
 #include <iostream>
 #include <map>
 #include <memory>
--- a/lib/image.cc
+++ b/lib/image.cc
@@ -5,9 +5,9 @@
 Image::Image()
 {}

-Image::Image(std::set<std::shared_ptr<Sector>>& sectors)
+Image::Image(std::set<std::shared_ptr<const Sector>>& sectors)
 {
-	for (std::shared_ptr<Sector> sector : sectors)
+	for (auto& sector : sectors)
 	{
 		key_t key = std::make_tuple(sector->logicalTrack, sector->logicalSide, sector->logicalSector);
 		_sectors[key] = sector;
@@ -15,9 +15,9 @@ Image::Image(std::set<std::shared_ptr<Sector>>& sectors)
 	calculateSize();
 }

-const std::shared_ptr<Sector>& Image::get(unsigned track, unsigned side, unsigned sectorid) const
+std::shared_ptr<const Sector> Image::get(unsigned track, unsigned side, unsigned sectorid) const
 {
-	const static std::shared_ptr<Sector> NONE;
+	static std::shared_ptr<const Sector> NONE;

 	key_t key = std::make_tuple(track, side, sectorid);
 	auto i = _sectors.find(key);
@@ -26,14 +26,15 @@ const std::shared_ptr<Sector>& Image::get(unsigned track, unsigned side, unsigne
 	return i->second;
 }

-const std::shared_ptr<Sector>& Image::put(unsigned track, unsigned side, unsigned sectorid)
+std::shared_ptr<Sector> Image::put(unsigned track, unsigned side, unsigned sectorid)
 {
 	key_t key = std::make_tuple(track, side, sectorid);
 	std::shared_ptr<Sector> sector = std::make_shared<Sector>();
 	sector->logicalTrack = track;
 	sector->logicalSide = side;
 	sector->logicalSector = sectorid;
-	return _sectors[key] = sector;
+	_sectors[key] = sector;
+	return sector;
 }

 void Image::calculateSize()
--- a/lib/image.h
+++ b/lib/image.h
@@ -18,16 +18,16 @@ private:

 public:
 	Image();
-	Image(std::set<std::shared_ptr<Sector>>& sectors);
+	Image(std::set<std::shared_ptr<const Sector>>& sectors);

 public:
 	class const_iterator
 	{
-		typedef std::map<key_t, std::shared_ptr<Sector>>::const_iterator wrapped_iterator_t;
+		typedef std::map<key_t, std::shared_ptr<const Sector>>::const_iterator wrapped_iterator_t;

 	public:
 		const_iterator(const wrapped_iterator_t& it): _it(it) {}
-		Sector* operator* () { return _it->second.get(); }
+		const Sector* operator* () { return _it->second.get(); }
 		void operator++ () { _it++; }
 		bool operator== (const const_iterator& other) const { return _it == other._it; }
 		bool operator!= (const const_iterator& other) const { return _it != other._it; }
@@ -39,8 +39,8 @@ public:
 public:
 	void calculateSize();

-	const std::shared_ptr<Sector>& get(unsigned track, unsigned side, unsigned sectorId) const;
-	const std::shared_ptr<Sector>& put(unsigned track, unsigned side, unsigned sectorId);
+	std::shared_ptr<const Sector> get(unsigned track, unsigned side, unsigned sectorId) const;
+	std::shared_ptr<Sector> put(unsigned track, unsigned side, unsigned sectorId);

 	const_iterator begin() const { return const_iterator(_sectors.cbegin()); }
 	const_iterator end() const { return const_iterator(_sectors.cend()); }
@@ -50,7 +50,7 @@ public:

 private:
 	Geometry _geometry = {0, 0, 0};
-	std::map<key_t, std::shared_ptr<Sector>> _sectors;
+	std::map<key_t, std::shared_ptr<const Sector>> _sectors;
 };

 #endif
--- a/lib/logger.cc
+++ b/lib/logger.cc
@@ -88,12 +88,12 @@ std::string Logger::toString(const AnyLogMessage& message)
                indent();
                stream << "sectors:";

-                std::vector<std::shared_ptr<Sector>> sectors(
+                std::vector<std::shared_ptr<const Sector>> sectors(
                    m.sectors.begin(), m.sectors.end());
                std::sort(sectors.begin(),
                    sectors.end(),
-                    [](const std::shared_ptr<Sector>& s1,
-                        const std::shared_ptr<Sector>& s2)
+                    [](const std::shared_ptr<const Sector>& s1,
+                        const std::shared_ptr<const Sector>& s2)
                    {
                        return s1->logicalSector < s2->logicalSector;
                    });
--- a/lib/logger.h
+++ b/lib/logger.h
@@ -21,8 +21,8 @@ struct DiskContextLogMessage

 struct SingleReadLogMessage
 {
-	std::shared_ptr<TrackDataFlux> trackDataFlux;
-	std::set<std::shared_ptr<Sector>> sectors;
+	std::shared_ptr<const TrackDataFlux> trackDataFlux;
+	std::set<std::shared_ptr<const Sector>> sectors;
 };

 struct TrackReadLogMessage
--- a/lib/reader.cc
+++ b/lib/reader.cc
@@ -21,14 +21,17 @@

 static std::unique_ptr<FluxSink> outputFluxSink;

-static std::shared_ptr<Fluxmap> readFluxmap(FluxSource& fluxsource, unsigned cylinder, unsigned head)
+static std::shared_ptr<Fluxmap> readFluxmap(
+    FluxSource& fluxsource, unsigned cylinder, unsigned head)
 {
    Logger() << DiskContextLogMessage{cylinder, head}
             << BeginReadOperationLogMessage();
    std::shared_ptr<Fluxmap> fluxmap = fluxsource.readFlux(cylinder, head);
    fluxmap->rescale(1.0 / config.flux_source().rescale());
    Logger() << EndReadOperationLogMessage()
-			 << fmt::format("{0:.0} ms in {1} bytes", fluxmap->duration()/1e6, fluxmap->bytes());
+             << fmt::format("{0:.0} ms in {1} bytes",
+                    fluxmap->duration() / 1e6,
+                    fluxmap->bytes());
    return fluxmap;
 }

@@ -37,35 +40,56 @@ static bool conflictable(Sector::Status status)
    return (status == Sector::OK) || (status == Sector::CONFLICT);
 }

-static std::set<std::shared_ptr<Sector>> collect_sectors(std::set<std::shared_ptr<Sector>>& track_sectors)
+static std::set<std::shared_ptr<const Sector>> collect_sectors(
+    std::set<std::shared_ptr<const Sector>>& track_sectors)
 {
    typedef std::tuple<unsigned, unsigned, unsigned> key_t;
-	std::map<key_t, std::shared_ptr<Sector>> sectors;
+    std::multimap<key_t, std::shared_ptr<const Sector>> sectors;

-	for (auto& replacement : track_sectors)
+    for (const auto& sector : track_sectors)
    {
-		key_t sectorid = {replacement->logicalTrack, replacement->logicalSide, replacement->logicalSector};
-		auto replacing = sectors[sectorid];
-		if (replacing && conflictable(replacing->status) && conflictable(replacement->status))
-		{
-			if (replacement->data != replacing->data)
-			{
-				Logger() << fmt::format("multiple conflicting copies of sector {} seen",
-						std::get<2>(sectorid));
-				replacing->status = replacement->status = Sector::CONFLICT;
-			}
-		}
-		if (!replacing || ((replacing->status != Sector::OK) && (replacement->status == Sector::OK)))
-			sectors[sectorid] = replacement;
+        key_t sectorid = {
+            sector->logicalTrack, sector->logicalSide, sector->logicalSector};
+        sectors.insert({sectorid, sector});
    }

-	std::set<std::shared_ptr<Sector>> sector_set;
-	for (auto& i : sectors)
-		sector_set.insert(i.second);
+    std::set<std::shared_ptr<const Sector>> sector_set;
+    auto it = sectors.begin();
+    while (it != sectors.end())
+    {
+        auto ub = sectors.upper_bound(it->first);
+        auto new_sector = std::accumulate(it,
+            ub,
+            it->second,
+            [&](auto left, auto& rightit) -> std::shared_ptr<const Sector>
+            {
+                auto& right = rightit.second;
+                if ((left->status == Sector::OK) &&
+                    (right->status == Sector::OK) &&
+                    (left->data != right->data))
+                {
+                    auto s = std::make_shared<Sector>(*left);
+                    s->status = Sector::CONFLICT;
+                    return s;
+                }
+                if (left->status == Sector::CONFLICT)
+                    return left;
+                if (right->status == Sector::CONFLICT)
+                    return right;
+                if (left->status == Sector::OK)
+                    return left;
+                if (right->status == Sector::OK)
+                    return right;
+                return left;
+            });
+        sector_set.insert(new_sector);
+        it = ub;
+    }
    return sector_set;
 }

-void readDiskCommand(FluxSource& fluxsource, AbstractDecoder& decoder, ImageWriter& writer)
+void readDiskCommand(
+    FluxSource& fluxsource, AbstractDecoder& decoder, ImageWriter& writer)
 {
    if (config.decoder().has_copy_flux_to())
        outputFluxSink = FluxSink::create(config.decoder().copy_flux_to());
@@ -77,8 +101,8 @@ void readDiskCommand(FluxSource& fluxsource, AbstractDecoder& decoder, ImageWrit
        for (int head : iterate(config.heads()))
        {
            auto track = std::make_shared<TrackFlux>();
-			std::set<std::shared_ptr<Sector>> track_sectors;
-			std::set<std::shared_ptr<Record>> track_records;
+            std::set<std::shared_ptr<const Sector>> track_sectors;
+            std::set<std::shared_ptr<const Record>> track_records;
            Fluxmap totalFlux;

            for (int retry = config.decoder().retries(); retry >= 0; retry--)
@@ -86,18 +110,20 @@ void readDiskCommand(FluxSource& fluxsource, AbstractDecoder& decoder, ImageWrit
                auto fluxmap = readFluxmap(fluxsource, cylinder, head);
                totalFlux.appendDesync().appendBytes(fluxmap->rawBytes());

-				auto trackdataflux = decoder.decodeToSectors(fluxmap, cylinder, head);
+                auto trackdataflux =
+                    decoder.decodeToSectors(fluxmap, cylinder, head);
                track->trackDatas.push_back(trackdataflux);

-				track_sectors.insert(trackdataflux->sectors.begin(), trackdataflux->sectors.end());
-				track_records.insert(trackdataflux->records.begin(), trackdataflux->records.end());
-
-				auto collected_sectors = collect_sectors(track_sectors);
+                track_sectors.insert(trackdataflux->sectors.begin(),
+                    trackdataflux->sectors.end());
+                track_records.insert(trackdataflux->records.begin(),
+                    trackdataflux->records.end());

                bool hasBadSectors = false;
-				std::set<unsigned> required_sectors = decoder.requiredSectors(cylinder, head);
-				std::set<std::shared_ptr<Sector>> result_sectors;
-				for (const auto& sector : collected_sectors)
+                std::set<unsigned> required_sectors =
+                    decoder.requiredSectors(cylinder, head);
+                std::set<std::shared_ptr<const Sector>> result_sectors;
+                for (const auto& sector : collect_sectors(track_sectors))
                {
                    result_sectors.insert(sector);
                    required_sectors.erase(sector->logicalSector);
@@ -128,7 +154,8 @@ void readDiskCommand(FluxSource& fluxsource, AbstractDecoder& decoder, ImageWrit
                if (retry == 0)
                    Logger() << fmt::format("giving up");
                else
-					Logger() << fmt::format("retrying; {} retries remaining", retry);
+                    Logger()
+                        << fmt::format("retrying; {} retries remaining", retry);
            }

            if (outputFluxSink)
@@ -152,7 +179,9 @@ void readDiskCommand(FluxSource& fluxsource, AbstractDecoder& decoder, ImageWrit
                std::cout << "\nDecoded sectors follow:\n\n";
                for (const auto& sector : track_sectors)
                {
-					std::cout << fmt::format("{}.{:02}.{:02}: I+{:.2f}us with {:.2f}us clock: status {}\n",
+                    std::cout << fmt::format(
+                        "{}.{:02}.{:02}: I+{:.2f}us with {:.2f}us clock: "
+                        "status {}\n",
                        sector->logicalTrack,
                        sector->logicalSide,
                        sector->logicalSector,
@@ -164,13 +193,15 @@ void readDiskCommand(FluxSource& fluxsource, AbstractDecoder& decoder, ImageWrit
                }
            }

-			track->sectors = collect_sectors(track_sectors);
+            for (const auto& sector : collect_sectors(track_sectors))
+                track->sectors.insert(sector);
+
            Logger() << TrackReadLogMessage{track};
            diskflux->tracks.push_back(track);
        }
    }

-	std::set<std::shared_ptr<Sector>> all_sectors;
+    std::set<std::shared_ptr<const Sector>> all_sectors;
    for (auto& track : diskflux->tracks)
        for (auto& sector : track->sectors)
            all_sectors.insert(sector);
--- a/lib/sector.cc
+++ b/lib/sector.cc
@@ -6,12 +6,18 @@ std::string Sector::statusToString(Status status)
 {
    switch (status)
    {
-        case Status::OK:           return "OK";
-        case Status::BAD_CHECKSUM: return "bad checksum";
-        case Status::MISSING:      return "sector not found";
-        case Status::DATA_MISSING: return "present but no data found";
-        case Status::CONFLICT:     return "conflicting data";
-        default:                   return fmt::format("unknown error {}", status);
+        case Status::OK:
+            return "OK";
+        case Status::BAD_CHECKSUM:
+            return "bad checksum";
+        case Status::MISSING:
+            return "sector not found";
+        case Status::DATA_MISSING:
+            return "present but no data found";
+        case Status::CONFLICT:
+            return "conflicting data";
+        default:
+            return fmt::format("unknown error {}", status);
    }
 }

@@ -19,12 +25,18 @@ std::string Sector::statusToChar(Status status)
 {
    switch (status)
    {
-        case Status::OK:           return "";
-        case Status::MISSING:      return "?";
-        case Status::BAD_CHECKSUM: return "!";
-        case Status::DATA_MISSING: return "!";
-        case Status::CONFLICT:     return "*";
-        default:                   return "?";
+        case Status::OK:
+            return "";
+        case Status::MISSING:
+            return "?";
+        case Status::BAD_CHECKSUM:
+            return "!";
+        case Status::DATA_MISSING:
+            return "!";
+        case Status::CONFLICT:
+            return "*";
+        default:
+            return "?";
    }
 }

@@ -43,12 +55,14 @@ Sector::Status Sector::stringToStatus(const std::string& value)
    return Status::INTERNAL_ERROR;
 }

-bool sectorPointerSortPredicate(std::shared_ptr<Sector>& lhs, std::shared_ptr<Sector>& rhs)
+bool sectorPointerSortPredicate(
+    std::shared_ptr<const Sector>& lhs, std::shared_ptr<const Sector>& rhs)
 {
    return *lhs < *rhs;
 }

-bool sectorPointerEqualsPredicate(std::shared_ptr<Sector>& lhs, std::shared_ptr<Sector>& rhs)
+bool sectorPointerEqualsPredicate(
+    std::shared_ptr<const Sector>& lhs, std::shared_ptr<const Sector>& rhs)
 {
    if (!lhs && !rhs)
        return true;
@@ -56,5 +70,3 @@ bool sectorPointerEqualsPredicate(std::shared_ptr<Sector>& lhs, std::shared_ptr<
        return false;
    return *lhs == *rhs;
 }
-
-
--- a/lib/sector.h
+++ b/lib/sector.h
@@ -44,21 +44,30 @@ public:
    std::vector<std::shared_ptr<Record>> records;

    std::tuple<int, int, int, Status> key() const
-	{ return std::make_tuple(logicalTrack, logicalSide, logicalSector, status); }
+    {
+        return std::make_tuple(
+            logicalTrack, logicalSide, logicalSector, status);
+    }

    bool operator==(const Sector& rhs) const
-	{ return key() == rhs.key(); }
+    {
+        return key() == rhs.key();
+    }

    bool operator!=(const Sector& rhs) const
-	{ return key() != rhs.key(); }
+    {
+        return key() != rhs.key();
+    }

    bool operator<(const Sector& rhs) const
-	{ return key() < rhs.key(); }
-
+    {
+        return key() < rhs.key();
+    }
 };

-extern bool sectorPointerSortPredicate(std::shared_ptr<Sector>& lhs, std::shared_ptr<Sector>& rhs);
-extern bool sectorPointerEqualsPredicate(std::shared_ptr<Sector>& lhs, std::shared_ptr<Sector>& rhs);
+extern bool sectorPointerSortPredicate(
+    std::shared_ptr<const Sector>& lhs, std::shared_ptr<const Sector>& rhs);
+extern bool sectorPointerEqualsPredicate(
+    std::shared_ptr<const Sector>& lhs, std::shared_ptr<const Sector>& rhs);

 #endif
-
--- a/lib/writer.cc
+++ b/lib/writer.cc
@@ -109,8 +109,8 @@ void writeTracksAndVerify(FluxSink& fluxSink,
                    const auto trackdata =
                        decoder.decodeToSectors(writtenFluxmap, cylinder, head);

-                    std::vector<std::shared_ptr<Sector>> gotSectors =
-                        trackdata->sectors;
+                    std::vector<std::shared_ptr<const Sector>> gotSectors(
+						trackdata->sectors.begin(), trackdata->sectors.end());
                    gotSectors.erase(std::remove_if(gotSectors.begin(),
                                         gotSectors.end(),
                                         [](const auto& s)