* Some index updating functions did not take into account that not all nodes

are put into all indices. For example, symlinks aren't part of the size
  index. This fixes bug #2704.
* Also, Inode::NeedsTrimming() now ignores symlinks that have no data stream,
  it's const now, too.
* Added helper methods Inode::In{Name|Size|LastModified}Index(), and use it
  where appropriate.
* Clarified Inode::Create() description with regards to name vs. parent.


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

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

@@ -321,7 +321,7 @@ Index::UpdateName(Transaction &transaction, const char* oldName,
 status_t
 Index::InsertSize(Transaction &transaction, Inode* inode)
 {
-	ASSERT(inode->IsFile());
+	ASSERT(inode->InSizeIndex());
 
 	off_t size = inode->Size();
 	return Update(transaction, "size", B_INT64_TYPE, NULL, 0, (uint8*)&size,
@@ -332,7 +332,7 @@ Index::InsertSize(Transaction &transaction, Inode* inode)
 status_t
 Index::RemoveSize(Transaction &transaction, Inode* inode)
 {
-	ASSERT(inode->IsFile());
+	ASSERT(inode->InSizeIndex());
 
 	// Inode::OldSize() is the size that's in the index
 	off_t size = inode->OldSize();
@@ -344,7 +344,7 @@ Index::RemoveSize(Transaction &transaction, Inode* inode)
 status_t
 Index::UpdateSize(Transaction &transaction, Inode* inode)
 {
-	ASSERT(inode->IsFile());
+	ASSERT(inode->InSizeIndex());
 
 	off_t oldSize = inode->OldSize();
 	off_t newSize = inode->Size();
@@ -362,7 +362,7 @@ Index::UpdateSize(Transaction &transaction, Inode* inode)
 status_t
 Index::InsertLastModified(Transaction &transaction, Inode* inode)
 {
-	ASSERT(inode->IsFile() || inode->IsSymLink());
+	ASSERT(inode->InLastModifiedIndex());
 
 	off_t modified = inode->LastModified();
 	return Update(transaction, "last_modified", B_INT64_TYPE, NULL, 0,
@@ -373,7 +373,7 @@ Index::InsertLastModified(Transaction &transaction, Inode* inode)
 status_t
 Index::RemoveLastModified(Transaction &transaction, Inode* inode)
 {
-	ASSERT(inode->IsFile() || inode->IsSymLink());
+	ASSERT(inode->InLastModifiedIndex());
 
 	// Inode::OldLastModified() is the value which is in the index
 	off_t modified = inode->OldLastModified();
@@ -386,7 +386,7 @@ status_t
 Index::UpdateLastModified(Transaction &transaction, Inode* inode,
 	off_t modified)
 {
-	ASSERT(inode->IsFile() || inode->IsSymLink());
+	ASSERT(inode->InLastModifiedIndex());
 
 	off_t oldModified = inode->OldLastModified();
 	if (modified == -1)

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

@@ -2063,18 +2063,19 @@ Inode::Append(Transaction& transaction, off_t bytes)
 }
 
 
-/*!	Checks wether or not this inode's data stream needs to be trimmed
+/*!	Checks whether or not this inode's data stream needs to be trimmed
 	because of an earlier preallocation.
 	Returns true if there are any blocks to be trimmed.
 */
 bool
-Inode::NeedsTrimming()
+Inode::NeedsTrimming() const
 {
 	// We never trim preallocated index blocks to make them grow as smooth as
 	// possible. There are only few indices anyway, so this doesn't hurt.
 	// Also, if an inode is already in deleted state, we don't bother trimming
 	// it.
-	if (IsIndex() || IsDeleted())
+	if (IsIndex() || IsDeleted()
+		|| (IsSymLink() && (Flags() & INODE_LONG_SYMLINK) == 0))
 		return false;
 
 	off_t roundedSize = round_up(Size(), fVolume->BlockSize());
@@ -2301,36 +2302,36 @@ Inode::Remove(Transaction& transaction, const char* name, ino_t* _id,
 	// are updated here (name, size, & last_modified)
 
 	Index index(fVolume);
-	if (inode->IsRegularNode()) {
+	if (inode->InNameIndex()) {
 		index.RemoveName(transaction, name, inode);
 			// If removing from the index fails, it is not regarded as a
 			// fatal error and will not be reported back!
 			// Deleted inodes won't be visible in queries anyway.
 	}
 
-	if (inode->IsFile() || inode->IsSymLink()) {
+	if (inode->InSizeIndex())
-		if (inode->IsFile())
 		index.RemoveSize(transaction, inode);
+	if (inode->InLastModifiedIndex())
 		index.RemoveLastModified(transaction, inode);
-	}
 
 	return inode->WriteBack(transaction);
 }
 
 
-/*!	Creates the inode with the specified parent directory, and automatically
+/*!	Creates the inode with the specified \a parent directory, and automatically
 	adds the created inode to that parent directory. If an attribute directory
-	is created, it will also automatically  be added to the parent inode as
+	is created, it will also automatically  be added to the \a parent inode as
 	such. However, the indices root node, and the regular root node won't be
 	added to the super block.
 	It will also create the initial B+tree for the inode if it's a directory
 	of any kind.
+	\a name may be \c NULL, but only if no \a parent is given.
 	If the "_id" or "_inode" variable is given and non-NULL to store the
 	inode's ID, the inode stays locked - you have to call put_vnode() if you
 	don't use it anymore.
-	If the node already exists, this method will fail if O_EXCL is set, or it's
+	If the node already exists, this method will fail if \c O_EXCL is set, or
-	a directory or a symlink. Otherwise, it will just be returned. If O_TRUNC
+	it's a directory or a symlink. Otherwise, it will just be returned.
-	has been specified, the file will also be truncated.
+	If \c O_TRUNC has been specified, the file will also be truncated.
 */
 status_t
 Inode::Create(Transaction& transaction, Inode* parent, const char* name,
@@ -2495,7 +2496,7 @@ Inode::Create(Transaction& transaction, Inode* parent, const char* name,
 	// (live queries might want to access us after this)
 
 	Index index(volume);
-	if (inode->IsRegularNode() && name != NULL) {
+	if (inode->InNameIndex() && name != NULL) {
 		// the name index only contains regular files
 		// (but not the root node where name == NULL)
 		status = index.InsertName(transaction, name, inode);
@@ -2514,14 +2515,13 @@ Inode::Create(Transaction& transaction, Inode* parent, const char* name,
 
 	inode->UpdateOldLastModified();
 
-	// The "size" & "last_modified" indices don't contain directories
+	// The "size" & "last_modified" indices don't contain directories.
-	if (inode->IsFile() || inode->IsSymLink()) {
+	// If adding to these indices fails, the inode creation will not be
-		// if adding to these indices fails, the inode creation will not be
+	// harmed; they are considered less important than the "name" index.
-		//  harmed; they are considered less important than the "name" index
+	if (inode->InSizeIndex())
-		if (inode->IsFile())
 		index.InsertSize(transaction, inode);
+	if (inode->InLastModifiedIndex())
 		index.InsertLastModified(transaction, inode);
-	}
 
 	if (inode->IsFile() || inode->IsAttribute()) {
 		inode->SetFileCache(file_cache_create(volume->ID(), inode->ID(),
@@ -2549,6 +2549,30 @@ Inode::Create(Transaction& transaction, Inode* parent, const char* name,
 }
 
 
+/*!	Checks whether or not this node should be part of the name index */
+bool
+Inode::InNameIndex() const
+{
+	return IsRegularNode();
+}
+
+
+/*!	Checks whether or not this node should be part of the size index */
+bool
+Inode::InSizeIndex() const
+{
+	return IsFile();
+}
+
+
+/*!	Checks whether or not this node should be part of the last modified index */
+bool
+Inode::InLastModifiedIndex() const
+{
+	return IsFile() || IsSymLink();
+}
+
+
 //	#pragma mark - AttributeIterator
 
 

src/add-ons/kernel/file_systems/bfs/Inode.h

@@ -140,12 +140,13 @@ public:
 			status_t		SetFileSize(Transaction& transaction, off_t size);
 			status_t		Append(Transaction& transaction, off_t bytes);
 			status_t		TrimPreallocation(Transaction& transaction);
-			bool			NeedsTrimming();
+			bool			NeedsTrimming() const;
 
 			status_t		Free(Transaction& transaction);
 			status_t		Sync();
 
 			bfs_inode&		Node() { return fNode; }
+			const bfs_inode& Node() const { return fNode; }
 
 			// create/remove inodes
 			status_t		Remove(Transaction& transaction, const char* name,
@@ -165,6 +166,10 @@ public:
 			off_t			OldSize() { return fOldSize; }
 			off_t			OldLastModified() { return fOldLastModified; }
 
+			bool			InNameIndex() const;
+			bool			InSizeIndex() const;
+			bool			InLastModifiedIndex() const;
+
 			// file cache
 			void*			FileCache() const { return fCache; }
 			void			SetFileCache(void* cache) { fCache = cache; }

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

@@ -723,12 +723,14 @@ bfs_write_stat(fs_volume* _volume, fs_vnode* _node, const struct stat* stat,
 
 	bfs_inode& node = inode->Node();
 
-	if (mask & B_STAT_SIZE) {
+	if ((mask & B_STAT_SIZE) != 0) {
 		// Since WSTAT_SIZE is the only thing that can fail directly, we
 		// do it first, so that the inode state will still be consistent
 		// with the on-disk version
 		if (inode->IsDirectory())
 			return B_IS_A_DIRECTORY;
+		if (!inode->IsFile())
+			return B_BAD_VALUE;
 
 		if (inode->Size() != stat->st_size) {
 			off_t oldSize = inode->Size();
@@ -751,19 +753,19 @@ bfs_write_stat(fs_volume* _volume, fs_vnode* _node, const struct stat* stat,
 		}
 	}
 
-	if (mask & B_STAT_MODE) {
+	if ((mask & B_STAT_MODE) != 0) {
 		PRINT(("original mode = %ld, stat->st_mode = %d\n", node.Mode(), stat->st_mode));
 		node.mode = HOST_ENDIAN_TO_BFS_INT32((node.Mode() & ~S_IUMSK)
 			| (stat->st_mode & S_IUMSK));
 	}
 
-	if (mask & B_STAT_UID)
+	if ((mask & B_STAT_UID) != 0)
 		node.uid = HOST_ENDIAN_TO_BFS_INT32(stat->st_uid);
-	if (mask & B_STAT_GID)
+	if ((mask & B_STAT_GID) != 0)
 		node.gid = HOST_ENDIAN_TO_BFS_INT32(stat->st_gid);
 
-	if (mask & B_STAT_MODIFICATION_TIME) {
+	if ((mask & B_STAT_MODIFICATION_TIME) != 0) {
-		if (inode->IsDirectory()) {
+		if (!inode->InLastModifiedIndex()) {
 			// directory modification times are not part of the index
 			node.last_modified_time = HOST_ENDIAN_TO_BFS_INT64(
 				(bigtime_t)stat->st_mtime << INODE_TIME_SHIFT);
@@ -774,7 +776,7 @@ bfs_write_stat(fs_volume* _volume, fs_vnode* _node, const struct stat* stat,
 				(bigtime_t)stat->st_mtime << INODE_TIME_SHIFT);
 		}
 	}
-	if (mask & B_STAT_CREATION_TIME) {
+	if ((mask & B_STAT_CREATION_TIME) != 0) {
 		node.create_time = HOST_ENDIAN_TO_BFS_INT64(
 			(bigtime_t)stat->st_crtime << INODE_TIME_SHIFT);
 	}
@@ -1296,7 +1298,10 @@ bfs_free_cookie(fs_volume* _volume, fs_vnode* _node, void* _cookie)
 			&& !inode->IsDeleted()
 			&& (needsTrimming
 				|| inode->OldLastModified() != inode->LastModified()
-				|| inode->OldSize() != inode->Size())) {
+				|| (inode->InSizeIndex()
+					// TODO: this can prevent the size update notification
+					// for nodes not in the index!
+					&& inode->OldSize() != inode->Size()))) {
 			locker.Unlock();
 			transaction.Start(volume, inode->BlockNumber());
 		}
@@ -1323,11 +1328,15 @@ bfs_free_cookie(fs_volume* _volume, fs_vnode* _node, void* _cookie)
 			}
 		}
 		if (inode->OldSize() != inode->Size()) {
+			if (inode->InSizeIndex())
 				index.UpdateSize(transaction, inode);
 			changedSize = true;
 		}
 		if (inode->OldLastModified() != inode->LastModified()) {
-			index.UpdateLastModified(transaction, inode, inode->LastModified());
+			if (inode->InLastModifiedIndex()) {
+				index.UpdateLastModified(transaction, inode,
+					inode->LastModified());
+			}
 			changedTime = true;
 
 			// updating the index doesn't write back the inode
@@ -1385,7 +1394,7 @@ bfs_read_link(fs_volume* _volume, fs_vnode* _node, char* buffer,
 	if (!inode->IsSymLink())
 		RETURN_ERROR(B_BAD_VALUE);
 
-	if (inode->Flags() & INODE_LONG_SYMLINK) {
+	if ((inode->Flags() & INODE_LONG_SYMLINK) != 0) {
 		if (inode->Size() < *_bufferSize)
 			*_bufferSize = inode->Size();