From ea8cd77597dea791a229152eb4e6a6062a532b72 Mon Sep 17 00:00:00 2001 From: Jeremy Powell Date: Mon, 30 Sep 2024 13:23:49 +1300 Subject: [PATCH 1/4] Make CompoundFile.Dispose public Allow disposal without a cast --- sources/OpenMcdf/CompoundFile.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sources/OpenMcdf/CompoundFile.cs b/sources/OpenMcdf/CompoundFile.cs index f8e1f76..b5e69ad 100644 --- a/sources/OpenMcdf/CompoundFile.cs +++ b/sources/OpenMcdf/CompoundFile.cs @@ -2489,8 +2489,7 @@ private void CloseCore(bool closeStream) #region IDisposable Members - - void IDisposable.Dispose() + public void Dispose() { Dispose(true); GC.SuppressFinalize(this); From 3517b4ba6cf789a2ed17f511400312040a5db813 Mon Sep 17 00:00:00 2001 From: Jeremy Powell Date: Mon, 30 Sep 2024 14:30:02 +1300 Subject: [PATCH 2/4] Add ContainsStream/Storage Allow checking to see whether a particular Stream or Storage already exists --- sources/OpenMcdf/CFStorage.cs | 128 ++++++++++------------------------ 1 file changed, 37 insertions(+), 91 deletions(-) diff --git a/sources/OpenMcdf/CFStorage.cs b/sources/OpenMcdf/CFStorage.cs index 541bb9c..87d07f1 100644 --- a/sources/OpenMcdf/CFStorage.cs +++ b/sources/OpenMcdf/CFStorage.cs @@ -146,6 +146,23 @@ public CFStream AddStream(string streamName) return new CFStream(CompoundFile, dirEntry); } + bool Contains(string name, StgType type, out IDirectoryEntry directoryEntry) + { + IDirectoryEntry tmp = DirectoryEntry.Mock(name, type); + if (!Children.TryLookup(tmp, out IRBNode node) || node is not IDirectoryEntry de || de.StgType != type) + { + directoryEntry = null; + return false; + } + + directoryEntry = de; + return true; + } + + public bool ContainsStream(string streamName) => Contains(streamName, StgType.StgStream, out _); + + public bool ContainsStorage(string storageName) => Contains(storageName, StgType.StgStorage, out _); + /// /// Get a named stream contained in the current storage if existing. /// @@ -171,22 +188,10 @@ public CFStream GetStream(string streamName) { CheckDisposed(); - IDirectoryEntry tmp = DirectoryEntry.Mock(streamName, StgType.StgStream); + if (!Contains(streamName, StgType.StgStream, out IDirectoryEntry outDe)) + throw new CFItemNotFound($"Cannot find item [{streamName}] within the current storage"); - //if (children == null) - //{ - // children = compoundFile.GetChildrenTree(SID); - //} - - - if (Children.TryLookup(tmp, out IRBNode outDe) && (((IDirectoryEntry)outDe).StgType == StgType.StgStream)) - { - return new CFStream(CompoundFile, (IDirectoryEntry)outDe); - } - else - { - throw new CFItemNotFound("Cannot find item [" + streamName + "] within the current storage"); - } + return new CFStream(CompoundFile, outDe); } /// @@ -212,28 +217,14 @@ public CFStream GetStream(string streamName) /// public bool TryGetStream(string streamName, out CFStream cfStream) { - bool result = false; - cfStream = null; - - try - { - CheckDisposed(); - - IDirectoryEntry tmp = DirectoryEntry.Mock(streamName, StgType.StgStream); - - - if (Children.TryLookup(tmp, out IRBNode outDe) && (((IDirectoryEntry)outDe).StgType == StgType.StgStream)) - { - cfStream = new CFStream(CompoundFile, (IDirectoryEntry)outDe); - result = true; - } - } - catch (CFDisposedException) + if (CompoundFile.IsClosed || !Contains(streamName, StgType.StgStream, out IDirectoryEntry directoryEntry)) { - result = false; + cfStream = null; + return false; } - return result; + cfStream = new CFStream(CompoundFile, directoryEntry); + return true; } /// @@ -259,24 +250,8 @@ public bool TryGetStream(string streamName, out CFStream cfStream) [Obsolete("Please use TryGetStream(string, out cfStream) instead.")] public CFStream TryGetStream(string streamName) { - CheckDisposed(); - - IDirectoryEntry tmp = DirectoryEntry.Mock(streamName, StgType.StgStream); - - //if (children == null) - //{ - // children = compoundFile.GetChildrenTree(SID); - //} - - - if (Children.TryLookup(tmp, out IRBNode outDe) && (((IDirectoryEntry)outDe).StgType == StgType.StgStream)) - { - return new CFStream(CompoundFile, (IDirectoryEntry)outDe); - } - else - { - return null; - } + TryGetStream(streamName, out CFStream cfStream); + return cfStream; } /// @@ -302,16 +277,10 @@ public CFStorage GetStorage(string storageName) { CheckDisposed(); - IDirectoryEntry template = DirectoryEntry.Mock(storageName, StgType.StgInvalid); + if (!Contains(storageName, StgType.StgStorage, out IDirectoryEntry outDe)) + throw new CFItemNotFound($"Cannot find item [{storageName}] within the current storage"); - if (Children.TryLookup(template, out IRBNode outDe) && ((IDirectoryEntry)outDe).StgType == StgType.StgStorage) - { - return new CFStorage(CompoundFile, outDe as IDirectoryEntry); - } - else - { - throw new CFItemNotFound("Cannot find item [" + storageName + "] within the current storage"); - } + return new CFStorage(CompoundFile, outDe); } /// @@ -335,18 +304,8 @@ public CFStorage GetStorage(string storageName) [Obsolete("Please use TryGetStorage(string, out cfStorage) instead.")] public CFStorage TryGetStorage(string storageName) { - CheckDisposed(); - - IDirectoryEntry template = DirectoryEntry.Mock(storageName, StgType.StgInvalid); - - if (Children.TryLookup(template, out IRBNode outDe) && ((IDirectoryEntry)outDe).StgType == StgType.StgStorage) - { - return new CFStorage(CompoundFile, outDe as IDirectoryEntry); - } - else - { - return null; - } + TryGetStorage(storageName, out CFStorage cfStorage); + return cfStorage; } /// @@ -371,27 +330,14 @@ public CFStorage TryGetStorage(string storageName) /// public bool TryGetStorage(string storageName, out CFStorage cfStorage) { - bool result = false; - cfStorage = null; - - try - { - CheckDisposed(); - - IDirectoryEntry template = DirectoryEntry.Mock(storageName, StgType.StgInvalid); - - if (Children.TryLookup(template, out IRBNode outDe) && ((IDirectoryEntry)outDe).StgType == StgType.StgStorage) - { - cfStorage = new CFStorage(CompoundFile, outDe as IDirectoryEntry); - result = true; - } - } - catch (CFDisposedException) + if (CompoundFile.IsClosed || !Contains(storageName, StgType.StgStorage, out IDirectoryEntry directoryEntry)) { - result = false; + cfStorage = null; + return false; } - return result; + cfStorage = new CFStorage(CompoundFile, directoryEntry); + return true; } /// From b7af36b1e26d417aaa66666fa2f18219d96f7ee3 Mon Sep 17 00:00:00 2001 From: Jeremy Powell Date: Mon, 30 Sep 2024 15:43:04 +1300 Subject: [PATCH 3/4] Fix RBTree enumeration Implement IEnumerable to allow generic enumeration. Also use a List for the enumerator to avoid internal multiple enumeration with LINQ. (i.e. don't use ElementAt) --- sources/OpenMcdf/RBTree/RBTree.cs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/sources/OpenMcdf/RBTree/RBTree.cs b/sources/OpenMcdf/RBTree/RBTree.cs index 4315167..577fb69 100644 --- a/sources/OpenMcdf/RBTree/RBTree.cs +++ b/sources/OpenMcdf/RBTree/RBTree.cs @@ -1,6 +1,7 @@ #define ASSERT using System; +using System.Collections; using System.Collections.Generic; using System.Linq; @@ -88,7 +89,7 @@ Color Color void AssignValueTo(IRBNode other); } - public class RBTree + public class RBTree : IEnumerable { public IRBNode Root { get; set; } @@ -513,25 +514,25 @@ private void DoVisitTreeNodes(Action action, IRBNode walker) public class RBTreeEnumerator : IEnumerator { int position = -1; - private readonly Queue heap = new Queue(); + private readonly List list = new(); internal RBTreeEnumerator(RBTree tree) { - tree.VisitTreeNodes(item => heap.Enqueue(item)); + tree.VisitTreeNodes(item => list.Add(item)); } - public IRBNode Current => heap.ElementAt(position); - public void Dispose() { } - object System.Collections.IEnumerator.Current => heap.ElementAt(position); + public IRBNode Current => list[position]; + + object IEnumerator.Current => list[position]; public bool MoveNext() { position++; - return position < heap.Count; + return position < list.Count; } public void Reset() @@ -540,10 +541,9 @@ public void Reset() } } - public RBTreeEnumerator GetEnumerator() - { - return new RBTreeEnumerator(this); - } + public IEnumerator GetEnumerator() => new RBTreeEnumerator(this); + + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); private const int INDENT_STEP = 15; From 6d6a5151267c30d817ce134e9f6ad0a4554d9038 Mon Sep 17 00:00:00 2001 From: Jeremy Powell Date: Mon, 30 Sep 2024 14:36:22 +1300 Subject: [PATCH 4/4] Add TryDelete Also, avoid redundant casts and lookups --- sources/OpenMcdf/CFStorage.cs | 76 +++++++++++++---------------- sources/OpenMcdf/CompoundFile.cs | 27 ---------- sources/OpenMcdf/DirectoryEntry.cs | 21 ++++++++ sources/OpenMcdf/IDirectoryEntry.cs | 1 + 4 files changed, 55 insertions(+), 70 deletions(-) diff --git a/sources/OpenMcdf/CFStorage.cs b/sources/OpenMcdf/CFStorage.cs index 87d07f1..0a3a262 100644 --- a/sources/OpenMcdf/CFStorage.cs +++ b/sources/OpenMcdf/CFStorage.cs @@ -9,6 +9,7 @@ using RedBlackTree; using System; using System.Collections.Generic; +using System.Linq; namespace OpenMcdf { @@ -138,7 +139,7 @@ public CFStream AddStream(string streamName) } catch (RBTreeException) { - CompoundFile.ResetDirectoryEntry(dirEntry.SID); + dirEntry.Reset(); throw new CFDuplicatedItemException("An entry with name '" + streamName + "' is already present in storage '" + Name + "' "); } @@ -384,7 +385,7 @@ IDirectoryEntry cfo } catch (RBTreeDuplicatedItemException) { - CompoundFile.ResetDirectoryEntry(cfo.SID); + cfo.Reset(); throw new CFDuplicatedItemException("An entry with name '" + storageName + "' is already present in storage '" + Name + "' "); } @@ -470,95 +471,84 @@ List subStorages /// Raised if trying to delete item from a closed compound file /// Raised if item to delete is not found /// Raised if trying to delete root storage - public void Delete(string entryName) + public void Delete(string entryName) => DeleteCore(entryName, true); + + public void TryDelete(string entryName) => DeleteCore(entryName, false); + + bool DeleteCore(string entryName, bool throwOnError) { - CheckDisposed(); + if (CompoundFile.IsClosed) + return throwOnError ? throw new CFDisposedException("Owner Compound file has been closed and owned items have been invalidated") : false; // Find entry to delete IDirectoryEntry tmp = DirectoryEntry.Mock(entryName, StgType.StgInvalid); - - Children.TryLookup(tmp, out IRBNode foundObj); if (foundObj == null) - throw new CFItemNotFound("Entry named [" + entryName + "] was not found"); - - //if (foundObj.GetType() != typeCheck) - // throw new CFException("Entry named [" + entryName + "] has not the correct type"); + return throwOnError ? throw new CFItemNotFound($"Entry named [{entryName}] was not found") : false; - if (((IDirectoryEntry)foundObj).StgType == StgType.StgRoot) - throw new CFException("Root storage cannot be removed"); + IDirectoryEntry directoryEntry = (IDirectoryEntry)foundObj; + DeleteCore(directoryEntry); + return true; + } + void DeleteCore(IDirectoryEntry directoryEntry) + { IRBNode altDel; - switch (((IDirectoryEntry)foundObj).StgType) + switch (directoryEntry.StgType) { + case StgType.StgRoot: + throw new CFException("Root storage cannot be removed"); + case StgType.StgStorage: - CFStorage temp = new CFStorage(CompoundFile, (IDirectoryEntry)foundObj); + CFStorage temp = new CFStorage(CompoundFile, directoryEntry); // This is a storage. we have to remove children items first - foreach (IRBNode de in temp.Children) + foreach (IDirectoryEntry de in temp.Children.Cast()) { - IDirectoryEntry ded = de as IDirectoryEntry; - temp.Delete(ded.Name); + temp.DeleteCore(de); } // ...then we Remove storage item from children tree... - Children.Delete(foundObj, out altDel); + Children.Delete(directoryEntry, out altDel); // ...after which we need to rethread the root of siblings tree... - if (Children.Root != null) - DirEntry.Child = (Children.Root as IDirectoryEntry).SID; - else - DirEntry.Child = DirectoryEntry.NOSTREAM; + DirEntry.Child = Children.Root == null ? DirectoryEntry.NOSTREAM : ((IDirectoryEntry)Children.Root).SID; // ...and remove directory (storage) entry if (altDel != null) { - foundObj = altDel; + directoryEntry = (IDirectoryEntry)altDel; } - CompoundFile.InvalidateDirectoryEntry(((IDirectoryEntry)foundObj).SID); + directoryEntry.Reset(); break; case StgType.StgStream: // Free directory associated data stream. - CompoundFile.FreeAssociatedData((foundObj as IDirectoryEntry).SID); + CompoundFile.FreeAssociatedData(directoryEntry.SID); // Remove item from children tree - Children.Delete(foundObj, out altDel); + Children.Delete(directoryEntry, out altDel); // Rethread the root of siblings tree... - if (Children.Root != null) - DirEntry.Child = (Children.Root as IDirectoryEntry).SID; - else - DirEntry.Child = DirectoryEntry.NOSTREAM; + DirEntry.Child = Children.Root == null ? DirectoryEntry.NOSTREAM : ((IDirectoryEntry)Children.Root).SID; // Delete operation could possibly have cloned a directory, changing its SID. // Invalidate the ACTUALLY deleted directory. if (altDel != null) { - foundObj = altDel; + directoryEntry = (IDirectoryEntry)altDel; } - CompoundFile.InvalidateDirectoryEntry(((IDirectoryEntry)foundObj).SID); + directoryEntry.Reset(); break; } - - //// Refresh recursively all SIDs (invariant for tree sorting) - //VisitedEntryAction action = delegate(CFSItem target) - //{ - // if( ((IDirectoryEntry)target).SID>foundObj.SID ) - // { - // ((IDirectoryEntry)target).SID--; - // } - - // ((IDirectoryEntry)target).LeftSibling--; - //}; } /// diff --git a/sources/OpenMcdf/CompoundFile.cs b/sources/OpenMcdf/CompoundFile.cs index b5e69ad..53062c5 100644 --- a/sources/OpenMcdf/CompoundFile.cs +++ b/sources/OpenMcdf/CompoundFile.cs @@ -1507,26 +1507,6 @@ internal List GetSectorChain(int secID, SectorType chainType) public CFSVersion Version => (CFSVersion)header.MajorVersion; - /// - /// Reset a directory entry setting it to StgInvalid in the Directory. - /// - /// Sid of the directory to invalidate - internal void ResetDirectoryEntry(int sid) - { - directoryEntries[sid].SetEntryName(string.Empty); - directoryEntries[sid].Left = null; - directoryEntries[sid].Right = null; - directoryEntries[sid].Parent = null; - directoryEntries[sid].StgType = StgType.StgInvalid; - directoryEntries[sid].StartSetc = DirectoryEntry.ZERO; - directoryEntries[sid].StorageCLSID = Guid.Empty; - directoryEntries[sid].Size = 0; - directoryEntries[sid].StateBits = 0; - directoryEntries[sid].StgColor = StgColor.Red; - directoryEntries[sid].CreationDate = new byte[8]; - directoryEntries[sid].ModifyDate = new byte[8]; - } - //internal class NodeFactory : IRBTreeDeserializer //{ @@ -2416,13 +2396,6 @@ private static int LowSaturation(int i) return i > 0 ? i : 0; } - internal void InvalidateDirectoryEntry(int sid) - { - if (sid >= directoryEntries.Count) - throw new CFException("Invalid SID of the directory entry to remove"); - - ResetDirectoryEntry(sid); - } internal void FreeAssociatedData(int sid) { diff --git a/sources/OpenMcdf/DirectoryEntry.cs b/sources/OpenMcdf/DirectoryEntry.cs index aaa9b51..7d10715 100644 --- a/sources/OpenMcdf/DirectoryEntry.cs +++ b/sources/OpenMcdf/DirectoryEntry.cs @@ -455,5 +455,26 @@ public void AssignValueTo(RedBlackTree.IRBNode other) d.storageCLSID = new Guid(storageCLSID.ToByteArray()); d.Child = Child; } + + /// + /// Reset a directory entry setting it to StgInvalid in the Directory. + /// + public void Reset() + { + // TODO: Delete IDirectoryEntry interface and use as DirectoryEntry + // member instead for improved performance from devirtualization + SetEntryName(string.Empty); + Left = null; + Right = null; + Parent = null; + StgType = StgType.StgInvalid; + StartSetc = ZERO; + StorageCLSID = Guid.Empty; + Size = 0; + StateBits = 0; + StgColor = StgColor.Red; + Array.Clear(CreationDate, 0, CreationDate.Length); + Array.Clear(ModifyDate, 0, ModifyDate.Length); + } } } diff --git a/sources/OpenMcdf/IDirectoryEntry.cs b/sources/OpenMcdf/IDirectoryEntry.cs index 231df12..1ad8a12 100644 --- a/sources/OpenMcdf/IDirectoryEntry.cs +++ b/sources/OpenMcdf/IDirectoryEntry.cs @@ -32,5 +32,6 @@ internal interface IDirectoryEntry : IComparable, IRBNode StgType StgType { get; set; } Guid StorageCLSID { get; set; } void Write(System.IO.Stream stream); + void Reset(); } }