Merge pull request #637 from davidgiven/cpm

Fix an issue with extent handling in the CP/M file system.

.clang-format

@@ -18,17 +18,19 @@ AlwaysBreakBeforeMultilineStrings: 'true'
 AlwaysBreakTemplateDeclarations: 'Yes'
 BinPackArguments: 'false'
 BinPackParameters: 'false'
-BreakConstructorInitializers: 'AfterColon'
 BreakBeforeBraces: Allman
+BreakConstructorInitializers: 'AfterColon'
 BreakInheritanceList: AfterColon
 BreakStringLiterals: 'true'
-IndentCaseLabels: 'true'
-IndentWidth: '4'
 ColumnLimit: '80'
 ConstructorInitializerAllOnOneLineOrOnePerLine: 'true'
+FixNamespaceComments: 'false'
 IncludeBlocks: Preserve
+IndentCaseLabels: 'true'
+IndentWidth: '4'
 IndentWrappedFunctionNames: 'false'
 KeepEmptyLinesAtTheStartOfBlocks: 'true'
+NamespaceIndentation: All
 PointerAlignment: Left
 ReflowComments: 'true'
 SortIncludes: 'false'

lib/bytes.cc

@@ -215,6 +215,24 @@ Bytes Bytes::reverseBits() const
     return output;
 }
 
+Bytes Bytes::operator + (const Bytes& other)
+{
+    Bytes output;
+    ByteWriter bw(output);
+    bw += *this;
+    bw += other;
+    return output;
+}
+
+Bytes Bytes::operator * (size_t count)
+{
+    Bytes output;
+    ByteWriter bw(output);
+    while (count--)
+        bw += *this;
+    return output;
+}
+
 uint8_t toByte(
     std::vector<bool>::const_iterator start,
     std::vector<bool>::const_iterator end)

lib/bytes.h

@@ -64,6 +64,9 @@ public:
 	std::vector<bool> toBits() const;
 	Bytes reverseBits() const;
 
+	Bytes operator + (const Bytes& other);
+	Bytes operator * (size_t count);
+
     ByteReader reader() const;
     ByteWriter writer();
 

lib/flags.cc

@@ -244,7 +244,6 @@ ConfigProto FlagGroup::parseSingleConfigFile(const std::string& filename,
         ss << f.rdbuf();
 
         ConfigProto config;
-        std::cout << ss.str() << '\n';
         if (!google::protobuf::TextFormat::MergeFromString(ss.str(), &config))
             Error() << "couldn't load external config proto";
         return config;

lib/vfs/cpmfs.cc

@@ -188,7 +188,6 @@ public:
             /* Find a directory entry for this logical extent. */
 
             std::unique_ptr<Entry> entry;
-            bool moreExtents = false;
             for (int d = 0; d < _config.dir_entries(); d++)
             {
                 entry = getEntry(d);
@@ -196,9 +195,10 @@ public:
                     continue;
                 if (path[0] != entry->filename)
                     continue;
-                if (entry->extent > logicalExtent)
-                    moreExtents = true;
-                if (entry->extent == logicalExtent)
+                if (entry->extent < logicalExtent)
+                    continue;
+                if ((entry->extent & ~_logicalExtentMask) ==
+                    (logicalExtent & ~_logicalExtentMask))
                     break;
             }
 
@@ -212,10 +212,10 @@ public:
             /* Copy the data out. */
 
             int i =
-                (entry->extent & ~_logicalExtentMask) * _blocksPerLogicalExtent;
+                (logicalExtent & _logicalExtentMask) * _blocksPerLogicalExtent;
             unsigned records =
                 (entry->extent == logicalExtent) ? entry->records : 128;
-            while ((records != 0) && (i != entry->allocation_map.size()))
+            while (records != 0)
             {
                 Bytes block;
                 unsigned blockid = entry->allocation_map[i];
@@ -265,7 +265,7 @@ private:
             physicalExtentSize = _config.block_size() * 8;
         }
         _logicalExtentsPerEntry = physicalExtentSize / 16384;
-        _logicalExtentMask = (1 << _logicalExtentsPerEntry) - 1;
+        _logicalExtentMask = _logicalExtentsPerEntry - 1;
         _blocksPerLogicalExtent = 16384 / _config.block_size();
 
         _directory = getCpmBlock(0, _dirBlocks);

tests/build.mk

@@ -27,6 +27,7 @@ $(call declare-test,applesingle)
 $(call declare-test,bitaccumulator)
 $(call declare-test,bytes)
 $(call declare-test,compression)
+$(call declare-test,cpmfs)
 $(call declare-test,csvreader)
 $(call declare-test,flags)
 $(call declare-test,fluxmapreader)

tests/cpmfs.cc

@@ -0,0 +1,149 @@
+#include "globals.h"
+#include "lib/vfs/vfs.h"
+#include "lib/vfs/sectorinterface.h"
+#include "lib/vfs/vfs.pb.h"
+#include "lib/image.h"
+#include "lib/proto.h"
+#include "lib/sector.h"
+#include "snowhouse/snowhouse.h"
+#include <google/protobuf/text_format.h>
+
+using namespace snowhouse;
+
+static Bytes blank_dirent = Bytes{0xe5} * 32;
+
+namespace
+{
+    class TestSectorInterface : public SectorInterface
+    {
+    public:
+        std::shared_ptr<const Sector> get(
+            unsigned track, unsigned side, unsigned sectorId)
+        {
+            auto s = _image.get(track, side, sectorId);
+            if (!s)
+                Error() << fmt::format(
+                    "missing sector c{}.h{}.s{}", track, side, sectorId);
+            return s;
+        }
+
+        std::shared_ptr<Sector> put(
+            unsigned track, unsigned side, unsigned sectorId)
+        {
+            return _image.put(track, side, sectorId);
+        }
+
+    private:
+        Image _image;
+    };
+}
+
+static Bytes createDirent(const std::string& filename,
+    int extent,
+	int records,
+    const std::initializer_list<int> blocks)
+{
+    Bytes dirent;
+    ByteWriter bw(dirent);
+    bw.write_8(0);
+    bw.append(filename);
+    while (bw.pos != 12)
+        bw.write_8(' ');
+
+    bw.write_8(extent & 0x1f);
+    bw.write_8(0);
+    bw.write_8(extent >> 5);
+    bw.write_8(records);
+
+    for (int block : blocks)
+        bw.write_8(block);
+    while (bw.pos != 32)
+        bw.write_8(0);
+
+    return dirent;
+}
+
+static void setBlock(const std::shared_ptr<SectorInterface>& sectors, int block, Bytes data)
+{
+	for (int i=0; i<8; i++)
+		sectors->put(block, 0, i)->data = data.slice(i*256, 256);
+}
+
+static void testPartialExtent()
+{
+    auto sectors = std::make_shared<TestSectorInterface>();
+    auto fs = Filesystem::createCpmFsFilesystem(config.filesystem(), sectors);
+
+	setBlock(sectors, 0, 
+        createDirent("FILE", 0, 1, {1, 0, 0, 0, 0, 0, 0, 0, 0}) +
+        (blank_dirent * 63));
+	setBlock(sectors, 1, {1});
+
+    auto files = fs->list(Path());
+    AssertThat(files.size(), Equals(1));
+
+    auto data = fs->getFile(Path("0:FILE"));
+    AssertThat(data.size(), Equals(128));
+    AssertThat(data[0x4000 * 0], Equals(1));
+}
+
+static void testLogicalExtents()
+{
+    auto sectors = std::make_shared<TestSectorInterface>();
+    auto fs = Filesystem::createCpmFsFilesystem(config.filesystem(), sectors);
+
+	setBlock(sectors, 0, 
+        createDirent("FILE", 1, 128, {1, 0, 0, 0, 0, 0, 0, 0, 2}) +
+        createDirent("FILE", 2, 128, {3}) + (blank_dirent * 62));
+	setBlock(sectors, 1, {1});
+	setBlock(sectors, 2, {2});
+	setBlock(sectors, 3, {3});
+
+    auto files = fs->list(Path());
+    AssertThat(files.size(), Equals(1));
+
+    auto data = fs->getFile(Path("0:FILE"));
+    AssertThat(data.size(), Equals(0x4000 * 3));
+    AssertThat(data[0x4000 * 0], Equals(1));
+    AssertThat(data[0x4000 * 1], Equals(2));
+    AssertThat(data[0x4000 * 2], Equals(3));
+}
+
+int main(void)
+{
+    try
+    {
+        const std::string text = R"M(
+			layout {
+				tracks: 10
+				sides: 1
+				layoutdata {
+					sector_size: 256
+					physical {
+						start_sector: 0
+						count: 8
+					}
+				}
+			}
+
+			filesystem {
+				type: CPMFS
+				cpmfs {
+					block_size: 2048
+					dir_entries: 64
+				}
+			}
+		)M";
+        google::protobuf::TextFormat::MergeFromString(text, &config);
+
+		testPartialExtent();
+        testLogicalExtents();
+    }
+    catch (const ErrorException& e)
+    {
+        std::cerr << e.message << '\n';
+        exit(1);
+    }
+
+    return 0;
+}

tests/greaseweazle.cc

@@ -5,24 +5,6 @@
 #include "fluxmap.h"
 #include "lib/usb/greaseweazle.h"
 
-static Bytes operator + (const Bytes& left, const Bytes& right)
-{
-    Bytes output;
-    ByteWriter bw(output);
-    bw += left;
-    bw += right;
-    return output;
-}
-
-static Bytes operator * (const Bytes& left, size_t count)
-{
-    Bytes output;
-    ByteWriter bw(output);
-    while (count--)
-        bw += left;
-    return output;
-}
-
 #define E28(val) \
 	(1 | ((val)<<1) & 0xff), \
 	(1 | ((val)>>6) & 0xff), \

tests/kryoflux.cc

@@ -5,24 +5,6 @@
 #include "fluxmap.h"
 #include "fluxsource/kryoflux.h"
 
-static Bytes operator + (const Bytes& left, const Bytes& right)
-{
-    Bytes output;
-    ByteWriter bw(output);
-    bw += left;
-    bw += right;
-    return output;
-}
-
-static Bytes operator * (const Bytes& left, size_t count)
-{
-    Bytes output;
-    ByteWriter bw(output);
-    while (count--)
-        bw += left;
-    return output;
-}
-
 static void test_convert(const Bytes& kryofluxbytes, const Bytes& fluxmapbytes)
 {
     std::unique_ptr<Fluxmap> fluxmap = readStream(kryofluxbytes);