hopefully final s5fixes

1 files changed, 73 insertions, 125 deletions

diff --git a/kernel/fs/s5fs/s5fs_subr.c b/kernel/fs/s5fs/s5fs_subr.c
index bd6b886..78af47b 100644
--- a/kernel/fs/s5fs/s5fs_subr.c
+++ b/kernel/fs/s5fs/s5fs_subr.c
@@ -99,8 +99,6 @@ static inline void s5_release_file_block(pframe_t **pfp)
  *                  the file
  *  alloc         - If set, allocate the block / indirect block as necessary
  *                  If clear, don't allocate sparse blocks
- *  newp          - Return parameter that should be set to 1 if the returned
- *                  block number is new (block has just been allocated)
  *
  * Return a disk block number on success, or:
  *  - 0: The block is sparse, and alloc is clear, OR
@@ -116,118 +114,102 @@ static inline void s5_release_file_block(pframe_t **pfp)
  *  - Use s5_alloc_block to allocate blocks.
  *  - Be sure to mark the inode as dirty when appropriate, i.e. when you are
  *    making changes to the actual s5_inode_t struct. Hint: Does allocating a
- *    direct block dirty the inode?(yes) What about allocating the indirect block?(yes)
- *    Finally, what about allocating a block pointed to by the indirect block?(no)
+ *    direct block dirty the inode? What about allocating the indirect block?
+ *    Finally, what about allocating a block pointed to by the indirect block?
  *  - Cases to consider:
- *    - 1) file_blocknum < S_NDIRECT_BLOCKS
- *    - 2) Indirect block is not allocated but alloc is set. Be careful not to
+ *    1) file_blocknum < S_NDIRECT_BLOCKS
+ *    2) Indirect block is not allocated but alloc is set. Be careful not to
  *       leak a block in an error case!
-    - 2a) Make sure you allocate the indirect block on disk and create a 
-         corresponding pframe_t on the mobj (Hint: see s5_cache_and_clear_block).
- *   - 3) Indirect block is allocated. The desired block may be sparse, and you
+ *    3) Indirect block is allocated. The desired block may be sparse, and you
  *       may have to allocate it.
- *   - 4) The indirect block has not been allocated and alloc is clear.
+ *    4) The indirect block has not been allocated and alloc is clear.
  */
 long s5_file_block_to_disk_block(s5_node_t *sn, size_t file_blocknum,
                                  int alloc, int *newp)
 {
-    // NOT_YET_IMPLEMENTED("S5FS: s5_file_block_to_disk_block");    
-    // return 0;
-    *newp=0;
+    // NOT_YET_IMPLEMENTED("S5FS: s5_file_block_to_disk_block");
+    if (file_blocknum >= S5_MAX_FILE_BLOCKS)
+    {
+        return -EINVAL;
+    }
+
+    // update newp to 0 for any nullish values
+    *newp = 0;
+
 
     s5fs_t *s5 = VNODE_TO_S5FS(&sn->vnode);
-    long block_num = 0;
+
+    // variables for tracking blocknums
+    long blocknum = 0;
+    short to_free = 0;
+    long prev_blocknum = 0;
     pframe_t *pf;
 
-    // Case 1: file_blocknum < S5_NDIRECT_BLOCKS
-    if (file_blocknum < S5_NDIRECT_BLOCKS){
-        block_num = sn->inode.s5_direct_blocks[file_blocknum];        
-        if (!block_num && alloc)
+    if (file_blocknum < S5_NDIRECT_BLOCKS)
+    {
+        blocknum = sn->inode.s5_direct_blocks[file_blocknum];
+        if (!blocknum && alloc)
         {
-            long alloced_block_num = s5_alloc_block(s5);
-    
-            if (alloced_block_num > 0) 
+            blocknum = s5_alloc_block(s5);
+            if (blocknum < 0)
             {
-                sn->inode.s5_direct_blocks[file_blocknum] = (uint32_t)alloced_block_num;
-                sn->dirtied_inode = 1;
-                *newp = 1;    
-                return alloced_block_num;
+                return blocknum;
             }
+
+            sn->inode.s5_direct_blocks[file_blocknum] = blocknum;
+            sn->dirtied_inode = 1;
+            *newp = 1;
         }
+        return blocknum;
     }
-    else if (file_blocknum >= S5_NDIRECT_BLOCKS &&  file_blocknum < S5_MAX_FILE_BLOCKS)
-    {
-        uint32_t indirect_block_num = sn->inode.s5_indirect_block;
-        mobj_t * mobjp = &s5->s5f_mobj;
-
-        // Case 2: Indirect block is not allocated but alloc is set
-        if (!indirect_block_num && alloc){
-            long retval = s5_alloc_block(s5);
-            
-            if (retval < 0) return (long)retval;
-            
-            mobj_lock(mobjp);
-            pframe_t *indir_pf = s5_cache_and_clear_block(mobjp, retval, retval);
-            mobj_unlock(mobjp);
-
-            block_num = s5_alloc_block(s5);
-
-
-            if (block_num <= 0)
-            { 
-                s5_free_block(s5, sn->inode.s5_indirect_block);
-                mobj_delete_pframe(&s5->s5f_mobj, retval);
-                return (long) block_num;
-            }
-            else 
-            {
-                sn->inode.s5_indirect_block = retval;
-                s5_get_meta_disk_block(s5, sn->inode.s5_indirect_block, 1, &pf);
-                memcpy(
-                    (uint32_t*)pf->pf_addr + (file_blocknum - S5_NDIRECT_BLOCKS), 
-                    &block_num, 
-                    sizeof(uint32_t)
-                );
-
-                s5_release_disk_block(&pf); 
-                
-                sn->dirtied_inode = 1; 
-                *newp = 1;  
-            }
 
-            s5_release_disk_block(&indir_pf);
+    if (!sn->inode.s5_indirect_block && alloc)
+    {
+        blocknum = s5_alloc_block(s5);
+        if (blocknum < 0)
+        {
+            return blocknum;
         }
 
-        // Case 3: Indirect block is allocated
-        else if (indirect_block_num)
-        {
-            s5_get_meta_disk_block(s5, indirect_block_num, 1, &pf);
-            file_blocknum = file_blocknum - S5_NDIRECT_BLOCKS; 
-            memcpy(
-                &block_num, 
-                (uint32_t*)(pf->pf_addr) + file_blocknum, 
-                sizeof(uint32_t)
-            );
+        sn->inode.s5_indirect_block = blocknum;
+        sn->dirtied_inode = 1;
+        *newp = 1;
+        
+        mobj_lock(&s5->s5f_mobj);
+        pf = s5_cache_and_clear_block(&s5->s5f_mobj, blocknum, 1);
+        mobj_unlock(&s5->s5f_mobj);
+
+        s5_release_disk_block(&pf);
 
-            if (!block_num && alloc)
+        prev_blocknum = blocknum;
+        to_free = 1;
+    }
+
+    s5_get_meta_disk_block(s5, sn->inode.s5_indirect_block, 1, &pf);
+    uint32_t *indirectBlock = (uint32_t *)pf->pf_addr;
+    blocknum = indirectBlock[file_blocknum - S5_NDIRECT_BLOCKS];
+
+    if (!blocknum && alloc)
+    {
+        blocknum = s5_alloc_block(s5);
+        if (blocknum < 0)
+        {   
+            if (to_free) 
             {
-                block_num = s5_alloc_block(s5);
-                
-                if (block_num > 0) 
-                {
-                    memcpy((uint32_t*)(pf->pf_addr) + file_blocknum, &block_num , sizeof(uint32_t));
-                    *newp = 1;
-                }
+                s5_free_block(s5, prev_blocknum);
             }
+
             s5_release_disk_block(&pf);
+            return blocknum;
         }
-    }
-    else 
-    {
-        return -EINVAL;
+
+        indirectBlock[file_blocknum - S5_NDIRECT_BLOCKS] = blocknum;
+        sn->dirtied_inode = 1;
+        *newp = 1;
     }
 
-    return block_num;
+    s5_release_disk_block(&pf);
+    return blocknum;
 }
 
 pframe_t *s5_cache_and_clear_block(mobj_t *mo, long block, long loc) {
@@ -235,7 +217,6 @@ pframe_t *s5_cache_and_clear_block(mobj_t *mo, long block, long loc) {
     mobj_create_pframe(mo, block, loc, &pf);
     pf->pf_addr = page_alloc();
     memset(pf->pf_addr, 0, PAGE_SIZE);
-    pf->pf_dirty = 1;  // XXX do this later
     return pf;
 }
 
@@ -314,7 +295,6 @@ ssize_t s5_read_file(s5_node_t *sn, size_t pos, char *buf, size_t len)
     return bytes_read;
 }
 
-
 /* Write to a file.
  *
  *  sn  - The s5_node representing the file to write to
@@ -568,7 +548,7 @@ long s5_alloc_inode(s5fs_t *s5fs, uint16_t type, devid_t devid)
 
     pframe_t *pf;
     s5_inode_t *inode;
-    s5_get_inode(s5fs, new_ino, 1, &pf, &inode);
+    s5_get_inode(s5fs, (ino_t)new_ino, 1, &pf, &inode);
 
     s5fs->s5f_super.s5s_free_inode = inode->s5_un.s5_next_free;
     KASSERT(inode->s5_un.s5_next_free != inode->s5_number);
@@ -811,38 +791,7 @@ void s5_replace_dirent(s5_node_t *sn, const char *name, size_t namelen,
 {
     vnode_t *dir = &sn->vnode;
     s5_inode_t *inode = &sn->inode;
-    // NOT_YET_IMPLEMENTED("S5FS: s5_replace_dirent");
-
-    // Assert that the directory exists
-    KASSERT(S_ISDIR(dir->vn_mode));
-
-    // Find the position of the old directory entry
-    size_t filepos;
-    long inode_num = s5_find_dirent(sn, name, namelen, &filepos);
-
-    // Assert that the directory entry exists
-    KASSERT(inode_num >= 0);
-    // Assert that the directory entry corresponds to the old s5_node
-    KASSERT(inode_num == old->inode.s5_number);
-
-    // Form the new directory entry
-    s5_dirent_t new_dirent;
-    strncpy(new_dirent.s5d_name, name, namelen);
-    new_dirent.s5d_inode = new->inode.s5_number;
-
-    // Write the new directory entry
-    s5_write_file(sn, filepos, (char *)&new_dirent, sizeof(s5_dirent_t));
-
-    // Update linkcounts and dirty inodes appropriately
-    old->inode.s5_linkcount--;
-
-    new->inode.s5_linkcount++;
-    new->dirtied_inode = 1;
-
-    sn->dirtied_inode = 1;
-
-    // Update the directory length
-    dir->vn_len = inode->s5_un.s5_size;
+    NOT_YET_IMPLEMENTED("S5FS: s5_replace_dirent");
 }
 
 /* Create a directory entry.
@@ -906,7 +855,7 @@ long s5_link(s5_node_t *dir, const char *name, size_t namelen,
 }
 
 /* Return the number of file blocks allocated for sn. This means any
- * direct or indirect file blocks that are not sparse. If the indirect
+ * file blocks that are not sparse, direct or indirect. If the indirect
  * block itself is allocated, that must also count. This function should not
  * fail.
  *
@@ -962,7 +911,6 @@ long s5_inode_blocks(s5_node_t *sn)
     return num_file_blocks;
 }
 
-
 /**
  * Given a s5_node_t, frees the associated direct blocks and 
  * the indirect blocks if they exist.