hrev54107 adds 1 changeset to branch 'master'
old head: 4835a173bdd8e3829c5c5b4ac9208d947506d9b0
new head: e1b7c1c7ac1188a3b3e1694a958042468b95e781
overview:
https://git.haiku-os.org/haiku/log/?qt=range&q=e1b7c1c7ac11+%5E4835a173bdd8
----------------------------------------------------------------------------
e1b7c1c7ac11: storage/SymLink: Fix Be API regression in ReadLink
After this patch, "UnitTester BSymLink" passes.
BSymLink::ReadLink() in BeOS would always return the length of the
link unless an error occurred. Before this patch, Haiku instead seemed
to emulate posix readlink() behavior, returning the number of bytes
copied into the output buffer.
BeOS also did not guarantee that the string written into the output
buffer is NULL terminated if the output buffer cannot contain the
entire link contents, but the Haiku implementation does since it is is
a basic safety issue.
This patch fixes this and updates the Haiku API docs to describe the
behavior explicitly.
Fixing this required changing behavior in bfs_read_link, which
required changes in many more places.
docs/user/storage/SymLink.dox:
src/kits/storage/SymLink.cpp:
* Don't return B_BUFFER_OVERFLOW if the provided buffer is not large
enough to hold the link contents.
* Update documentation to clearly describe behavior.
src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp:
* Change bfs_read_link() to always return the link length. This is
called by common_read_link in the VFS, which is called by
_kern_read_link().
src/add-ons/kernel/file_systems/btrfs/kernel_interface.cpp:
src/add-ons/kernel/file_systems/exfat/kernel_interface.cpp:
src/add-ons/kernel/file_systems/ext2/kernel_interface.cpp:
src/add-ons/kernel/file_systems/iso9660/kernel_interface.cpp:
src/add-ons/kernel/file_systems/netfs/client/netfs.cpp:
src/add-ons/kernel/file_systems/nfs/nfs_add_on.c:
src/add-ons/kernel/file_systems/ramfs/kernel_interface.cpp:
src/add-ons/kernel/file_systems/reiserfs/Iterators.cpp:
src/add-ons/kernel/file_systems/reiserfs/Iterators.h:
src/add-ons/kernel/file_systems/reiserfs/Volume.cpp:
src/add-ons/kernel/file_systems/reiserfs/Volume.h:
* Update the implementation of read_link for these filesystems. Some
of them were incorrect, and some had just copied the posix behavior of
bfs from before this patch.
* Use user_memcpy in ext2_read_link()
* Use user_memcpy in nfs fs_read_link()
* Use user_memcpy in reiserfs StreamReader::_ReadIndirectItem and
StreamReader::_ReadDirectItem
* Remove unused method Volume::ReadObject in reiserfs.
src/add-ons/kernel/file_systems/packagefs/nodes/UnpackingLeafNode.cpp:
src/add-ons/kernel/file_systems/packagefs/package_links/PackageLinkSymlink.cpp:
* Update UnpackingLeafNode::ReadSymlink and
PackageSymLink::ReadSymLink() to set the bufferSize out parameter to
the symlink length. Both of these are called by
packagefs_read_symlink.
* Use user_memcpy
src/add-ons/kernel/file_systems/netfs/client/netfs.cpp:
* netfs seems mostly unimplemented. Added a FIXME note for future
implementers so that they know to implement the correct behavior.
src/system/libroot/posix/unistd/link.c:
* readlinkat() was just wrapping _kern_read_link() because before this
patch it had expected posix behavior. But now it does not, so we
need to return the number of bytes written to the output
buffer.
src/build/libroot/fs.cpp:
* Update _kern_read_link() in the compatibility code to emulate the
Haiku behavior on the host system. This is done by using an
intermediate buffer that is guaranteed to fit the link contents and
returning its length. The intermediate buffer is copied into the
output buffer until there is no more room.
src/tests/kits/storage/SymLinkTest.cpp:
* This patch also resolves some test failures similar to those
resolved in ee8cf35f0 which fixed tests for BNode. The tests were
failing because Haiku's error checking is just better.
BeOS allowed constructing a BSymLink with BSymLink(BDirectory*,
const char*) with the entry name of "". The same is true of the
equivilant SetTo() method. The BSymLink object will appear valid
until you attempt to use it by, for example, calling the ReadLink
method, which will return B_BAD_VALUE.
Haiku does a more appropriate thing and returns B_ENTRY_NOT_FOUND,
for this constructor and the equivilant SetTo(BDirectory*, const
char*) method. This patch fixes these test assertions to match Haiku
behavior.
docs/develop/file_systems/overview.txt:
* Add notes for future filesystem driver implementers to call this
mistake when implementing fs_vnode_ops::read_symlink.
docs/user/drivers/fs_interface.dox:
* Fix documentation for fs_vnode_ops::read_symlink
Change-Id: I8bcb8b2a0c9333059c84ace15844c32d4efeed9d
Reviewed-on: https://review.haiku-os.org/c/haiku/+/2502
Reviewed-by: waddlesplash <waddlesplash@xxxxxxxxx>
Reviewed-by: Axel Dörfler <axeld@xxxxxxxxxxxxxxxx>
[ Kyle Ambroff-Kao <kyle@xxxxxxxxxxxxxx> ]
----------------------------------------------------------------------------
Revision: hrev54107
Commit: e1b7c1c7ac1188a3b3e1694a958042468b95e781
URL: https://git.haiku-os.org/haiku/commit/?id=e1b7c1c7ac11
Author: Kyle Ambroff-Kao <kyle@xxxxxxxxxxxxxx>
Date: Mon Apr 20 05:02:41 2020 UTC
Committer: Axel Dörfler <axeld@xxxxxxxxxxxxxxxx>
Commit-Date: Thu Apr 30 10:13:41 2020 UTC
----------------------------------------------------------------------------
21 files changed, 189 insertions(+), 99 deletions(-)
docs/develop/file_systems/overview.txt | 32 ++++++++++++
docs/user/drivers/fs_interface.dox | 7 ++-
docs/user/storage/SymLink.dox | 8 ++-
.../kernel/file_systems/bfs/kernel_interface.cpp | 14 ++---
.../file_systems/btrfs/kernel_interface.cpp | 12 ++++-
.../file_systems/exfat/kernel_interface.cpp | 13 ++++-
.../file_systems/ext2/kernel_interface.cpp | 20 ++++++--
.../file_systems/iso9660/kernel_interface.cpp | 13 +++--
.../kernel/file_systems/netfs/client/netfs.cpp | 4 ++
src/add-ons/kernel/file_systems/nfs/nfs_add_on.c | 5 +-
.../packagefs/nodes/UnpackingLeafNode.cpp | 10 ++--
.../package_links/PackageLinkSymlink.cpp | 10 ++--
.../file_systems/ramfs/kernel_interface.cpp | 7 ++-
.../kernel/file_systems/reiserfs/Iterators.cpp | 11 ++--
.../kernel/file_systems/reiserfs/Iterators.h | 2 +
.../kernel/file_systems/reiserfs/Volume.cpp | 54 +++++++-------------
.../kernel/file_systems/reiserfs/Volume.h | 5 +-
src/build/libroot/fs.cpp | 23 ++++++---
src/kits/storage/SymLink.cpp | 9 ++--
src/system/libroot/posix/unistd/link.c | 7 ++-
src/tests/kits/storage/SymLinkTest.cpp | 22 +++++---
----------------------------------------------------------------------------
diff --git a/docs/develop/file_systems/overview.txt
b/docs/develop/file_systems/overview.txt
index 919a899751..fff1190e02 100644
--- a/docs/develop/file_systems/overview.txt
+++ b/docs/develop/file_systems/overview.txt
@@ -6,6 +6,38 @@ Filesystem drivers are in src/add-ons/kernel/file_system
A filesystem usually relies on an underlying block device, but that's not
required. For example, NFS is a network filesystem, so it doesn't need one.
+Implementation notes
+--------------------
+Each filesystem driver must define a few structures which act as the
+interface between the VFS and the filesystem implementation. These
+structures contain function pointers, some of which are optional,
+which should point to functions defined by the implementer that
+perform the appropriate filesystem peration as defined by the
+documentation.
+
+See docs/user/drivers/fs_interface.dox for more detailed documentation
+of this interface.
+
+It's important to note that while there may be some similarities in
+the interface with that of other operations systems, one should not
+make any assumptions about the desired behavior based soley on the
+function prototypes defined in fs_interface.h.
+
+The following is a list of notes calling out some potential mistakes.
+
+# fs_vnode_ops.read_symlink
+
+Defining this function means that the filesystem driver supports
+symbolic links, and the function that f_vnode_ops.read_symlink points
+to should read the contents of a symlink from the specified node.
+
+This may seem similar to the posix function readlink(), but it is
+slightly different. Unlike readlink(), which returns the number of
+bytes copied into the output buffer, fs_vnode_ops.read_symlink is
+expected to always return the length of the symlink contents, even if
+the provided buffer is not large enough to contain the entire symlink
+contents.
+
Development tools
-----------------
diff --git a/docs/user/drivers/fs_interface.dox
b/docs/user/drivers/fs_interface.dox
index 6ed3ed74f7..cf27d1fa8e 100644
--- a/docs/user/drivers/fs_interface.dox
+++ b/docs/user/drivers/fs_interface.dox
@@ -961,8 +961,8 @@
to hold the complete string, only the first \c *_bufferSize bytes of the
string shall be written to the buffer; the buffer shall not be
null-terminated in this case. Furthermore the variable \a _bufferSize
- points to shall be set to the length of the string written to the
buffer,
- not including any terminating null character (if written).
+ shall be set to the length of the symlink contents, even if the entire
+ contents did not fit in the provided \a buffer.
\param volume The volume object.
\param link The node object.
@@ -970,8 +970,7 @@
written to.
\param _bufferSize Pointer to a pre-allocated variable containing the
size
of the buffer supplied to the function. Upon successful
completion the
- hook shall store the number of bytes actually written into the
buffer
- in the variable.
+ hook shall store the length of the symlink contents.
\retval B_OK Everything went fine.
\retval B_BAD_VALUE \a link does not identify a symbolic link.
*/
diff --git a/docs/user/storage/SymLink.dox b/docs/user/storage/SymLink.dox
index 936bbabefc..a89eb1409f 100644
--- a/docs/user/storage/SymLink.dox
+++ b/docs/user/storage/SymLink.dox
@@ -113,10 +113,16 @@
The string written to the buffer is guaranteed to be \c NULL terminated.
+ This function does not return the number of bytes written into
+ the provided buffer. It returns the length of the symlink's
+ contents, even if that contents does not fit within the
+ provided buffer. If the buffer cannot contain the entire
+ contents then it will be truncated to \a size.
+
\param buffer The buffer to read the symlink's contents into.
\param size The size of \a buffer.
- \return The number of bytes written into the buffer or an error code.
+ \return The length of the symlink's contents or an error code.
\retval B_BAD_VALUE \a buf was \c NULL or the object didn't refer to a
symbolic link.
\retval B_FILE_ERROR The object was not initialized.
diff --git a/src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp
b/src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp
index 7fee9bb606..6b9faf2375 100644
--- a/src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp
+++ b/src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp
@@ -1592,21 +1592,21 @@ bfs_read_link(fs_volume* _volume, fs_vnode* _node,
char* buffer,
RETURN_ERROR(B_BAD_VALUE);
if ((inode->Flags() & INODE_LONG_SYMLINK) != 0) {
- if ((uint64)inode->Size() < (uint64)*_bufferSize)
- *_bufferSize = inode->Size();
-
status_t status = inode->ReadAt(0, (uint8*)buffer, _bufferSize);
if (status < B_OK)
RETURN_ERROR(status);
+ *_bufferSize = inode->Size();
return B_OK;
}
- size_t linkLen = strlen(inode->Node().short_symlink);
- if (linkLen < *_bufferSize)
- *_bufferSize = linkLen;
+ size_t linkLength = strlen(inode->Node().short_symlink);
+
+ size_t bytesToCopy = min_c(linkLength, *_bufferSize);
+
+ *_bufferSize = linkLength;
- return user_memcpy(buffer, inode->Node().short_symlink, *_bufferSize);
+ return user_memcpy(buffer, inode->Node().short_symlink, bytesToCopy);
}
diff --git a/src/add-ons/kernel/file_systems/btrfs/kernel_interface.cpp
b/src/add-ons/kernel/file_systems/btrfs/kernel_interface.cpp
index bb2575e531..bbe0c32b6d 100644
--- a/src/add-ons/kernel/file_systems/btrfs/kernel_interface.cpp
+++ b/src/add-ons/kernel/file_systems/btrfs/kernel_interface.cpp
@@ -489,7 +489,17 @@ btrfs_read_link(fs_volume* _volume, fs_vnode* _node, char*
buffer,
size_t* _bufferSize)
{
Inode* inode = (Inode*)_node->private_node;
- return inode->ReadAt(0, (uint8*)buffer, _bufferSize);
+
+ if (!inode->IsSymLink())
+ return B_BAD_VALUE;
+
+ status_t result = inode->ReadAt(0, reinterpret_cast<uint8*>(buffer),
+ _bufferSize);
+ if (result != B_OK)
+ return result;
+
+ *_bufferSize = inode->Size();
+ return B_OK;
}
diff --git a/src/add-ons/kernel/file_systems/exfat/kernel_interface.cpp
b/src/add-ons/kernel/file_systems/exfat/kernel_interface.cpp
index 45a5f1e36d..e9c88caa2a 100644
--- a/src/add-ons/kernel/file_systems/exfat/kernel_interface.cpp
+++ b/src/add-ons/kernel/file_systems/exfat/kernel_interface.cpp
@@ -534,7 +534,18 @@ exfat_read_link(fs_volume *_volume, fs_vnode *_node, char
*buffer,
size_t *_bufferSize)
{
Inode* inode = (Inode*)_node->private_node;
- return inode->ReadAt(0, (uint8*)buffer, _bufferSize);
+
+ if (!inode->IsSymLink())
+ return B_BAD_VALUE;
+
+ status_t result = inode->ReadAt(0, reinterpret_cast<uint8*>(buffer),
+ _bufferSize);
+ if (result != B_OK)
+ return result;
+
+ *_bufferSize = inode->Size();
+
+ return B_OK;
}
diff --git a/src/add-ons/kernel/file_systems/ext2/kernel_interface.cpp
b/src/add-ons/kernel/file_systems/ext2/kernel_interface.cpp
index 6eecf9bcce..1bc83cd64e 100644
--- a/src/add-ons/kernel/file_systems/ext2/kernel_interface.cpp
+++ b/src/add-ons/kernel/file_systems/ext2/kernel_interface.cpp
@@ -5,6 +5,7 @@
*/
+#include <algorithm>
#include <dirent.h>
#include <util/kernel_cpp.h>
#include <string.h>
@@ -1298,13 +1299,22 @@ ext2_read_link(fs_volume *_volume, fs_vnode *_node,
char *buffer,
if (!inode->IsSymLink())
return B_BAD_VALUE;
- if (inode->Size() < (off_t)*_bufferSize)
- *_bufferSize = inode->Size();
+ if (inode->Size() > EXT2_SHORT_SYMLINK_LENGTH) {
+ status_t result = inode->ReadAt(0,
reinterpret_cast<uint8*>(buffer),
+ _bufferSize);
+ if (result != B_OK)
+ return result;
+ } else {
+ size_t bytesToCopy =
std::min(static_cast<size_t>(inode->Size()),
+ *_bufferSize);
- if (inode->Size() > EXT2_SHORT_SYMLINK_LENGTH)
- return inode->ReadAt(0, (uint8 *)buffer, _bufferSize);
+ status_t result = user_memcpy(buffer, inode->Node().symlink,
+ bytesToCopy);
+ if (result != B_OK)
+ return result;
+ }
- memcpy(buffer, inode->Node().symlink, *_bufferSize);
+ *_bufferSize = inode->Size();
return B_OK;
}
diff --git a/src/add-ons/kernel/file_systems/iso9660/kernel_interface.cpp
b/src/add-ons/kernel/file_systems/iso9660/kernel_interface.cpp
index 0a9bd987b4..0e963506bd 100644
--- a/src/add-ons/kernel/file_systems/iso9660/kernel_interface.cpp
+++ b/src/add-ons/kernel/file_systems/iso9660/kernel_interface.cpp
@@ -8,6 +8,7 @@
*/
+#include <algorithm>
#include <ctype.h>
#ifndef FS_SHELL
@@ -563,14 +564,12 @@ fs_read_link(fs_volume* _volume, fs_vnode* _node, char*
buffer,
return B_BAD_VALUE;
size_t length = strlen(node->attr.slName);
- if (length > *_bufferSize)
- memcpy(buffer, node->attr.slName, *_bufferSize);
- else {
- memcpy(buffer, node->attr.slName, length);
- *_bufferSize = length;
- }
- return B_OK;
+ size_t bytesToCopy = std::min(length, *_bufferSize);
+
+ *_bufferSize = length;
+
+ return user_memcpy(buffer, node->attr.slName, bytesToCopy);
}
diff --git a/src/add-ons/kernel/file_systems/netfs/client/netfs.cpp
b/src/add-ons/kernel/file_systems/netfs/client/netfs.cpp
index 5a0dca1fa4..2f7669e668 100644
--- a/src/add-ons/kernel/file_systems/netfs/client/netfs.cpp
+++ b/src/add-ons/kernel/file_systems/netfs/client/netfs.cpp
@@ -576,6 +576,10 @@ netfs_read_link(void *ns, void *_node, char *buffer,
size_t *bufferSize)
Node* node = (Node*)_node;
PRINT("netfs_read_link(%p, %p, %p, %lu)\n", ns, node, buffer,
*bufferSize);
+
+ // TODO: If this were to be implemented (which it isn't, it currently
just
+ // returns B_BAD_VALUE) then this will need to be changed to return the
+ // length of the node and not the number of bytes read into buffer.
status_t error = node->GetVolume()->ReadLink(node, buffer, *bufferSize,
bufferSize);
PRINT("netfs_read_link() done: (%" B_PRIx32 ", %lu)\n", error,
diff --git a/src/add-ons/kernel/file_systems/nfs/nfs_add_on.c
b/src/add-ons/kernel/file_systems/nfs/nfs_add_on.c
index 9b32a3cd28..d7a97677a1 100644
--- a/src/add-ons/kernel/file_systems/nfs/nfs_add_on.c
+++ b/src/add-ons/kernel/file_systems/nfs/nfs_add_on.c
@@ -2290,13 +2290,12 @@ fs_readlink(fs_volume *_volume, fs_vnode *_node, char
*buf, size_t *bufsize)
length = XDRInPacketGetDynamic(&reply, data);
- length = min_c(length, *bufsize);
- memcpy(buf, data, length);
+ status_t result = user_memcpy(buf, data, min_c(length, *bufsize));
*bufsize = length;
XDRInPacketDestroy(&reply);
XDROutPacketDestroy(&call);
- return B_OK;
+ return result;
}
static status_t
diff --git
a/src/add-ons/kernel/file_systems/packagefs/nodes/UnpackingLeafNode.cpp
b/src/add-ons/kernel/file_systems/packagefs/nodes/UnpackingLeafNode.cpp
index eb2ad11cd0..cd8f76b6c3 100644
--- a/src/add-ons/kernel/file_systems/packagefs/nodes/UnpackingLeafNode.cpp
+++ b/src/add-ons/kernel/file_systems/packagefs/nodes/UnpackingLeafNode.cpp
@@ -272,11 +272,13 @@ UnpackingLeafNode::ReadSymlink(void* buffer, size_t*
bufferSize)
return B_OK;
}
- size_t toCopy = std::min(strlen(linkPath), *bufferSize);
- memcpy(buffer, linkPath, toCopy);
- *bufferSize = toCopy;
+ size_t linkLength = strnlen(linkPath, B_PATH_NAME_LENGTH);
- return B_OK;
+ size_t bytesToCopy = std::min(linkLength, *bufferSize);
+
+ *bufferSize = linkLength;
+
+ return user_memcpy(buffer, linkPath, bytesToCopy);
}
diff --git
a/src/add-ons/kernel/file_systems/packagefs/package_links/PackageLinkSymlink.cpp
b/src/add-ons/kernel/file_systems/packagefs/package_links/PackageLinkSymlink.cpp
index 1480a779dc..06045a7653 100644
---
a/src/add-ons/kernel/file_systems/packagefs/package_links/PackageLinkSymlink.cpp
+++
b/src/add-ons/kernel/file_systems/packagefs/package_links/PackageLinkSymlink.cpp
@@ -159,11 +159,13 @@ PackageLinkSymlink::Read(io_request* request)
status_t
PackageLinkSymlink::ReadSymlink(void* buffer, size_t* bufferSize)
{
- size_t toCopy = std::min(strlen(fLinkPath), *bufferSize);
- memcpy(buffer, fLinkPath, toCopy);
- *bufferSize = toCopy;
+ size_t linkLength = strnlen(fLinkPath, B_PATH_NAME_LENGTH);
- return B_OK;
+ size_t bytesToCopy = std::min(linkLength, *bufferSize);
+
+ *bufferSize = linkLength;
+
+ return user_memcpy(buffer, fLinkPath, bytesToCopy);
}
diff --git a/src/add-ons/kernel/file_systems/ramfs/kernel_interface.cpp
b/src/add-ons/kernel/file_systems/ramfs/kernel_interface.cpp
index f4a32711e0..287ef24dd3 100644
--- a/src/add-ons/kernel/file_systems/ramfs/kernel_interface.cpp
+++ b/src/add-ons/kernel/file_systems/ramfs/kernel_interface.cpp
@@ -375,8 +375,11 @@ ramfs_read_symlink(fs_volume* _volume, fs_vnode* _node,
char *buffer,
size_t toRead = min(*bufferSize,
symLink->GetLinkedPathLength());
if (toRead > 0)
- memcpy(buffer,
symLink->GetLinkedPath(), toRead);
- *bufferSize = toRead;
+ error = user_memcpy(buffer,
symLink->GetLinkedPath(),
+ toRead);
+
+ if (error == B_OK)
+ *bufferSize =
symLink->GetLinkedPathLength();
} else {
FATAL("Node %" B_PRIdINO " pretends to be a
SymLink, but isn't!\n",
node->GetID());
diff --git a/src/add-ons/kernel/file_systems/reiserfs/Iterators.cpp
b/src/add-ons/kernel/file_systems/reiserfs/Iterators.cpp
index a4a34dc2e8..e28b7ca503 100644
--- a/src/add-ons/kernel/file_systems/reiserfs/Iterators.cpp
+++ b/src/add-ons/kernel/file_systems/reiserfs/Iterators.cpp
@@ -1522,7 +1522,12 @@ StreamReader::_ReadIndirectItem(off_t offset, void
*buffer, size_t bufferSize)
off_t blockOffset = i * (off_t)fBlockSize;
uint32 localOffset = max_c(0LL, offset - blockOffset);
uint32 toRead = min_c(fBlockSize - localOffset,
bufferSize);
- memcpy(buffer, (uint8*)block->GetData() + localOffset,
toRead);
+ status_t copyResult = user_memcpy(buffer,
+ reinterpret_cast<uint8*>(block->GetData()) +
localOffset,
+ toRead);
+ if (copyResult != B_OK)
+ return copyResult;
+
block->Put();
bufferSize -= toRead;
buffer = (uint8*)buffer + toRead;
@@ -1542,7 +1547,7 @@ StreamReader::_ReadDirectItem(off_t offset, void *buffer,
size_t bufferSize)
{
//PRINT(("StreamReader::_ReadDirectItem(%Ld, %p, %lu)\n", offset, buffer,
bufferSize));
// copy the data into the buffer
- memcpy(buffer, (uint8*)fItem.GetData() + offset, bufferSize);
- return B_OK;
+ return user_memcpy(buffer,
+ reinterpret_cast<uint8*>(fItem.GetData()) + offset, bufferSize);
}
diff --git a/src/add-ons/kernel/file_systems/reiserfs/Iterators.h
b/src/add-ons/kernel/file_systems/reiserfs/Iterators.h
index eb69dc37df..5952b4d720 100644
--- a/src/add-ons/kernel/file_systems/reiserfs/Iterators.h
+++ b/src/add-ons/kernel/file_systems/reiserfs/Iterators.h
@@ -226,6 +226,8 @@ public:
uint32 GetObjectID() const { return fItemIterator.GetObjectID(); }
uint64 GetOffset() const { return fItemIterator.GetOffset(); }
+ off_t StreamSize() const { return fStreamSize; }
+
status_t ReadAt(off_t position, void *buffer, size_t bufferSize,
size_t *bytesRead);
diff --git a/src/add-ons/kernel/file_systems/reiserfs/Volume.cpp
b/src/add-ons/kernel/file_systems/reiserfs/Volume.cpp
index 84fbb349ad..2122a7ee6b 100644
--- a/src/add-ons/kernel/file_systems/reiserfs/Volume.cpp
+++ b/src/add-ons/kernel/file_systems/reiserfs/Volume.cpp
@@ -406,37 +406,6 @@ Volume::FindDirEntry(VNode *dir, const char *entryName,
VNode *foundNode,
return error;
}
-// ReadObject
-/*! \brief Reads data from an object.
-
- For subsequent reads better use a StreamReader.
-
- \param node The object to be read from.
- \param offset The file offset at which to start with reading.
- \param buffer The buffer into which the data shall be written.
- \param bufferSize The number of bytes to be read.
- \param bytesRead Pointer to a size_t to be set to the number of bytes
- actually read.
- \return \c B_OK, if everything went fine.
-*/
-status_t
-Volume::ReadObject(VNode *node, off_t offset, void *buffer, size_t bufferSize,
- size_t *bytesRead)
-{
- status_t error = (node && buffer && bytesRead && offset >= 0
- ? B_OK : B_BAD_VALUE);
- // read files or symlinks only
- if (error == B_OK && !(node->IsFile() || node->IsSymlink()))
- error = B_BAD_VALUE;
- // let a StreamReader do the actual work
- if (error == B_OK) {
- StreamReader reader(fTree, node->GetDirID(),
node->GetObjectID());
- error = reader.InitCheck();
- if (error == B_OK)
- error = reader.ReadAt(offset, buffer, bufferSize,
bytesRead);
- }
- return error;
-}
// ReadLink
/*! \brief Reads a symlink.
@@ -447,15 +416,30 @@ Volume::ReadObject(VNode *node, off_t offset, void
*buffer, size_t bufferSize,
\param node The symlink to be read from.
\param buffer The buffer into which the data shall be written.
\param bufferSize The number of bytes to be read.
- \param bytesRead Pointer to a size_t to be set to the number of bytes
- actually read.
+ \param linkLength Pointer to a size_t to be set to the length of the
link.
\return \c B_OK, if everything went fine.
*/
status_t
Volume::ReadLink(VNode *node, char *buffer, size_t bufferSize,
- size_t *bytesRead)
+ size_t *linkLength)
{
- return ReadObject(node, 0, buffer, bufferSize, bytesRead);
+ if (node == NULL || linkLength == NULL)
+ return B_BAD_VALUE;
+
+ if (!node->IsSymlink())
+ return B_BAD_VALUE;
+
+ StreamReader reader(fTree, node->GetDirID(), node->GetObjectID());
+ status_t result = reader.InitCheck();
+ if (result != B_OK)
+ return result;
+
+ size_t bytesCopied = bufferSize;
+ result = reader.ReadAt(0, buffer, bufferSize, &bytesCopied);
+
+ *linkLength = reader.StreamSize();
+
+ return result;
}
// FindEntry
diff --git a/src/add-ons/kernel/file_systems/reiserfs/Volume.h
b/src/add-ons/kernel/file_systems/reiserfs/Volume.h
index 230d6b8108..793cbd5d5c 100644
--- a/src/add-ons/kernel/file_systems/reiserfs/Volume.h
+++ b/src/add-ons/kernel/file_systems/reiserfs/Volume.h
@@ -72,10 +72,9 @@ public:
status_t FindDirEntry(VNode *dir, const char *entryName,
VNode *foundNode, bool
failIfHidden = false);
- status_t ReadObject(VNode *node, off_t offset, void *buffer,
- size_t bufferSize, size_t
*bytesRead);
+
status_t ReadLink(VNode *node, char *buffer, size_t bufferSize,
- size_t *bytesRead);
+ size_t *linkLength);
status_t FindEntry(const VNode *rootDir, const char *path,
VNode *foundNode);
diff --git a/src/build/libroot/fs.cpp b/src/build/libroot/fs.cpp
index c5f59a566e..517668a90c 100644
--- a/src/build/libroot/fs.cpp
+++ b/src/build/libroot/fs.cpp
@@ -1070,19 +1070,28 @@ _kern_read_link(int fd, const char *path, char *buffer,
size_t *_bufferSize)
if (error != B_OK)
return error;
- // readlink
ssize_t bytesRead = readlink(realPath.c_str(), buffer, *_bufferSize);
if (bytesRead < 0)
return errno;
- if (*_bufferSize > 0) {
- if ((size_t)bytesRead == *_bufferSize)
- bytesRead--;
-
+ // On Haiku _kern_read_link will return the length of the link, *not*
+ // the number of bytes written to the buffer parameter. To emulate that
+ // here, we use readlink() to read the links contents, and then if it is
+ // possible that the link contents didn't fit in buffer, we'll fall back
+ // to lstat() to get the full length of the link.
+ if (static_cast<size_t>(bytesRead) < *_bufferSize) {
buffer[bytesRead] = '\0';
- }
+ *_bufferSize = bytesRead;
+ } else {
+ // The number of bytes copied by readlink() tells us that the
entire
+ // link might not have fit into buffer. Fall back to getting
the full
+ // length of the link using lstat.
+ struct stat linkStat;
+ if (lstat(realPath.c_str(), &linkStat) != 0)
+ return errno;
- *_bufferSize = bytesRead;
+ *_bufferSize = linkStat.st_size;
+ }
return B_OK;
}
diff --git a/src/kits/storage/SymLink.cpp b/src/kits/storage/SymLink.cpp
index 2b6b490ced..991826d8fb 100644
--- a/src/kits/storage/SymLink.cpp
+++ b/src/kits/storage/SymLink.cpp
@@ -94,11 +94,10 @@ BSymLink::ReadLink(char* buffer, size_t size)
if (result < B_OK)
return result;
- // null-terminate
- if (linkLen >= size)
- return B_BUFFER_OVERFLOW;
-
- buffer[linkLen] = '\0';
+ if (linkLen < size)
+ buffer[linkLen] = '\0';
+ else if (size > 0)
+ buffer[size - 1] = '\0';
return linkLen;
}
diff --git a/src/system/libroot/posix/unistd/link.c
b/src/system/libroot/posix/unistd/link.c
index 74ab991040..78919280f7 100644
--- a/src/system/libroot/posix/unistd/link.c
+++ b/src/system/libroot/posix/unistd/link.c
@@ -34,7 +34,12 @@ readlinkat(int fd, const char *path, char *buffer, size_t
bufferSize)
if (linkLen < bufferSize)
buffer[linkLen] = '\0';
- return linkLen;
+ // _kern_read_link() returns the length of the link, but readlinkat() is
+ // supposed to return the number of bytes placed into buffer. If the
+ // buffer is larger than the link contents, then linkLen is the number
+ // of bytes written to the buffer. Otherwise, bufferSize bytes will have
+ // been written.
+ return min_c(linkLen, bufferSize);
}
diff --git a/src/tests/kits/storage/SymLinkTest.cpp
b/src/tests/kits/storage/SymLinkTest.cpp
index 782944c70a..3014d9299d 100644
--- a/src/tests/kits/storage/SymLinkTest.cpp
+++ b/src/tests/kits/storage/SymLinkTest.cpp
@@ -285,7 +285,7 @@ SymLinkTest::InitTest1()
BDirectory pathDir(dirSuperLink);
CPPUNIT_ASSERT( pathDir.InitCheck() == B_OK );
BSymLink link(&pathDir, "");
- CPPUNIT_ASSERT( link.InitCheck() == B_OK );
+ CPPUNIT_ASSERT(link.InitCheck() == B_ENTRY_NOT_FOUND);
}
NextSubTest();
{
@@ -467,8 +467,8 @@ SymLinkTest::InitTest2()
//
NextSubTest();
CPPUNIT_ASSERT( pathDir.SetTo(dirSuperLink) == B_OK );
- CPPUNIT_ASSERT( link.SetTo(&pathDir, "") == B_OK );
- CPPUNIT_ASSERT( link.InitCheck() == B_OK );
+ CPPUNIT_ASSERT(link.SetTo(&pathDir, "") == B_ENTRY_NOT_FOUND);
+ CPPUNIT_ASSERT(link.InitCheck() == B_ENTRY_NOT_FOUND);
//
NextSubTest();
CPPUNIT_ASSERT( pathDir.SetTo(existingSuperFile) == B_OK );
@@ -556,9 +556,19 @@ SymLinkTest::ReadLinkTest()
NextSubTest();
char smallBuffer[2];
CPPUNIT_ASSERT( link.SetTo(dirLink) == B_OK );
- CPPUNIT_ASSERT( link.ReadLink(smallBuffer, sizeof(smallBuffer))
- == (ssize_t)strlen(dirLink) );
- CPPUNIT_ASSERT( strncmp(smallBuffer, existingDir, sizeof(smallBuffer))
== 0 );
+ ssize_t linkLength = link.ReadLink(smallBuffer, sizeof(smallBuffer));
+ CPPUNIT_ASSERT(linkLength == static_cast<ssize_t>(strlen(existingDir)));
+ CPPUNIT_ASSERT_EQUAL('/', smallBuffer[0]);
+ CPPUNIT_ASSERT_EQUAL('\0', smallBuffer[1]);
+
+ // Invoke with one extra byte of length to ensure that the result is
NULL
+ // terminated.
+ NextSubTest();
+ buffer[17] = 'x';
+ CPPUNIT_ASSERT(link.ReadLink(buffer, 18));
+ CPPUNIT_ASSERT_EQUAL('\0', buffer[17]);
+ CPPUNIT_ASSERT_EQUAL(strcmp(buffer, "/tmp/existing-dir"), 0);
+
// bad args
NextSubTest();
CPPUNIT_ASSERT( link.SetTo(fileLink) == B_OK );
