Clean up inode alias tracking

New helper functions:

	- d_associate()
	- d_disassociate()

i_nlink, d_inode, and i_dentry are no longer changed outside of these
functions.  We maintain the invariants:

	i_nlink == list_size(&i_dentry) == # of d_inode references

put_inode() is removed; free_inode() is now private to inode.c.

This fixes at least one bug: when freeing a dentry, it was not being
deleted from the inode's dentry list.

diff --git a/include/wimlib/inode.h b/include/wimlib/inode.h
index 4b9567757aa1ba2f8fd78ac0d1a25c4c9867678e..11c2361b81bf5da59dd6b2e71fe9d7c46c28f9b5 100644 (file)
--- a/include/wimlib/inode.h
+++ b/include/wimlib/inode.h
@@ -304,21 +304,10 @@ new_inode(void) _malloc_attribute;
 extern struct wim_inode *
 new_timeless_inode(void) _malloc_attribute;
 
 extern struct wim_inode *
 new_timeless_inode(void) _malloc_attribute;
 

-extern void
-put_inode(struct wim_inode *inode);
-
-extern void
-free_inode(struct wim_inode *inode);
-

 /* Iterate through each alias of the specified inode.  */
 #define inode_for_each_dentry(dentry, inode) \
        list_for_each_entry((dentry), &(inode)->i_dentry, d_alias)
 
 /* Iterate through each alias of the specified inode.  */
 #define inode_for_each_dentry(dentry, inode) \
        list_for_each_entry((dentry), &(inode)->i_dentry, d_alias)
 

-/* Add a new alias for the specified inode.  Does not increment i_nlink; that
- * must be done separately if needed.  */
-#define inode_add_dentry(dentry, inode) \
-       list_add_tail(&(dentry)->d_alias, &(inode)->i_dentry)
-

 /* Return an alias of the specified inode.  */
 #define inode_first_dentry(inode) \
        container_of(inode->i_dentry.next, struct wim_dentry, d_alias)
 /* Return an alias of the specified inode.  */
 #define inode_first_dentry(inode) \
        container_of(inode->i_dentry.next, struct wim_dentry, d_alias)


@@ -328,6 +317,17 @@ free_inode(struct wim_inode *inode);
 #define inode_first_full_path(inode) \
        dentry_full_path(inode_first_dentry(inode))
 
 #define inode_first_full_path(inode) \
        dentry_full_path(inode_first_dentry(inode))
 

+extern void
+d_associate(struct wim_dentry *dentry, struct wim_inode *inode);
+
+extern void
+d_disassociate(struct wim_dentry *dentry);
+
+#ifdef WITH_FUSE
+extern void
+inode_dec_num_opened_fds(struct wim_inode *inode);
+#endif
+

 /* Is the inode a directory?
  * This doesn't count directories with reparse data.
  * wimlib only allows inodes of this type to have children.
 /* Is the inode a directory?
  * This doesn't count directories with reparse data.
  * wimlib only allows inodes of this type to have children.

diff --git a/src/dentry.c b/src/dentry.c
index 862c6640c9bb54b9bef52610824604091b830e13..38b4efa60291db98cae4620e268153b1e2b8ec95 100644 (file)
--- a/src/dentry.c
+++ b/src/dentry.c
@@ -925,6 +925,7 @@ _new_dentry_with_inode(const tchar *name, struct wim_dentry **dentry_ret,
                       bool timeless)
 {
        struct wim_dentry *dentry;
                       bool timeless)
 {
        struct wim_dentry *dentry;

+       struct wim_inode *inode;

        int ret;
 
        ret = new_dentry(name, &dentry);
        int ret;
 
        ret = new_dentry(name, &dentry);


@@ -932,15 +933,16 @@ _new_dentry_with_inode(const tchar *name, struct wim_dentry **dentry_ret,
                return ret;
 
        if (timeless)
                return ret;
 
        if (timeless)

-               dentry->d_inode = new_timeless_inode();
+               inode = new_timeless_inode();

        else
        else

-               dentry->d_inode = new_inode();
-       if (dentry->d_inode == NULL) {
+               inode = new_inode();
+       if (!inode) {

                free_dentry(dentry);
                return WIMLIB_ERR_NOMEM;
        }
 
                free_dentry(dentry);
                return WIMLIB_ERR_NOMEM;
        }
 

-       inode_add_dentry(dentry, dentry->d_inode);
+       d_associate(dentry, inode);
+

        *dentry_ret = dentry;
        return 0;
 }
        *dentry_ret = dentry;
        return 0;
 }


@@ -997,19 +999,17 @@ dentry_tree_clear_inode_visited(struct wim_dentry *root)
 /*
  * Free a WIM dentry.
  *
 /*
  * Free a WIM dentry.
  *

- * In addition to freeing the dentry itself, this decrements the link count of
- * the corresponding inode (if any).  If the inode's link count reaches 0, the
- * inode is freed as well.
+ * In addition to freeing the dentry itself, this disassociates the dentry from
+ * its inode.  If the inode is no longer in use, it will be freed as well.

  */
 void
 free_dentry(struct wim_dentry *dentry)
 {
        if (dentry) {
  */
 void
 free_dentry(struct wim_dentry *dentry)
 {
        if (dentry) {

+               d_disassociate(dentry);

                FREE(dentry->file_name);
                FREE(dentry->short_name);
                FREE(dentry->_full_path);
                FREE(dentry->file_name);
                FREE(dentry->short_name);
                FREE(dentry->_full_path);

-               if (dentry->d_inode)
-                       put_inode(dentry->d_inode);

                FREE(dentry);
        }
 }
                FREE(dentry);
        }
 }

diff --git a/src/inode.c b/src/inode.c
index 9e4177c9ce57e46bd94151b42d9be592afc2b6e3..6384b11c408d4ccb9212cccbf3c10053ea475d30 100644 (file)
--- a/src/inode.c
+++ b/src/inode.c
@@ -61,7 +61,7 @@ new_timeless_inode(void)
        struct wim_inode *inode = CALLOC(1, sizeof(struct wim_inode));
        if (inode) {
                inode->i_security_id = -1;
        struct wim_inode *inode = CALLOC(1, sizeof(struct wim_inode));
        if (inode) {
                inode->i_security_id = -1;

-               inode->i_nlink = 1;
+               /*inode->i_nlink = 0;*/

                inode->i_next_stream_id = 1;
                inode->i_not_rpfixed = 1;
                inode->i_canonical_streams = 1;
                inode->i_next_stream_id = 1;
                inode->i_not_rpfixed = 1;
                inode->i_canonical_streams = 1;


@@ -71,21 +71,6 @@ new_timeless_inode(void)
        return inode;
 }
 
        return inode;
 }
 

-/* Decrement an inode's link count.  */
-void
-put_inode(struct wim_inode *inode)
-{
-       wimlib_assert(inode->i_nlink != 0);
-       if (--inode->i_nlink == 0) {
-               /* If FUSE mounts are enabled, we must keep a unlinked inode
-                * around until all file descriptors to it have been closed.  */
-       #ifdef WITH_FUSE
-               if (inode->i_num_opened_fds == 0)
-       #endif
-                       free_inode(inode);
-       }
-}
-

 /* Free memory allocated within an alternate data stream entry.  */
 static void
 destroy_ads_entry(struct wim_ads_entry *ads_entry)
 /* Free memory allocated within an alternate data stream entry.  */
 static void
 destroy_ads_entry(struct wim_ads_entry *ads_entry)


@@ -93,14 +78,9 @@ destroy_ads_entry(struct wim_ads_entry *ads_entry)
        FREE(ads_entry->stream_name);
 }
 
        FREE(ads_entry->stream_name);
 }
 

-/* Free an inode.  Only use this if there can't be other links to the inode or
- * if it doesn't matter if there are.  */
-void
+static void

 free_inode(struct wim_inode *inode)
 {
 free_inode(struct wim_inode *inode)
 {

-       if (unlikely(!inode))
-               return;
-

        if (unlikely(inode->i_ads_entries)) {
                for (unsigned i = 0; i < inode->i_num_ads; i++)
                        destroy_ads_entry(&inode->i_ads_entries[i]);
        if (unlikely(inode->i_ads_entries)) {
                for (unsigned i = 0; i < inode->i_num_ads; i++)
                        destroy_ads_entry(&inode->i_ads_entries[i]);


@@ -115,6 +95,65 @@ free_inode(struct wim_inode *inode)
        FREE(inode);
 }
 
        FREE(inode);
 }
 

+static inline void
+free_inode_if_unneeded(struct wim_inode *inode)
+{
+       if (inode->i_nlink)
+               return;
+#ifdef WITH_FUSE
+       if (inode->i_num_opened_fds)
+               return;
+#endif
+       free_inode(inode);
+}
+
+/* Associate a dentry with the specified inode.  */
+void
+d_associate(struct wim_dentry *dentry, struct wim_inode *inode)
+{
+       wimlib_assert(!dentry->d_inode);
+
+       list_add_tail(&dentry->d_alias, &inode->i_dentry);
+       dentry->d_inode = inode;
+       inode->i_nlink++;
+}
+
+/* Disassociate a dentry from its inode, if any.  Following this, free the inode
+ * if it is no longer in use.  */
+void
+d_disassociate(struct wim_dentry *dentry)
+{
+       struct wim_inode *inode = dentry->d_inode;
+
+       if (unlikely(!inode))
+               return;
+
+       wimlib_assert(inode->i_nlink > 0);
+
+       list_del(&dentry->d_alias);
+       dentry->d_inode = NULL;
+       inode->i_nlink--;
+
+       free_inode_if_unneeded(inode);
+}
+
+#ifdef WITH_FUSE
+void
+inode_dec_num_opened_fds(struct wim_inode *inode)
+{
+       wimlib_assert(inode->i_num_opened_fds > 0);
+
+       if (--inode->i_num_opened_fds == 0) {
+               /* The last file descriptor to this inode was closed.  */
+               FREE(inode->i_fds);
+               inode->i_fds = NULL;
+               inode->i_num_allocated_fds = 0;
+
+               free_inode_if_unneeded(inode);
+       }
+}
+#endif
+

 /*
  * Returns the alternate data stream entry belonging to @inode that has the
  * stream name @stream_name, or NULL if the inode has no alternate data stream
 /*
  * Returns the alternate data stream entry belonging to @inode that has the
  * stream name @stream_name, or NULL if the inode has no alternate data stream

diff --git a/src/inode_fixup.c b/src/inode_fixup.c
index 616c5ff39eaee5dcabdd987e4c1ba185da1c7f92..7624562371ea97b1d3557ff5e25cc5cafd2c5ce0 100644 (file)
--- a/src/inode_fixup.c
+++ b/src/inode_fixup.c
@@ -102,10 +102,8 @@ inode_table_insert(struct wim_dentry *dentry, void *_params)
                        continue;
                }
                /* Transfer this dentry to the existing inode.  */
                        continue;
                }
                /* Transfer this dentry to the existing inode.  */

-               free_inode(d_inode);
-               dentry->d_inode = inode;
-               inode->i_nlink++;
-               inode_add_dentry(dentry, inode);
+               d_disassociate(dentry);
+               d_associate(dentry, inode);

                return 0;
        }
 
                return 0;
        }
 

diff --git a/src/inode_table.c b/src/inode_table.c
index 6e1c143b3c33336d4767627b98aff8893b717fba..49dd821776d3ba0815843d823fb6638ccfa71bda 100644 (file)
--- a/src/inode_table.c
+++ b/src/inode_table.c
@@ -50,35 +50,6 @@ destroy_inode_table(struct wim_inode_table *table)
        FREE(table->array);
 }
 
        FREE(table->array);
 }
 

-static struct wim_inode *
-inode_table_get_inode(struct wim_inode_table *table, u64 ino, u64 devno)
-{
-       size_t pos;
-       struct wim_inode *inode;
-       struct hlist_node *cur;
-
-       /* Search for an existing inode having the same inode number and device
-        * number.  */
-       pos = hash_u64(hash_u64(ino) + hash_u64(devno)) % table->capacity;
-       hlist_for_each_entry(inode, cur, &table->array[pos], i_hlist) {
-               if (inode->i_ino == ino && inode->i_devno == devno) {
-                       /* Found; use the existing inode.  */
-                       inode->i_nlink++;
-                       return inode;
-               }
-       }
-
-       /* Create a new inode and insert it into the table.  */
-       inode = new_timeless_inode();
-       if (inode) {
-               inode->i_ino = ino;
-               inode->i_devno = devno;
-               hlist_add_head(&inode->i_hlist, &table->array[pos]);
-               table->num_entries++;
-       }
-       return inode;
-}
-

 /*
  * Allocate a new dentry, with hard link detection.
  *
 /*
  * Allocate a new dentry, with hard link detection.
  *


@@ -130,23 +101,39 @@ inode_table_new_dentry(struct wim_inode_table *table, const tchar *name,
                        return ret;
                list_add_tail(&dentry->d_inode->i_list, &table->extra_inodes);
        } else {
                        return ret;
                list_add_tail(&dentry->d_inode->i_list, &table->extra_inodes);
        } else {

+               size_t pos;
+               struct hlist_node *cur;
+

                /* File that can be hardlinked--- search the table for an
                 * existing inode matching the inode number and device;
                 * otherwise create a new inode. */
                ret = new_dentry(name, &dentry);
                if (ret)
                        return ret;
                /* File that can be hardlinked--- search the table for an
                 * existing inode matching the inode number and device;
                 * otherwise create a new inode. */
                ret = new_dentry(name, &dentry);
                if (ret)
                        return ret;

-               inode = inode_table_get_inode(table, ino, devno);
+
+               /* Search for an existing inode having the same inode number and
+                * device number.  */
+               pos = hash_u64(hash_u64(ino) + hash_u64(devno)) % table->capacity;
+               hlist_for_each_entry(inode, cur, &table->array[pos], i_hlist) {
+                       if (inode->i_ino == ino && inode->i_devno == devno) {
+                               /* Found; use the existing inode.  */
+                               inode_ref_streams(inode);
+                               goto have_inode;
+                       }
+               }
+
+               /* Create a new inode and insert it into the table.  */
+               inode = new_timeless_inode();

                if (!inode) {
                        free_dentry(dentry);
                        return WIMLIB_ERR_NOMEM;
                }
                if (!inode) {
                        free_dentry(dentry);
                        return WIMLIB_ERR_NOMEM;
                }

-               /* If using an existing inode, we need to gain a reference to
-                * each of its streams. */
-               if (inode->i_nlink > 1)
-                       inode_ref_streams(inode);
-               dentry->d_inode = inode;
-               inode_add_dentry(dentry, inode);
+               inode->i_ino = ino;
+               inode->i_devno = devno;
+               hlist_add_head(&inode->i_hlist, &table->array[pos]);
+               table->num_entries++;
+       have_inode:
+               d_associate(dentry, inode);

        }
        *dentry_ret = dentry;
        return 0;
        }
        *dentry_ret = dentry;
        return 0;

diff --git a/src/mount_image.c b/src/mount_image.c
index 0bd4a1f1d76d3463aa642ce2581f8b780f38e1c2..0672c9c223834bf56c3f58ae4716586091be6671 100644 (file)
--- a/src/mount_image.c
+++ b/src/mount_image.c
@@ -324,15 +324,7 @@ close_wimfs_fd(struct wimfs_fd *fd)
        if (fd->f_idx < inode->i_next_fd)
                inode->i_next_fd = fd->f_idx;
        FREE(fd);
        if (fd->f_idx < inode->i_next_fd)
                inode->i_next_fd = fd->f_idx;
        FREE(fd);

-       if (--inode->i_num_opened_fds == 0) {
-               /* The last file descriptor to this inode was closed.  */
-               FREE(inode->i_fds);
-               inode->i_fds = NULL;
-               inode->i_num_allocated_fds = 0;
-               if (inode->i_nlink == 0)
-                       /* No links to this inode remain.  Get rid of it.  */
-                       free_inode(inode);
-       }
+       inode_dec_num_opened_fds(inode);

        return ret;
 }
 
        return ret;
 }
 


@@ -1439,12 +1431,10 @@ wimfs_link(const char *existing_path, const char *new_path)
        if (new_dentry(new_name, &new_alias))
                return -ENOMEM;
 
        if (new_dentry(new_name, &new_alias))
                return -ENOMEM;
 

-       new_alias->d_inode = inode;
-       inode_add_dentry(new_alias, inode);
+       inode_ref_streams(inode);
+       d_associate(new_alias, inode);

        dentry_add_child(dir, new_alias);
        touch_inode(dir->d_inode);
        dentry_add_child(dir, new_alias);
        touch_inode(dir->d_inode);

-       inode->i_nlink++;
-       inode_ref_streams(inode);

        return 0;
 }
 
        return 0;
 }