tidy up syscall.c, add comments, add missing checks, fix bug in open

kernel/syscall.c

@@ -47,6 +47,11 @@ static int get_fildes(void)
 	task_t* curr_task = per_core(current_task);
 	int fd; 
 
+	/* 
+	 * Seach the process specific file descriptor table for a free 
+	 * descriptor. The new descriptor returned by the call is the lowest
+     	 * numbered descriptor currently not in use by the process. 
+	 */
 	for (fd = 0; fd < NR_OPEN; fd++) {
 		if (curr_task->fildes_table[fd] == NULL) {
 			/* Init Filedescriptor */
@@ -60,32 +65,55 @@ static int get_fildes(void)
 	return -EMFILE;
 }
 
+static int sys_write(int fd, const char* buf, size_t len)
+{
+	task_t* curr_task = per_core(current_task);
+
+	/* fd is negative or greater than the maximum allowable number */ 
+	if (BUILTIN_EXPECT((fd >= NR_OPEN) || (fd < 0), 0))
+		return -EBADF;
+	/* fd is not an active, valid file descriptor. */
+	if (curr_task->fildes_table[fd] == NULL)
+		return -EBADF;
+
+	return write_fs(curr_task->fildes_table[fd], (uint8_t*)buf, len);
+}
+
 static int sys_open(const char* name, int flags, int mode)
 {
 	int fd, check;
-	fildes_t* file = NULL;
+	task_t* curr_task = per_core(current_task);
 
+	/* no name is given */
+	if (name == NULL)
+		return -EINVAL;
+
+	/* Get a free file descriptor */
 	fd = get_fildes();
-	/* validate the fd */
-	if (fd < 0)
-		return fd;
 
-	file = per_core(current_task)->fildes_table[fd];
+	/* fd is negative or greater than the maximum allowable number */ 
-	file->mode = mode;
+	if (BUILTIN_EXPECT((fd >= NR_OPEN) || (fd < 0), 0))
-	file->flags = flags;
+		return fd; /* in this case fd = errno */
-	file->count = 1;
+
-	check = open_fs(file, (char*) name);
+	/* init the whole file descriptor structure */
+	curr_task->fildes_table[fd]->mode = mode;
+	curr_task->fildes_table[fd]->flags = flags;
+	curr_task->fildes_table[fd]->count = 1;
+	check = open_fs(curr_task->fildes_table[fd], (char*) name);
+
+	/* file doesn't exist! */
 	if (check < 0) {
-		/* file doesn't exist! */
+		/* tidy up the fildescriptor */
-		kfree(file, sizeof(fildes_t));
+		kfree(curr_task->fildes_table[fd], sizeof(fildes_t));
-		file = NULL;
+		curr_task->fildes_table[fd] = NULL;
 		return check;
  	}
 	
 	return fd;
 }
 
-static int sys_stat(const char* name, struct stat* st) {
+static int sys_stat(const char* name, struct stat* st) 
+{
 	vfs_node_t* node;
 
 	node = findnode_fs(name);
@@ -112,43 +140,102 @@ static int sys_stat(const char* name, struct stat* st) {
 	return 0;
 }
 
+static int sys_read(int fd, const char* buf, size_t len)
+{
+	task_t* curr_task = per_core(current_task);
+
+	/* fd is negative or greater than the maximum allowable number */ 
+	if (BUILTIN_EXPECT((fd >= NR_OPEN) || (fd < 0), 0))
+		return -EBADF;
+	/* fd is not an active, valid file descriptor. */
+	if (curr_task->fildes_table[fd] == NULL)
+		return -EBADF;
+
+	return read_fs(curr_task->fildes_table[fd], (uint8_t*)buf, len);
+}
+
+
 #if defined(CONFIG_LWIP) && LWIP_SOCKET
 static int sys_socket(int domain, int type, int protocol)
 {
-	int fd;
+	int fd, sock;
-	fildes_t* file = NULL;
+	task_t* curr_task = per_core(current_task);
 
+	/* Get a free file descriptor */
 	fd = get_fildes();
-	/* validate the fd */
-	if (fd < 0)
-		return fd;
 
-	file = per_core(current_task)->fildes_table[fd];
+	/* validate the file descriptor */
-	file->offset = lwip_socket(domain, type, protocol);
+	if (BUILTIN_EXPECT((fd >= NR_OPEN) || (fd < 0), 0))
-	file->node = findnode_fs("/dev/socket"); 
+		return fd; /* in this case fd = errno */
-	file->count = 1;
+
+	/* Get a valid lwip descriptor */
+	sock = lwip_socket(domain, type, protocol);
+
+	/* validate the file descriptor */
+	if (BUILTIN_EXPECT(sock < 0, 0))
+		return sock; /* in this case sock = errno */
+
+	/* 
+	 * init the whole file descriptor structure.
+	 * We use the offset to save the lwip descriptor
+	 * TODO: find another solution or change the name 'offset'
+	 */
+	curr_task->fildes_table[fd]->offset = sock;
+	curr_task->fildes_table[fd]->count = 1;
+	curr_task->fildes_table[fd]->node = findnode_fs("/dev/socket"); 
+
+	/* file doesn't exist! */
+	if (curr_task->fildes_table[fd]->node == NULL) {
+		/* tidy up the fildescriptor */
+		kfree(curr_task->fildes_table[fd], sizeof(fildes_t));
+		curr_task->fildes_table[fd] = NULL;
+		return -ENOENT;
+ 	}
 
 	return fd;
 }
 
 static int sys_accept(int s, struct sockaddr* addr, socklen_t* addrlen)
 {
-	int fd;
+	int fd, sock2;
-	fildes_t* file = NULL;
+	task_t* curr_task = per_core(current_task);
 
-	if (BUILTIN_EXPECT(s >= NR_OPEN, 0))
+	/* validate the 'socket'-filedescriptor */
-		return -EINVAL;
+	if (BUILTIN_EXPECT((s >= NR_OPEN) || (s < 0), 0))
+		return -EBADF;
+	/* validate the 'socket'-file descriptor object */
+	if (curr_task->fildes_table[s] == NULL)
+		return -EBADF;
 
+	/* Get a free file descriptor */
 	fd = get_fildes();
-	/* validate the fd */
-	if (fd < 0) 
-		return fd;
 
-	file = per_core(current_task)->fildes_table[fd];
+	/* validate the file descriptor */
-	file->offset = lwip_accept(s, addr, addrlen);
+	if (BUILTIN_EXPECT((fd >= NR_OPEN) || (fd < 0), 0))
-	file->node = findnode_fs("/dev/socket");
+		return fd; /* in this case fd = errno */
-	file->count = 1;
+	/* Get a valid lwip descriptor */
+	sock2 = lwip_accept(s, addr, addrlen);
+	/* validate the file descriptor */
+	if (BUILTIN_EXPECT(sock2 < 0, 0))
+		return sock2; /* in this case sock = errno */
+
+	/* 
+	 * init the whole file descriptor structure.
+	 * We use the offset to save the lwip descriptor
+	 * TODO: find another solution or change the name 'offset'
+	 */
+	curr_task->fildes_table[fd]->offset = sock2;
+	curr_task->fildes_table[fd]->count = 1;
+	curr_task->fildes_table[fd]->node = findnode_fs("/dev/socket");
 	
+	/* file doesn't exist! */
+	if (curr_task->fildes_table[fd]->node == NULL) {
+		/* tidy up the fildescriptor */
+		kfree(curr_task->fildes_table[fd], sizeof(fildes_t));
+		curr_task->fildes_table[fd] = NULL;
+		return -ENOENT;
+ 	}
+
 	return fd;
 }
 
@@ -157,13 +244,26 @@ static int sys_accept(int s, struct sockaddr* addr, socklen_t* addrlen)
 
 static int sys_close(int fd)
 {
+	int check;
 	task_t* curr_task = per_core(current_task);
-	
-	if (BUILTIN_EXPECT(fd >= NR_OPEN, 0))
-		return -EINVAL;
 
+	/* fd is negative or greater than the maximum allowable number */ 
+	if (BUILTIN_EXPECT((fd >= NR_OPEN) || (fd < 0), 0))
+		return -EBADF;
+	/* fd is not an active, valid file descriptor. */
+	if (curr_task->fildes_table[fd] == NULL)
+		return -EBADF;
+
+	/*
+	 * The close() call deletes a descriptor from the per-process object 
+	 * reference table.  If this is the last reference to the underlying 
+	 * object, the object will be deactivated. 
+	 */
 	if (curr_task->fildes_table[fd]->count == 1) {
-		close_fs(curr_task->fildes_table[fd]);
+		check = close_fs(curr_task->fildes_table[fd]);
+		/* close command failed -> return check = errno */
+		if (BUILTIN_EXPECT(check < 0, 0))
+			return check; 
 		kfree(curr_task->fildes_table[fd], sizeof(fildes_t));
 		curr_task->fildes_table[fd] = NULL;
 	} else {
@@ -178,30 +278,42 @@ static int sys_close(int fd)
 static int sys_lseek(int fd, off_t pos, int origin)
 {
 	int ret = -EINVAL;
-	fildes_t* file;
+	task_t* curr_task = per_core(current_task);
 
-	if (BUILTIN_EXPECT(fd >= NR_OPEN, 0))
+	/* fd is negative or greater than the maximum allowable number */ 
-		return -EINVAL;
+	if (BUILTIN_EXPECT((fd >= NR_OPEN) || (fd < 0), 0))
+		return -EBADF;
+	/* fd is not an active, valid file descriptor. */
+	if (curr_task->fildes_table[fd] == NULL)
+		return -EBADF;
 
-	file = per_core(current_task)->fildes_table[fd];
+	/* TODO: in case of Fildes is associated with a pipe, socket, or FIFO. return ESPIPE */
-
+	/* TODO: The seek location is too large to be stored in an object of type off_t. return EOVERFLOW*/
-	if (BUILTIN_EXPECT(!((file->node->type == FS_FILE) 
-			|| (file->node->type == FS_CHARDEVICE)), 0))
-		return -EINVAL;
 
 	switch(origin) 
-	{	
+	{
 	case SEEK_SET: {	/* set file offset to offset */
-			file->offset = pos;
+			ret = pos;
+			/* The seek location is negative. */
+			if (ret < 0) 
+				return -EINVAL;
+			curr_task->fildes_table[fd]->offset = ret;
 			ret = 0;
 			break;
 		}
 	case SEEK_CUR: {	/* set file offset to current plus offset */
-			ret = pos + file->offset;
+			ret = pos + curr_task->fildes_table[fd]->offset;
+			/* The seek location is negative. */
+			if (ret < 0) 
+				return -EINVAL;
 			break;
 		}
 	case SEEK_END: {	/* set file offset to EOF plus offset */
-			file->offset = pos + file->node->block_size;
+			ret = pos + curr_task->fildes_table[fd]->node->block_size;
+			/* The seek location is negative. */
+			if (ret < 0) 
+				return -EINVAL;
+			curr_task->fildes_table[fd]->offset = ret;
 			ret = 0;
 			break;
 		}
@@ -209,6 +321,7 @@ static int sys_lseek(int fd, off_t pos, int origin)
 		ret = -EINVAL;
 		break;
 	}
+
 	return ret;
 }
 
@@ -217,12 +330,19 @@ static int sys_dup(int fd)
 	task_t* curr_task = per_core(current_task);
 	int new_fd;
 
+	/* fd is negative or greater than the maximum allowable number */ 
 	if (BUILTIN_EXPECT((fd >= NR_OPEN) || (fd < 0), 0))
 		return -EBADF;
+	/* fd is not an active, valid file descriptor. */
 	if (curr_task->fildes_table[fd] == NULL)
 		return -EBADF;
 
+	/* Get a free file descriptor */
 	new_fd = get_fildes();
+	/* validate the file descriptor */
+	if (BUILTIN_EXPECT((new_fd >= NR_OPEN) || (fd < 0), 0))
+		return new_fd; /* in this case fd = errno */
+
 	curr_task->fildes_table[new_fd] = curr_task->fildes_table[fd];
 	curr_task->fildes_table[fd]->count++;
 
@@ -233,18 +353,26 @@ static int sys_dup2(int fd, int fd2)
 {
 	task_t* curr_task = per_core(current_task);
 
+	/* fd and fd2 is negative or greater than the maximum allowable number */ 
 	if (BUILTIN_EXPECT((fd >= NR_OPEN) || (fd < 0), 0))
 		return -EBADF;
 	if (BUILTIN_EXPECT((fd2 >= NR_OPEN) || (fd2 < 0), 0))
 		return -EBADF;
+	/* fd is not an active, valid file descriptor. */
 	if (curr_task->fildes_table[fd] == NULL)
 		return -EBADF;
 
-	if (fd == fd2) 
+	/* If fd and fd2 are equal, then dup2() just returns fd2 */
+	if (fd == fd2)
 		return fd2;
+	
+	/*
+	 * if descriptor fd2 is already in use, it is first deallocated 
+	 * as if a close(2) call had been done first
+	 */
 	if (curr_task->fildes_table[fd2] != NULL)
 		sys_close(fd2);
-	
+
 	curr_task->fildes_table[fd2] = curr_task->fildes_table[fd];
 	curr_task->fildes_table[fd]->count++;
 
@@ -296,9 +424,7 @@ int syscall_handler(uint32_t sys_nr, ...)
 			int fd = va_arg(vl, int);
 			const char* buf = va_arg(vl, const char*);
 			size_t len = va_arg(vl, size_t);
-			if (fd >= 0)
+			ret = sys_write(fd, buf, len);
-				ret = write_fs(per_core(current_task)->fildes_table[fd],
-					(uint8_t*)buf, len);
 			break;
 		}
 	case __NR_open: {
@@ -310,8 +436,7 @@ int syscall_handler(uint32_t sys_nr, ...)
 		}
 	case __NR_close: {
 			int fd = va_arg(vl, int);
-			if (fd >= 0)
+			ret = sys_close(fd);
-				ret = sys_close(fd);
 			break;
  		}
 	case __NR_dup: {
@@ -328,7 +453,6 @@ int syscall_handler(uint32_t sys_nr, ...)
 	case __NR_stat: {
 			const char* name = va_arg(vl, const char*);
 			struct stat* st = va_arg(vl, struct stat*);
-	
 			ret = sys_stat(name, st);
 			break;
 		}
@@ -336,17 +460,14 @@ int syscall_handler(uint32_t sys_nr, ...)
 			int fd = va_arg(vl, int);
 			const char* buf = va_arg(vl, const char*);
  			size_t len = va_arg(vl, size_t);
-			if (fd >= 0)
+			ret = sys_read(fd, buf, len);
-				ret = read_fs(per_core(current_task)->fildes_table[fd], 
-					(uint8_t*)buf, len);
 			break;
 		}
 	case __NR_lseek: {
 			int fd = va_arg(vl, int);
 			off_t pos = va_arg(vl, off_t);
  			int origin = va_arg(vl, int);
-			if (fd >= 0)
+			ret = sys_lseek(fd, pos, origin);
-				ret = sys_lseek(fd, pos, origin);
 			break;
 		}
 	case __NR_sbrk: {