[Haiku-commits] r31313 - in haiku/trunk/src: add-ons/kernel/file_systems/bfs tests/system/libroot/os

axeld at BerliOS axeld at mail.berlios.de
Mon Jun 29 12:40:14 CEST 2009


Author: axeld
Date: 2009-06-29 12:40:01 +0200 (Mon, 29 Jun 2009)
New Revision: 31313
ViewCVS: http://svn.berlios.de/viewcvs/haiku?rev=31313&view=rev

Added:
   haiku/trunk/src/tests/system/libroot/os/fs_attr_test.cpp
Modified:
   haiku/trunk/src/add-ons/kernel/file_systems/bfs/Inode.cpp
   haiku/trunk/src/add-ons/kernel/file_systems/bfs/Inode.h
   haiku/trunk/src/tests/system/libroot/os/Jamfile
Log:
* Inode::_AddSmallData() now supports writing at an arbitrary position.
  However, Inode::WriteAttribute() still has a number of problems when this is
  actually used; contents could get lost when an attribute is moved from the
  small data section to an attribute file, and the index might not be updated
  correctly when you write within the first 256 bytes, but not at position 0.
  Since these problems aren't exposed with how we're using BFS right now, it's
  not that bad, though (Inode::WriteAttribute() supports everything correctly
  that it had to under BeOS).
* Added test application for certain fs_attr functions.


Modified: haiku/trunk/src/add-ons/kernel/file_systems/bfs/Inode.cpp
===================================================================
--- haiku/trunk/src/add-ons/kernel/file_systems/bfs/Inode.cpp	2009-06-29 09:56:22 UTC (rev 31312)
+++ haiku/trunk/src/add-ons/kernel/file_systems/bfs/Inode.cpp	2009-06-29 10:40:01 UTC (rev 31313)
@@ -663,9 +663,9 @@
 */
 status_t
 Inode::_AddSmallData(Transaction& transaction, NodeGetter& nodeGetter,
-	const char* name, uint32 type, const uint8* data, size_t length, bool force)
+	const char* name, uint32 type, off_t pos, const uint8* data, size_t length,
+	bool force)
 {
-	// TODO: support new write attr semantics and write offset!
 	bfs_inode* node = nodeGetter.WritableNode();
 
 	if (node == NULL || name == NULL || data == NULL)
@@ -673,7 +673,7 @@
 
 	// reject any requests that can't fit into the small_data section
 	uint32 nameLength = strlen(name);
-	uint32 spaceNeeded = sizeof(small_data) + nameLength + 3 + length + 1;
+	uint32 spaceNeeded = sizeof(small_data) + nameLength + 3 + pos + length + 1;
 	if (spaceNeeded > fVolume->InodeSize() - sizeof(bfs_inode))
 		return B_DEVICE_FULL;
 
@@ -697,22 +697,22 @@
 			last = last->Next();
 
 		// try to change the attributes value
-		if (item->data_size > length
+		if (item->data_size > pos + length
 			|| force
-			|| ((uint8*)last + length - item->DataSize())
+			|| ((uint8*)last + pos + length - item->DataSize())
 					<= ((uint8*)node + fVolume->InodeSize())) {
 			// Make room for the new attribute if needed (and we are forced
 			// to do so)
-			if (force && ((uint8*)last + length - item->DataSize())
+			if (force && ((uint8*)last + pos + length - item->DataSize())
 					> ((uint8*)node + fVolume->InodeSize())) {
 				// We also take the free space at the end of the small_data
 				// section into account, and request only what's really needed
-				uint32 needed = length - item->DataSize() -
+				uint32 needed = pos + length - item->DataSize() -
 					(uint32)((uint8*)node + fVolume->InodeSize()
 						- (uint8*)last);
 
 				if (_MakeSpaceForSmallData(transaction, node, name, needed)
-						< B_OK)
+						!= B_OK)
 					return B_ERROR;
 
 				// reset our pointers
@@ -728,9 +728,11 @@
 					last = last->Next();
 			}
 
+			size_t oldDataSize = item->DataSize();
+
 			// Normally, we can just overwrite the attribute data as the size
 			// is specified by the type and does not change that often
-			if (length != item->DataSize()) {
+			if (pos + length != item->DataSize()) {
 				// move the attributes after the current one
 				small_data* next = item->Next();
 				if (!next->IsLast(node)) {
@@ -747,13 +749,18 @@
 						- (uint8*)last);
 				}
 
-				item->data_size = HOST_ENDIAN_TO_BFS_INT16(length);
+				item->data_size = HOST_ENDIAN_TO_BFS_INT16(pos + length);
 			}
 
 			item->type = HOST_ENDIAN_TO_BFS_INT32(type);
-			memcpy(item->Data(), data, length);
-			item->Data()[length] = '\0';
 
+			if (oldDataSize < pos) {
+				// Fill gap with zeros
+				memset(item->Data() + oldDataSize, 0, pos - oldDataSize);
+			}
+			memcpy(item->Data() + pos, data, length);
+			item->Data()[pos + length] = '\0';
+
 			return B_OK;
 		}
 
@@ -785,7 +792,7 @@
 	item->name_size = HOST_ENDIAN_TO_BFS_INT16(nameLength);
 	item->data_size = HOST_ENDIAN_TO_BFS_INT16(length);
 	strcpy(item->Name(), name);
-	memcpy(item->Data(), data, length);
+	memcpy(item->Data() + pos, data, length);
 
 	// correctly terminate the small_data section
 	item = item->Next();
@@ -911,7 +918,7 @@
 	NodeGetter node(fVolume, transaction, this);
 	const char nameTag[2] = {FILE_NAME_NAME, 0};
 
-	return _AddSmallData(transaction, node, nameTag, FILE_NAME_TYPE,
+	return _AddSmallData(transaction, node, nameTag, FILE_NAME_TYPE, 0,
 		(uint8*)name, strlen(name), true);
 }
 
@@ -1021,8 +1028,12 @@
 Inode::WriteAttribute(Transaction& transaction, const char* name, int32 type,
 	off_t pos, const uint8* buffer, size_t* _length)
 {
+	if (pos < 0)
+		return B_BAD_VALUE;
+
 	// needed to maintain the index
-	uint8 oldBuffer[BPLUSTREE_MAX_KEY_LENGTH], *oldData = NULL;
+	uint8 oldBuffer[BPLUSTREE_MAX_KEY_LENGTH];
+	uint8* oldData = NULL;
 	size_t oldLength = 0;
 
 	// TODO: we actually depend on that the contents of "buffer" are constant.
@@ -1038,7 +1049,9 @@
 	Inode* attribute = NULL;
 	status_t status = B_OK;
 
-	if (GetAttribute(name, &attribute) < B_OK) {
+	if (GetAttribute(name, &attribute) != B_OK) {
+		// No attribute inode exists yet
+
 		// save the old attribute data
 		NodeGetter node(fVolume, transaction, this);
 		recursive_lock_lock(&fSmallDataLock);
@@ -1057,7 +1070,8 @@
 		// 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),
 		// create a real attribute file
-		status = _AddSmallData(transaction, node, name, type, buffer, *_length);
+		status = _AddSmallData(transaction, node, name, type, pos, buffer,
+			*_length);
 		if (status == B_DEVICE_FULL) {
 			if (smallData != NULL) {
 				// remove the old attribute from the small data section - there
@@ -1068,7 +1082,7 @@
 
 			if (status == B_OK)
 				status = CreateAttribute(transaction, name, type, &attribute);
-			if (status < B_OK)
+			if (status != B_OK)
 				RETURN_ERROR(status);
 		} else if (status == B_OK)
 			status = WriteBack(transaction);
@@ -1087,7 +1101,7 @@
 
 			// check if the data fits into the small_data section again
 			NodeGetter node(fVolume, transaction, this);
-			status = _AddSmallData(transaction, node, name, type, buffer,
+			status = _AddSmallData(transaction, node, name, type, pos, buffer,
 				*_length);
 
 			if (status == B_OK) {
@@ -1166,6 +1180,10 @@
 }
 
 
+/*!	Returns the attribute inode with the specified \a name, in case it exists.
+	This method can only return real attribute files; the attributes in the
+	small data section are ignored.
+*/
 status_t
 Inode::GetAttribute(const char* name, Inode** _attribute)
 {

Modified: haiku/trunk/src/add-ons/kernel/file_systems/bfs/Inode.h
===================================================================
--- haiku/trunk/src/add-ons/kernel/file_systems/bfs/Inode.h	2009-06-29 09:56:22 UTC (rev 31312)
+++ haiku/trunk/src/add-ons/kernel/file_systems/bfs/Inode.h	2009-06-29 10:40:01 UTC (rev 31313)
@@ -196,7 +196,7 @@
 								NodeGetter& node, const char* name);
 			status_t		_AddSmallData(Transaction& transaction,
 								NodeGetter& node, const char* name, uint32 type,
-								const uint8* data, size_t length,
+								off_t pos, const uint8* data, size_t length,
 								bool force = false);
 			status_t		_GetNextSmallData(bfs_inode* node,
 								small_data** _smallData) const;

Modified: haiku/trunk/src/tests/system/libroot/os/Jamfile
===================================================================
--- haiku/trunk/src/tests/system/libroot/os/Jamfile	2009-06-29 09:56:22 UTC (rev 31312)
+++ haiku/trunk/src/tests/system/libroot/os/Jamfile	2009-06-29 10:40:01 UTC (rev 31313)
@@ -1,22 +1,23 @@
 SubDir HAIKU_TOP src tests system libroot os ;
 
-SetSubDirSupportedPlatforms libbe_test ;
+SimpleTest DriverSettingsTest :
+	DriverSettingsTest.cpp
+	driver_settings.c
+	: be
+;
 
-#SubDirCcFlags -fcheck-memory-usage -O0 -D_NO_INLINE_ASM ;
+SimpleTest ParseDateTest :
+	ParseDateTest.cpp parsedate.cpp
+;
 
-SimpleTest DriverSettingsTest
-	: DriverSettingsTest.cpp driver_settings.c
-	: be
-	;
+SimpleTest FindDirectoryTest :
+	FindDirectoryTest.cpp
+;
 
-SimpleTest ParseDateTest
-	: ParseDateTest.cpp parsedate.cpp
-	;
+SimpleTest fs_attr_test :
+	fs_attr_test.cpp
+;
 
-SimpleTest FindDirectoryTest
-	: FindDirectoryTest.cpp
-	;
-
 # Tell Jam where to find these sources
 SEARCH on [ FGristFiles
 		driver_settings.c

Added: haiku/trunk/src/tests/system/libroot/os/fs_attr_test.cpp
===================================================================
--- haiku/trunk/src/tests/system/libroot/os/fs_attr_test.cpp	2009-06-29 09:56:22 UTC (rev 31312)
+++ haiku/trunk/src/tests/system/libroot/os/fs_attr_test.cpp	2009-06-29 10:40:01 UTC (rev 31313)
@@ -0,0 +1,96 @@
+/*
+ * Copyright 2009, Axel Dörfler, axeld at pinc-software.de.
+ * This file may be used under the terms of the MIT License.
+ */
+
+
+#include <errno.h>
+#include <stdio.h>
+#include <string.h>
+
+#include <fs_attr.h>
+#include <TypeConstants.h>
+
+
+const char* kTestFileName = "/tmp/fs_attr_test";
+
+
+void
+test_read(int fd, const char* attribute, type_code type, const char* data,
+	size_t length)
+{
+	attr_info info;
+	if (fs_stat_attr(fd, attribute, &info) != 0) {
+		fprintf(stderr, "Could not stat attribute \"%s\": %s\n", attribute,
+			strerror(errno));
+		exit(1);
+	}
+
+	if (info.size != length) {
+		fprintf(stderr, "Length does not match for \"%s\": should be %ld, is "
+			"%lld\n", data, length, info.size);
+		exit(1);
+	}
+
+	if (info.type != type) {
+		fprintf(stderr, "Type does not match B_RAW_TYPE!\n");
+		exit(1);
+	}
+
+	char buffer[4096];
+	ssize_t bytesRead = fs_read_attr(fd, attribute, B_RAW_TYPE, 0, buffer,
+		length);
+	if (bytesRead != (ssize_t)length) {
+		fprintf(stderr, "Bytes read does not match: should be %ld, is %ld\n",
+			length, bytesRead);
+		exit(1);
+	}
+
+	if (memcmp(data, buffer, length)) {
+		fprintf(stderr, "Bytes do not match: should be \"%s\", is \"%s\"\n",
+			data, buffer);
+		exit(1);
+	}
+}
+
+
+int
+main(int argc, char** argv)
+{
+	int fd = open(kTestFileName, O_CREAT | O_TRUNC | O_WRONLY);
+	if (fd < 0) {
+		fprintf(stderr, "Creating test file \"%s\" failed: %s\n", kTestFileName,
+			strerror(errno));
+		return 1;
+	}
+
+	// Test the old BeOS API
+
+	fs_write_attr(fd, "TEST", B_STRING_TYPE, 0, "Hello BeOS", 11);
+	test_read(fd, "TEST", B_STRING_TYPE, "Hello BeOS", 11);
+
+	// TODO: this shows a bug in BFS; the index is not updated correctly
+	fs_write_attr(fd, "TEST", B_STRING_TYPE, 6, "Haiku", 6);
+	test_read(fd, "TEST", B_STRING_TYPE, "Hello Haiku", 12);
+
+	fs_write_attr(fd, "TESTraw", B_RAW_TYPE, 16, "Haiku", 6);
+	test_read(fd, "TESTraw", B_RAW_TYPE, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0Haiku",
+		22);
+
+	fs_write_attr(fd, "TESTraw", B_RAW_TYPE, 0, "Haiku", 6);
+	test_read(fd, "TESTraw", B_RAW_TYPE, "Haiku", 6);
+
+	char buffer[4096];
+	memset(buffer, 0, sizeof(buffer));
+	strcpy(buffer, "Hello");
+	fs_write_attr(fd, "TESTswitch", B_RAW_TYPE, 0, buffer,
+		strlen(buffer) + 1);
+	test_read(fd, "TESTswitch", B_RAW_TYPE, buffer, strlen(buffer) + 1);
+
+	strcpy(buffer + 4000, "Haiku");
+	fs_write_attr(fd, "TESTswitch", B_RAW_TYPE, 0, buffer, 4006);
+	test_read(fd, "TESTswitch", B_RAW_TYPE, buffer, 4006);
+
+	return 0;
+}
+




More information about the Haiku-commits mailing list