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@gmail.com>
Reviewed-by: Axel Dörfler <axeld@pinc-software.de>

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
 -----------------
 

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.
 */

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.

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);
 
-	return user_memcpy(buffer, inode->Node().short_symlink, *_bufferSize);
+	size_t bytesToCopy = min_c(linkLength, *_bufferSize);
+
+	*_bufferSize = linkLength;
+
+	return user_memcpy(buffer, inode->Node().short_symlink, bytesToCopy);
 }
 
 

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;
 }
 
 

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;
 }
 
 

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;
 }
 

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);
 }
 
 

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,

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

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);
 }
 
 

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);
 }
 
 

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());

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);
 }
 

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);
 

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

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);

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;
 }

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;
 }

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);
 }
 
 

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 );