* Aligned the semantics of the read_symlink() FS module hook with the

readlink() function. It is no longer required to null-terminate the
  string, shall not fail, if the buffer is too small, and shall return
  the length of the string actually written into the buffer.
* Adjusted rootfs, devfs, and bfs accordingly. Also adjusted their
  read_stat() hooks to return the correct symlink length in st_size.
* Our readlink() does now comply to the standard (and BeOS).
  Additionally if the buffer is big enough it is nice to non-conforming
  apps and null-terminates it.
* BSymLink::ReadLink() explicitly null-terminates the string now.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@24425 a95241bf-73f2-0310-859d-f6bbb57e9c96

docs/user/drivers/fs_interface.dox

@@ -1,9 +1,9 @@
 /*
- * Copyright 2007 Haiku Inc. All rights reserved.
+ * Copyright 2007-2008 Haiku Inc. All rights reserved.
  * Distributed under the terms of the MIT License.
  *
  * Authors:
- *		Ingo Weinhold
+ *		Ingo Weinhold <[email protected]>
  *		Niels Sascha Reedijk <[email protected]>
  *		Axel Dörfler <[email protected]>
  *
@@ -536,21 +536,24 @@
 			fs_vnode link, char *buffer, size_t *_bufferSize)
 	\brief Read the value of a symbolic link.
 
-	If the function is successful, the string written to the buffer shall be
-	null-terminated and the variable \a _bufferSize points to shall be set to
-	the length of that string, including the terminating null character.
+	If the function is successful, the symlink string shall be written to the
+	buffer. It does not need to be null-terminated. If the buffer is too small
+	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).
 
 	\param fs The volume handle.
 	\param link The node handle.
 	\param buffer Pointer to a pre-allocated buffer the link value shall be
 		written to.
-	\param buffer 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.
+	\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.
 	\retval B_OK Everything went fine.
 	\retval B_BAD_VALUE \a link does not identify a symbolic link.
-	\retval B_BUFFER_OVERFLOW The supplied buffer is not big enough to contain
-		the complete link value.
 */
 
 /*!

src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp

@@ -49,7 +49,7 @@ fill_stat_buffer(Inode *inode, struct stat &stat)
 
 	if (inode->IsSymLink() && (node.Flags() & INODE_LONG_SYMLINK) == 0) {
 		// symlinks report the size of the link here
-		stat.st_size = strlen(node.short_symlink) + 1;
+		stat.st_size = strlen(node.short_symlink);
 	} else
 		stat.st_size = inode->Size();
 }
@@ -1366,29 +1366,26 @@ bfs_read_link(void *_ns, void *_node, char *buffer, size_t *_bufferSize)
 	FUNCTION();
 
 	Inode *inode = (Inode *)_node;
-	size_t bufferSize = *_bufferSize;
 
 	if (!inode->IsSymLink())
 		RETURN_ERROR(B_BAD_VALUE);
 
 	if (inode->Flags() & INODE_LONG_SYMLINK) {
-		// we also need space for the terminating null byte
-		if (inode->Size() >= bufferSize) {
-			*_bufferSize = inode->Size() + 1;
-			return B_BUFFER_OVERFLOW;
-		}
+		if (inode->Size() < *_bufferSize)
+			*_bufferSize = inode->Size();
 
 		status_t status = inode->ReadAt(0, (uint8 *)buffer, _bufferSize);
 		if (status < B_OK)
 			RETURN_ERROR(status);
 
-		buffer[++*_bufferSize] = '\0';
 		return B_OK;
 	}
 
-	*_bufferSize = strlcpy(buffer, inode->Node().short_symlink, bufferSize) + 1;
-	if (*_bufferSize > bufferSize)
-		return B_BUFFER_OVERFLOW;
+	size_t linkLen = strlen(inode->Node().short_symlink);
+	if (linkLen < *_bufferSize)
+		*_bufferSize = linkLen;
+
+	memcpy(buffer, inode->Node().short_symlink, *_bufferSize);
 
 	return B_OK;
 }

src/kits/storage/Entry.cpp

@@ -928,10 +928,11 @@ BEntry::set(int dirFD, const char *path, bool traverse)
 			// we need to traverse the symlink
 			if (--linkLimit < 0)
 				return B_LINK_LIMIT;
-			size_t bufferSize = B_PATH_NAME_LENGTH;
+			size_t bufferSize = B_PATH_NAME_LENGTH - 1;
 			error = _kern_read_link(dirFD, leafName, tmpPath, &bufferSize);
 			if (error < 0)
 				return error;
+			tmpPath[bufferSize] = '\0';
 			path = tmpPath;
 			// next round...
 		}

src/kits/storage/LibBeAdapter.cpp

@@ -534,7 +534,6 @@ _kern_read_link(int fd, const char *path, char *buffer, size_t *_bufferSize)
 	if (result < 0)
 		return result;
 
-	buffer[result] = '\0';
 	*_bufferSize = result;
 	return B_OK;
 }

src/kits/storage/SymLink.cpp

@@ -93,7 +93,8 @@ BSymLink::~BSymLink()
 
 // ReadLink
 //! Reads the contents of the symbolic link into a buffer.
-/*!	\param buf the buffer
+/*!	The string written to the buffer will be null-terminated.
+	\param buf the buffer
 	\param size the size of the buffer
 	\return
 	- the number of bytes written into the buffer
@@ -110,11 +111,17 @@ BSymLink::ReadLink(char *buffer, size_t size)
 	if (InitCheck() != B_OK)
 		return B_FILE_ERROR;
 
-	status_t error = _kern_read_link(get_fd(), NULL, buffer, &size);
+	size_t linkLen = size;
+	status_t error = _kern_read_link(get_fd(), NULL, buffer, &linkLen);
 	if (error < B_OK)
 		return error;
 
-	return size;
+	// null-terminate
+	if (linkLen >= size)
+		return B_BUFFER_OVERFLOW;
+	buffer[linkLen] = '\0';
+
+	return linkLen;
 }
 
 // MakeLinkedPath

src/system/kernel/fs/devfs.cpp

@@ -1880,13 +1880,10 @@ devfs_read_link(fs_volume _fs, fs_vnode _link, char *buffer, size_t *_bufferSize
 	if (!S_ISLNK(link->stream.type))
 		return B_BAD_VALUE;
 
-	*_bufferSize = link->stream.u.symlink.length + 1;
-		// we always need to return the number of bytes we intend to write!
+	if (link->stream.u.symlink.length < *_bufferSize)
+		*_bufferSize = link->stream.u.symlink.length;
 
-	if (bufferSize <= link->stream.u.symlink.length)
-		return B_BUFFER_OVERFLOW;
-
-	memcpy(buffer, link->stream.u.symlink.path, link->stream.u.symlink.length + 1);
+	memcpy(buffer, link->stream.u.symlink.path, *_bufferSize);
 	return B_OK;
 }
 
@@ -2478,6 +2475,8 @@ devfs_read_stat(fs_volume _fs, fs_vnode _vnode, struct stat *stat)
 		// is this a real block device? then let's have it reported like that
 		if (stat->st_size != 0)
 			stat->st_mode = S_IFBLK | (vnode->stream.type & S_IUMSK);
+	} else if (S_ISLNK(vnode->stream.type)) {
+		stat->st_size = vnode->stream.u.symlink.length;
 	}
 
 	return B_OK;

src/system/kernel/fs/rootfs.cpp

@@ -805,18 +805,14 @@ static status_t
 rootfs_read_link(fs_volume _fs, fs_vnode _link, char *buffer, size_t *_bufferSize)
 {
 	struct rootfs_vnode *link = (rootfs_vnode*)_link;
-	size_t bufferSize = *_bufferSize;
 
 	if (!S_ISLNK(link->stream.type))
 		return B_BAD_VALUE;
 
-	*_bufferSize = link->stream.symlink.length + 1;
-		// we always need to return the number of bytes we intend to write!
+	if (link->stream.symlink.length < *_bufferSize)
+		*_bufferSize = link->stream.symlink.length;
 
-	if (bufferSize <= link->stream.symlink.length)
-		return B_BUFFER_OVERFLOW;
-
-	memcpy(buffer, link->stream.symlink.path, link->stream.symlink.length + 1);
+	memcpy(buffer, link->stream.symlink.path, *_bufferSize);
 	return B_OK;
 }
 
@@ -975,7 +971,10 @@ rootfs_read_stat(fs_volume _fs, fs_vnode _v, struct stat *stat)
 	// stream exists, but we know to return size 0, since we can only hold directories
 	stat->st_dev = fs->id;
 	stat->st_ino = vnode->id;
-	stat->st_size = 0;
+	if (S_ISLNK(vnode->stream.type))
+		stat->st_size = vnode->stream.symlink.length;
+	else
+		stat->st_size = 0;
 	stat->st_mode = vnode->stream.type;
 
 	stat->st_nlink = 1;

src/system/kernel/fs/vfs.cpp

@@ -1780,9 +1780,13 @@ vnode_path_to_vnode(struct vnode *vnode, char *path, bool traverseLeafLink,
 			}
 
 			if (FS_CALL(nextVnode, read_symlink) != NULL) {
+				bufferSize--;
 				status = FS_CALL(nextVnode, read_symlink)(
 					nextVnode->mount->cookie, nextVnode->private_node, buffer,
 					&bufferSize);
+				// null-terminate
+				if (status >= 0)
+					buffer[bufferSize] = '\0';
 			} else
 				status = B_BAD_VALUE;
 
@@ -7399,11 +7403,12 @@ _user_normalize_path(const char* userPath, bool traverseLink, char* buffer)
 		// read link
 		struct stat st;
 		if (FS_CALL(fileVnode, read_symlink) != NULL) {
-			size_t bufferSize = B_PATH_NAME_LENGTH;
+			size_t bufferSize = B_PATH_NAME_LENGTH - 1;
 			error = FS_CALL(fileVnode, read_symlink)(fileVnode->mount->cookie,
 				fileVnode->private_node, path, &bufferSize);
 			if (error != B_OK)
 				return error;
+			path[bufferSize] = '\0';
 		} else
 			return B_BAD_VALUE;
 	}
@@ -7704,7 +7709,7 @@ _user_read_link(int fd, const char *userPath, char *userBuffer, size_t *userBuff
 	if (status < B_OK)
 		return status;
 
-	if (user_strlcpy(userBuffer, buffer, bufferSize) < 0)
+	if (user_memcpy(userBuffer, buffer, bufferSize) != B_OK)
 		return B_BAD_ADDRESS;
 
 	return B_OK;

src/system/libroot/posix/unistd/link.c

@@ -20,13 +20,19 @@
 ssize_t
 readlink(const char *path, char *buffer, size_t bufferSize)
 {
-	status_t status = _kern_read_link(-1, path, buffer, &bufferSize);
-	if (status < B_OK && status != B_BUFFER_OVERFLOW) {
+	size_t linkLen = bufferSize;
+	status_t status = _kern_read_link(-1, path, buffer, &linkLen);
+	if (status < B_OK) {
 		errno = status;
 		return -1;
 	}
 
-	return bufferSize;
+	// If the buffer is big enough, null-terminate the string. That's not
+	// required by the standard, but helps non-conforming apps.
+	if (linkLen < bufferSize)
+		buffer[linkLen] = '\0';
+
+	return linkLen;
 }