diff --git a/io/entry.cpp b/io/entry.cpp index 4427633..5c33246 100644 --- a/io/entry.cpp +++ b/io/entry.cpp @@ -63,24 +63,26 @@ Entry::~Entry() */ void Entry::makeLabelUnique() { - if (m_parent) { - int index = 1; - string currentLabel(label()); - checkLabel: - for (Entry *sibling : m_parent->children()) { - if (sibling != this && currentLabel == sibling->label()) { - stringstream newLabel(currentLabel); - newLabel.seekp(0, ios_base::end); - if (newLabel.tellp()) { - newLabel << ' '; - } - newLabel << ++index; - currentLabel = newLabel.str(); - goto checkLabel; - } - } - m_label = currentLabel; + if (!m_parent) { + return; } + int index = 1; + string currentLabel(label()); +checkLabel: + for (Entry *sibling : m_parent->children()) { + if (sibling == this || currentLabel != sibling->label()) { + continue; + } + stringstream newLabel(currentLabel); + newLabel.seekp(0, ios_base::end); + if (newLabel.tellp()) { + newLabel << ' '; + } + newLabel << ++index; + currentLabel = newLabel.str(); + goto checkLabel; + } + m_label = currentLabel; } /*! @@ -88,32 +90,48 @@ void Entry::makeLabelUnique() * * If an \a index is specified the entry will be inserted as child at this position. * If \a parent is nullptr, the entry will be parentless. + * + * \remarks + * - The label might be adjusted to be unique within the new parent. + * - If the entry is just moved within its current parent (\a parent equals parent()), the + * specified \a index refers doesn't take the entry itself into account as it is removed from + * the children of \a parent and then re-inserted at \a index. */ void Entry::setParent(NodeEntry *parent, int index) { - if (m_parent != parent || (m_index != index && index >= 0)) { - if (m_parent) { - m_parent->m_children.erase(m_parent->m_children.begin() + m_index); - for (auto i = m_parent->m_children.begin() + m_index; i < m_parent->m_children.end(); ++i) { - (*i)->m_index -= 1; - } - } - if (parent) { - if (index < 0 || static_cast(index) >= parent->m_children.size()) { - m_index = parent->m_children.size(); - parent->m_children.push_back(this); - } else { - for (auto i = parent->m_children.insert(parent->m_children.begin() + index, this) + 1; i != parent->m_children.end(); ++i) { - (*i)->m_index += 1; - } - m_index = index; - } - } else { - m_index = -1; - } - m_parent = parent; - makeLabelUnique(); + // skip if \a parent already assigned and the index doesn't change, too + if (m_parent == parent && !(m_index != index && index >= 0)) { + return; } + + // detach the current parent + if (m_parent) { + m_parent->m_children.erase(m_parent->m_children.begin() + m_index); + for (auto i = m_parent->m_children.begin() + m_index; i < m_parent->m_children.end(); ++i) { + (*i)->m_index -= 1; + } + } + + // attach the new parent + if (parent) { + if (index < 0 || static_cast(index) >= parent->m_children.size()) { + m_index = parent->m_children.size(); + parent->m_children.push_back(this); + } else { + for (auto i = parent->m_children.insert(parent->m_children.begin() + index, this) + 1; i != parent->m_children.end(); ++i) { + (*i)->m_index += 1; + } + m_index = index; + } + } else { + m_index = -1; + } + + // actually assign the parent + m_parent = parent; + + // ensure the label is still unique within the new parent + makeLabelUnique(); } /*! @@ -121,15 +139,14 @@ void Entry::setParent(NodeEntry *parent, int index) */ bool Entry::isIndirectChildOf(NodeEntry *entry) const { - if (parent()) { - if (parent() == entry) { - return true; - } else { - return parent()->isIndirectChildOf(entry); - } - } else { + if (!parent()) { return false; } + if (parent() == entry) { + return true; + } else { + return parent()->isIndirectChildOf(entry); + } } /*! @@ -159,7 +176,7 @@ void Entry::path(std::list &res) const */ Entry *Entry::parse(istream &stream) { - byte version = stream.peek(); + const auto version = static_cast(stream.peek()); if (denotesNodeEntry(version)) { return new NodeEntry(stream); } else { @@ -214,29 +231,28 @@ NodeEntry::NodeEntry(istream &stream) : m_expandedByDefault(true) { BinaryReader reader(&stream); - byte version = reader.readByte(); - if (denotesNodeEntry(version)) { - if (version == 0x0 || version == 0x1) { - setLabel(reader.readLengthPrefixedString()); - if (version == 0x1) { // version 0x1 has an extended header - uint16 extendedHeaderSize = reader.readUInt16BE(); - if (extendedHeaderSize >= 1) { - byte flags = reader.readByte(); - m_expandedByDefault = flags & 0x80; - extendedHeaderSize -= 1; - } - m_extendedData = reader.readString(extendedHeaderSize); - } - uint32 childCount = reader.readUInt32BE(); - for (uint32 i = 0; i < childCount; ++i) { - Entry::parse(stream)->setParent(this); - } - } else { - throw ParsingException("Entry version not supported."); - } - } else { + const byte version = reader.readByte(); + if (!denotesNodeEntry(version)) { throw ParsingException("Node entry expected."); } + if (version != 0x0 && version != 0x1) { + throw ParsingException("Entry version not supported."); + } + setLabel(reader.readLengthPrefixedString()); + // read extended header for version 0x1 + if (version == 0x1) { + uint16 extendedHeaderSize = reader.readUInt16BE(); + if (extendedHeaderSize >= 1) { + byte flags = reader.readByte(); + m_expandedByDefault = flags & 0x80; + extendedHeaderSize -= 1; + } + m_extendedData = reader.readString(extendedHeaderSize); + } + const uint32 childCount = reader.readUInt32BE(); + for (uint32 i = 0; i != childCount; ++i) { + Entry::parse(stream)->setParent(this); + } } /*! @@ -247,7 +263,7 @@ NodeEntry::NodeEntry(istream &stream) NodeEntry::NodeEntry(const NodeEntry &other) : Entry(other) { - for (Entry *otherChild : other.m_children) { + for (Entry *const otherChild : other.m_children) { Entry *clonedChild = otherChild->clone(); clonedChild->m_parent = this; clonedChild->m_index = m_children.size(); @@ -260,7 +276,7 @@ NodeEntry::NodeEntry(const NodeEntry &other) */ NodeEntry::~NodeEntry() { - for (Entry *child : m_children) { + for (Entry *const child : m_children) { child->m_parent = nullptr; delete child; } @@ -270,6 +286,7 @@ NodeEntry::~NodeEntry() * \brief Deletes children from the node entry. * \param begin Specifies the index of the first children to delete. * \param end Specifies the index after the last children to delete. + * \remarks The children are actually destructed and deallocated. */ void NodeEntry::deleteChildren(int begin, int end) { @@ -284,6 +301,10 @@ void NodeEntry::deleteChildren(int begin, int end) /*! * \brief Replaces the child \a at the specified index with the specified \a newChild. + * \remarks The current child at the specified index is not destroyed and remains without + * a parent. + * \deprecated Since this leaves a parentless entry leading to potential memory leaks, this method + * will be removed in future releases (in its current form). */ void NodeEntry::replaceChild(size_t at, Entry *newChild) { @@ -304,43 +325,46 @@ void NodeEntry::replaceChild(size_t at, Entry *newChild) */ Entry *NodeEntry::entryByPath(list &path, bool includeThis, EntryType *creationType) { - if (path.size()) { - if (includeThis) { - if (path.front() == label()) { - path.pop_front(); - } else { - return nullptr; - } - } - if (path.size()) { - for (Entry *child : m_children) { - if (path.front() == child->label()) { - path.pop_front(); - if (path.empty()) { - return child; - } else if (child->type() == EntryType::Node) { - return static_cast(child)->entryByPath(path, false, creationType); - } else { - return nullptr; // can not resolve path since an account entry can not have children - } - } - } - if (creationType) { - if (path.size() == 1) { - switch (*creationType) { - case EntryType::Account: - return new AccountEntry(path.front(), this); - case EntryType::Node: - return new NodeEntry(path.front(), this); - } - } else { - return nullptr; - } - } + if (path.empty()) { + return nullptr; + } + + // check for current instance + if (includeThis) { + if (path.front() == label()) { + path.pop_front(); } else { - return this; + return nullptr; } } + if (path.empty()) { + return this; + } + + for (Entry *const child : m_children) { + if (path.front() != child->label()) { + continue; + } + path.pop_front(); + if (path.empty()) { + return child; + } else if (child->type() == EntryType::Node) { + return static_cast(child)->entryByPath(path, false, creationType); + } else { + return nullptr; // can not resolve path since an account entry can not have children + } + } + + // create a new entry + if (!creationType || path.size() != 1) { + return nullptr; + } + switch (*creationType) { + case EntryType::Account: + return new AccountEntry(path.front(), this); + case EntryType::Node: + return new NodeEntry(path.front(), this); + } return nullptr; } @@ -359,7 +383,7 @@ void NodeEntry::make(ostream &stream) const writer.writeString(m_extendedData); } writer.writeUInt32BE(m_children.size()); - for (const Entry *child : m_children) { + for (const Entry *const child : m_children) { child->make(stream); } } @@ -393,25 +417,24 @@ AccountEntry::AccountEntry(istream &stream) { BinaryReader reader(&stream); byte version = reader.readByte(); - if (!denotesNodeEntry(version)) { - version ^= 0x80; // set bit 0 to false - if (version == 0x0 || version == 0x1) { - setLabel(reader.readLengthPrefixedString()); - if (version == 0x1) { // version 0x1 has an extended header - uint16 extendedHeaderSize = reader.readUInt16BE(); - // currently there's nothing to read here - m_extendedData = reader.readString(extendedHeaderSize); - } - uint32 fieldCount = reader.readUInt32BE(); - for (uint32 i = 0; i < fieldCount; ++i) { - m_fields.push_back(Field(this, stream)); - } - } else { - throw ParsingException("Entry version not supported."); - } - } else { + if (denotesNodeEntry(version)) { throw ParsingException("Account entry expected."); } + version ^= 0x80; // set first bit to zero + if (version != 0x0 && version != 0x1) { + throw ParsingException("Entry version not supported."); + } + setLabel(reader.readLengthPrefixedString()); + // read extended header for version 0x1 + if (version == 0x1) { + const uint16 extendedHeaderSize = reader.readUInt16BE(); + // currently there's nothing to read here + m_extendedData = reader.readString(extendedHeaderSize); + } + const uint32 fieldCount = reader.readUInt32BE(); + for (uint32 i = 0; i != fieldCount; ++i) { + m_fields.push_back(Field(this, stream)); + } } /*! diff --git a/io/entry.h b/io/entry.h index 50cb3f9..dddbc7e 100644 --- a/io/entry.h +++ b/io/entry.h @@ -42,6 +42,7 @@ public: virtual Entry *clone() const = 0; static Entry *parse(std::istream &stream); static bool denotesNodeEntry(byte version); + static constexpr EntryType denotedEntryType(byte version); protected: Entry(const std::string &label = std::string(), NodeEntry *parent = nullptr); @@ -105,7 +106,7 @@ public: virtual EntryType type() const; const std::vector &children() const; void deleteChildren(int begin, int end); - void replaceChild(size_t at, Entry *newChild); + void replaceChild(std::size_t at, Entry *newChild); Entry *entryByPath(std::list &path, bool includeThis = true, EntryType *creationType = nullptr); bool isExpandedByDefault() const; void setExpandedByDefault(bool expandedByDefault); @@ -142,6 +143,11 @@ inline bool Entry::denotesNodeEntry(byte version) return (version & 0x80) == 0; } +constexpr EntryType Entry::denotedEntryType(byte version) +{ + return (version & 0x80) == 0 ? EntryType::Node : EntryType::Account; +} + class PASSWORD_FILE_EXPORT AccountEntry : public Entry { public: AccountEntry(); diff --git a/io/field.cpp b/io/field.cpp index 8587e72..c97361f 100644 --- a/io/field.cpp +++ b/io/field.cpp @@ -36,25 +36,24 @@ Field::Field(AccountEntry *tiedAccount, const string &name, const string &value) Field::Field(AccountEntry *tiedAccount, istream &stream) { BinaryReader reader(&stream); - int version = reader.readByte(); - if (version == 0x0 || version == 0x1) { - m_name = reader.readLengthPrefixedString(); - m_value = reader.readLengthPrefixedString(); - byte type = reader.readByte(); - if (isValidType(type)) { - m_type = static_cast(type); - } else { - throw ParsingException("Field type is not supported."); - } - if (version == 0x1) { // version 0x1 has an extended header - uint16 extendedHeaderSize = reader.readUInt16BE(); - // currently there's nothing to read here - m_extendedData = reader.readString(extendedHeaderSize); - } - m_tiedAccount = tiedAccount; - } else { + const int version = reader.readByte(); + if (version != 0x0 && version != 0x1) { throw ParsingException("Field version is not supported."); } + m_name = reader.readLengthPrefixedString(); + m_value = reader.readLengthPrefixedString(); + byte type = reader.readByte(); + if (!isValidType(type)) { + throw ParsingException("Field type is not supported."); + } + m_type = static_cast(type); + // read extended header for version 0x1 + if (version == 0x1) { + const uint16 extendedHeaderSize = reader.readUInt16BE(); + // currently there's nothing to read here + m_extendedData = reader.readString(extendedHeaderSize); + } + m_tiedAccount = tiedAccount; } /*! diff --git a/io/passwordfile.h b/io/passwordfile.h index 5a8f0e5..cafbb37 100644 --- a/io/passwordfile.h +++ b/io/passwordfile.h @@ -26,6 +26,7 @@ public: void create(); void close(); void load(); + // FIXME: use flags in v4 void save(bool useEncryption = true, bool useCompression = true); void clearEntries(); void clear();