Fixed queries a bit more (maybe even completely...). In several situations,

the old attribute value wasn't evaluated at all.
Also, Index::Update() got a type code of zero, instead of the real value
in situations where this didn't make any sense.
Minor cleanup in kernel_interface.cpp.
New method Volume::CheckForLiveQuery() that checks if there is a live query
depending on the specified attribute - not yet implemented, but already used;
always returns "true" right now, which doesn't cause any harm but reducing
the speed lightly.


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

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

@@ -226,12 +226,15 @@ Index::Update(Transaction *transaction, const char *name, int32 type, const uint
 	if (type == B_MIME_STRING_TYPE)
 		type = B_STRING_TYPE;
 
-	// if the two keys are identical, don't do anything
+	// If the two keys are identical, don't do anything - only compare if the
+	// type has been set, until we have a real type code, we can't do much
+	// about the comparison here
 	if (type != 0 && !compareKeys(type, oldKey, oldLength, newKey, newLength))
 		return B_OK;
 
 	// update all live queries about the change, if they have an index or not
-	fVolume->UpdateLiveQueries(inode, name, type, oldKey, oldLength, newKey, newLength);
+	if (type != 0)
+		fVolume->UpdateLiveQueries(inode, name, type, oldKey, oldLength, newKey, newLength);
 
 	status_t status;
 	if (((name != fName || strcmp(name, fName)) && (status = SetTo(name)) < B_OK)
@@ -242,6 +245,10 @@ Index::Update(Transaction *transaction, const char *name, int32 type, const uint
 	if (type == 0 && !compareKeys(Type(), oldKey, oldLength, newKey, newLength))
 		return B_OK;
 
+	// same for the live query update
+	if (type == 0)
+		fVolume->UpdateLiveQueries(inode, name, Type(), oldKey, oldLength, newKey, newLength);
+		
 	BPlusTree *tree;
 	if ((status = Node()->GetTree(&tree)) < B_OK)
 		return status;

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

@@ -548,12 +548,12 @@ Inode::AddSmallData(Transaction *transaction, const char *name, uint32 type,
 	// correctly terminate the small_data section
 	item = item->Next();
 	if (!item->IsLast(Node()))
-		memset(item,0,(uint8 *)Node() + fVolume->InodeSize() - (uint8 *)item);
+		memset(item, 0, (uint8 *)Node() + fVolume->InodeSize() - (uint8 *)item);
 
 	// update all current iterators
 	AttributeIterator *iterator = NULL;
 	while ((iterator = fIterators.Next(iterator)) != NULL)
-		iterator->Update(index,1);
+		iterator->Update(index, 1);
 
 	return B_OK;
 }
@@ -733,30 +733,28 @@ Inode::WriteAttribute(Transaction *transaction, const char *name, int32 type, of
 	uint8 oldBuffer[BPLUSTREE_MAX_KEY_LENGTH], *oldData = NULL;
 	size_t oldLength = 0;
 
-	// ToDo: we actually depend on that the contents of "buffer" are constant
+	// ToDo: we actually depend on that the contents of "buffer" are constant.
-	// if they get changed during the write (hey, user programs), we may mess
+	// If they get changed during the write (hey, user programs), we may mess
 	// up our index trees!
 
 	Index index(fVolume);
-	bool hasIndex = index.SetTo(name) == B_OK;
+	index.SetTo(name);
 
 	Inode *attribute = NULL;
 	status_t status = B_OK;
 
 	if (GetAttribute(name, &attribute) < B_OK) {
 		// save the old attribute data
-		if (hasIndex) {
+		fSmallDataLock.Lock();
-			fSmallDataLock.Lock();
 
-			small_data *smallData = FindSmallData(name);
+		small_data *smallData = FindSmallData(name);
-			if (smallData != NULL) {
+		if (smallData != NULL) {
-				oldLength = smallData->data_size;
+			oldLength = smallData->data_size;
-				if (oldLength > BPLUSTREE_MAX_KEY_LENGTH)
+			if (oldLength > BPLUSTREE_MAX_KEY_LENGTH)
-					oldLength = BPLUSTREE_MAX_KEY_LENGTH;
+				oldLength = BPLUSTREE_MAX_KEY_LENGTH;
-				memcpy(oldData = oldBuffer, smallData->Data(), oldLength);
+			memcpy(oldData = oldBuffer, smallData->Data(), oldLength);
-			}
-			fSmallDataLock.Unlock();
 		}
+		fSmallDataLock.Unlock();
 
 		// if the attribute doesn't exist yet (as a file), try to put it in the
 		// small_data section first - if that fails (due to insufficent space),
@@ -773,7 +771,7 @@ Inode::WriteAttribute(Transaction *transaction, const char *name, int32 type, of
 	if (attribute != NULL) {
 		if (attribute->Lock().LockWrite() == B_OK) {
 			// save the old attribute data (if this fails, oldLength will reflect it)
-			if (hasIndex) {
+			if (fVolume->CheckForLiveQuery(name)) {
 				oldLength = BPLUSTREE_MAX_KEY_LENGTH;
 				if (attribute->ReadAt(0, oldBuffer, &oldLength) == B_OK)
 					oldData = oldBuffer;
@@ -817,7 +815,7 @@ Inode::RemoveAttribute(Transaction *transaction, const char *name)
 	bool hasIndex = index.SetTo(name) == B_OK;
 
 	// update index for attributes in the small_data section
-	if (hasIndex) {
+	{
 		fSmallDataLock.Lock();
 
 		small_data *smallData = FindSmallData(name);
@@ -825,7 +823,7 @@ Inode::RemoveAttribute(Transaction *transaction, const char *name)
 			uint32 length = smallData->data_size;
 			if (length > BPLUSTREE_MAX_KEY_LENGTH)
 				length = BPLUSTREE_MAX_KEY_LENGTH;
-			index.Update(transaction, name, 0, smallData->Data(), length, NULL, 0, this);
+			index.Update(transaction, name, smallData->type, smallData->Data(), length, NULL, 0, this);
 		}
 		fSmallDataLock.Unlock();
 	}
@@ -842,11 +840,12 @@ Inode::RemoveAttribute(Transaction *transaction, const char *name)
 
 		// update index
 		Inode *attribute;
-		if (hasIndex && GetAttribute(name, &attribute) == B_OK) {
+		if ((hasIndex || fVolume->CheckForLiveQuery(name))
+			&& GetAttribute(name, &attribute) == B_OK) {
 			uint8 data[BPLUSTREE_MAX_KEY_LENGTH];
 			size_t length = BPLUSTREE_MAX_KEY_LENGTH;
 			if (attribute->ReadAt(0, data, &length) == B_OK)
-				index.Update(transaction, name, 0, data, length, NULL, 0, this);
+				index.Update(transaction, name, attribute->Type(), data, length, NULL, 0, this);
 
 			ReleaseAttribute(attribute);
 		}
@@ -2023,7 +2022,7 @@ Inode::Create(Transaction *transaction, Inode *parent, const char *name, int32 m
 	// Everything worked well until this point, we have a fully
 	// initialized inode, and we want to keep it
 	allocator.Keep();
-	
+
 	if (_id != NULL)
 		*_id = inode->ID();
 	if (_inode != NULL)

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

@@ -98,6 +98,7 @@ class Inode : public CachedBlock {
 		SimpleLock &SmallDataLock() { return fSmallDataLock; }
 
 		mode_t Mode() const { return Node()->mode; }
+		uint32 Type() const { return Node()->type; }
 		int32 Flags() const { return Node()->flags; }
 		bool IsContainer() const { return Mode() & (S_DIRECTORY | S_INDEX_DIR | S_ATTR_DIR); }
 			// note, that this test will also be true for S_IFBLK (not that it's used in the fs :)

src/add-ons/kernel/file_systems/bfs/ToDo

@@ -54,11 +54,13 @@ Inode
 Indices
 
  - consider Index::UpdateLastModified() writing back the updated inode
+ - clearing up Index::Update() and live query update (seems to be a bit confusing right now)
 
 
 Attributes
 
  - Inode::WriteAttribute() doesn't check if the attribute data may fit into the small_data region if there already is that attribute as an attribute file
+ - for indices, we could get the old data from there when doing a query update
 
 
 Volume

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

@@ -277,6 +277,20 @@ Volume::UpdateLiveQueries(Inode *inode, const char *attribute, int32 type, const
 }
 
 
+/** Checks if there is a live query whose results depend on the presence
+ *	or value of the specified attribute.
+ *	Don't use it if you already have all the data together to evaluate
+ *	the queries - it wouldn't safe you anything in this case.
+ */
+
+bool 
+Volume::CheckForLiveQuery(const char *attribute)
+{
+	// ToDo: check for a live query that depends on the specified attribute
+	return true;
+}
+
+
 void 
 Volume::AddQuery(Query *query)
 {

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

@@ -99,6 +99,7 @@ class Volume {
 		void				UpdateLiveQueries(Inode *inode, const char *attribute, int32 type,
 								const uint8 *oldKey, size_t oldLength,
 								const uint8 *newKey, size_t newLength);
+		bool				CheckForLiveQuery(const char *attribute);
 		void				AddQuery(Query *query);
 		void				RemoveQuery(Query *query);
 

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

@@ -1701,7 +1701,7 @@ bfs_read_attrdir(void *_ns, void *node, void *_cookie, long *num, struct dirent
 int
 bfs_remove_attr(void *_ns, void *_node, const char *name)
 {
-	FUNCTION_START(("name = \"%s\"\n",name));
+	FUNCTION_START(("name = \"%s\"\n", name));
 
 	if (_ns == NULL || _node == NULL || name == NULL)
 		return B_BAD_VALUE;
@@ -1746,7 +1746,8 @@ bfs_stat_attr(void *ns, void *_node, const char *name, struct attr_info *attrInf
 	Inode *inode = (Inode *)_node;
 	if (inode == NULL || inode->Node() == NULL)
 		RETURN_ERROR(B_ERROR);
-	
+
+	// first, try to find it in the small data region
 	small_data *smallData = NULL;
 	if (inode->SmallDataLock().Lock() == B_OK)
 	{
@@ -1759,12 +1760,12 @@ bfs_stat_attr(void *ns, void *_node, const char *name, struct attr_info *attrInf
 	if (smallData != NULL)
 		return B_OK;
 
-	// search in the attribute directory
+	// then, search in the attribute directory
 	Inode *attribute;
 	status_t status = inode->GetAttribute(name, &attribute);
 	if (status == B_OK) {
-		attrInfo->type = attribute->Node()->type;
+		attrInfo->type = attribute->Type();
-		attrInfo->size = attribute->Node()->data.size;
+		attrInfo->size = attribute->Size();
 
 		inode->ReleaseAttribute(attribute);
 		return B_OK;
@@ -1786,10 +1787,12 @@ bfs_write_attr(void *_ns, void *_node, const char *name, int type, const void *b
 	// Writing the name attribute using this function is not allowed,
 	// also using the reserved indices name, last_modified, and size
 	// shouldn't be allowed.
+	// ToDo: we might think about allowing to update those values, but
+	//	really change their corresponding values in the bfs_inode structure
 	if (name[0] == FILE_NAME_NAME && name[1] == '\0'
-		|| !strcmp(name,"name")
+		|| !strcmp(name, "name")
-		|| !strcmp(name,"last_modified")
+		|| !strcmp(name, "last_modified")
-		|| !strcmp(name,"size"))
+		|| !strcmp(name, "size"))
 		RETURN_ERROR(B_NOT_ALLOWED);
 
 	Volume *volume = (Volume *)_ns;