ramfs: Handle renames replacing existing files properly.

Previously, Entry::Link() behaved incorrectly in this case:
it would unlink the old Node before linking the new one.
But there's only one DoublyLinkedListLink inside Entry
for the Node to use, so this would clobber the lists
and thus produce KDLs.

Instead, make Link fail if there's already a node, and
thus force the caller to Unlink first. For now, just use
a "naive" implementation of this in the one case in
rename(); in the future we could make it more robust if
necessary.

Fixes the other KDL in #19583.
This commit is contained in:
Augustin Cavalier
2025-05-28 16:13:57 -04:00
parent 41b15d5fe4
commit 90f095da8d
3 changed files with 20 additions and 18 deletions
+16 -13
View File
@@ -36,7 +36,7 @@ Entry::InitCheck() const
return (fName.GetString() ? B_OK : B_NO_INIT); return (fName.GetString() ? B_OK : B_NO_INIT);
} }
// Link
status_t status_t
Entry::Link(Node *node) Entry::Link(Node *node)
{ {
@@ -45,25 +45,28 @@ Entry::Link(Node *node)
if (node == fNode) if (node == fNode)
return B_OK; return B_OK;
// We first link to the new node and then unlink the old one. // We can only be linked to one Node at a time, so force callers
Node *oldNode = fNode; // to decide what to do when we're already linked to a Node.
status_t error = node->Link(this); if (fNode != NULL)
if (error == B_OK) { return B_BAD_VALUE;
status_t status = node->Link(this);
if (status == B_OK)
fNode = node; fNode = node;
if (oldNode) return status;
oldNode->Unlink(this);
}
return error;
} }
// Unlink
status_t status_t
Entry::Unlink() Entry::Unlink()
{ {
status_t error = (fNode ? B_OK : B_BAD_VALUE); if (fNode == NULL)
if (error == B_OK && (error = fNode->Unlink(this)) == B_OK) return B_BAD_VALUE;
status_t status = fNode->Unlink(this);
if (status == B_OK)
fNode = NULL; fNode = NULL;
return error; return status;
} }
// SetName // SetName
@@ -29,7 +29,6 @@ public:
inline void SetParent(Directory *parent) { fParent = parent; } inline void SetParent(Directory *parent) { fParent = parent; }
Directory *GetParent() const { return fParent; } Directory *GetParent() const { return fParent; }
// inline void SetNode(Node *node) { fNode = node; }
status_t Link(Node *node); status_t Link(Node *node);
status_t Unlink(); status_t Unlink();
Node *GetNode() const { return fNode; } Node *GetNode() const { return fNode; }
@@ -37,8 +36,6 @@ public:
status_t SetName(const char *newName); status_t SetName(const char *newName);
inline const char *GetName() const { return fName.GetString(); } inline const char *GetName() const { return fName.GetString(); }
// inline Volume *GetVolume() const { return fVolume; }
inline DoublyLinkedListLink<Entry> *GetReferrerLink() inline DoublyLinkedListLink<Entry> *GetReferrerLink()
{ return &fReferrerLink; } { return &fReferrerLink; }
@@ -57,6 +54,7 @@ private:
Node *fNode; Node *fNode;
String fName; String fName;
DoublyLinkedListLink<Entry> fReferrerLink; DoublyLinkedListLink<Entry> fReferrerLink;
// iterator management // iterator management
DoublyLinkedList<EntryIterator> fIterators; DoublyLinkedList<EntryIterator> fIterators;
}; };
@@ -622,9 +622,10 @@ ramfs_rename(fs_volume* _volume, fs_vnode* _oldDir, const char *oldName,
error = oldDir->DeleteEntry(entry); error = oldDir->DeleteEntry(entry);
if (error == B_OK) { if (error == B_OK) {
// create the new one/relink the target entry // create the new one/relink the target entry
if (clobberEntry) if (clobberEntry != NULL) {
clobberEntry->Unlink();
error = clobberEntry->Link(node); error = clobberEntry->Link(node);
else } else
error = newDir->CreateEntry(node, newName); error = newDir->CreateEntry(node, newName);
if (error == B_OK) { if (error == B_OK) {