Re: [Cluster-devel] [Patch 09/44] fsck.gfs2: Rename the nlink functions to make them more intuitive

2011-08-12 Thread Steven Whitehouse 
Hi,

Why is the link count only 16 bits? Where is it checked for overflow?

Steve.

On Thu, 2011-08-11 at 17:01 -0400, Bob Peterson wrote:
 From 55e442c79adec0fa7f6d4e7f6700f14a630d4e3e Mon Sep 17 00:00:00 2001
 From: Bob Peterson rpete...@redhat.com
 Date: Mon, 8 Aug 2011 14:01:18 -0500
 Subject: [PATCH 09/44] fsck.gfs2: Rename the nlink functions to make them
  more intuitive
 
 Part of fsck's checks is to verify the count of links for directories,
 but the variable names and function names are too confusing to understand
 without in-depth analysis of what the code is doing.
 
 This patch renames the structure variable link_count to something that
 makes more intuitive sense: di_nlink, which matches the variable name in
 the dinode.  That distinguishes it from the number of links that fsck is
 trying to count manually.  It also renames the link count functions to
 make them more intuitively obvious as well: set_di_nlink sets the di_nlink
 based on the value passed in from the dinode, and incr_link_count and
 decr_link_count increment and decrement the counted links respectively.
 
 rhbz#675723
 ---
  gfs2/fsck/fsck.h |4 ++--
  gfs2/fsck/link.c |   16 ++--
  gfs2/fsck/link.h |   10 +-
  gfs2/fsck/lost_n_found.c |   29 ++---
  gfs2/fsck/pass1.c|2 +-
  gfs2/fsck/pass2.c|   20 ++--
  gfs2/fsck/pass3.c|4 ++--
  gfs2/fsck/pass4.c|   14 +++---
  8 files changed, 51 insertions(+), 48 deletions(-)
 
 diff --git a/gfs2/fsck/fsck.h b/gfs2/fsck/fsck.h
 index 25bc3b9..6353dfc 100644
 --- a/gfs2/fsck/fsck.h
 +++ b/gfs2/fsck/fsck.h
 @@ -29,8 +29,8 @@ struct inode_info
  {
  struct osi_node node;
  uint64_t   inode;
 -uint16_t   link_count;   /* the number of links the inode
 -  * thinks it has */
 +uint16_t   di_nlink;   /* the number of links the inode
 + * thinks it has */
  uint16_t   counted_links; /* the number of links we've found */
  };
  
 diff --git a/gfs2/fsck/link.c b/gfs2/fsck/link.c
 index 08ea94c..e49f3af 100644
 --- a/gfs2/fsck/link.c
 +++ b/gfs2/fsck/link.c
 @@ -13,22 +13,26 @@
  #include inode_hash.h
  #include link.h
  
 -int set_link_count(uint64_t inode_no, uint32_t count)
 +int set_di_nlink(struct gfs2_inode *ip)
  {
   struct inode_info *ii;
 + uint64_t inode_no = ip-i_di.di_num.no_addr;
 +
 + /*log_debug( _(Setting link count to %u for % PRIu64
 +(0x% PRIx64 )\n), count, inode_no, inode_no);*/
   /* If the list has entries, look for one that matches inode_no */
   ii = inodetree_find(inode_no);
   if (!ii)
   ii = inodetree_insert(inode_no);
   if (ii)
 - ii-link_count = count;
 + ii-di_nlink = ip-i_di.di_nlink;
   else
   return -1;
   return 0;
  }
  
 -int increment_link(uint64_t inode_no, uint64_t referenced_from,
 -const char *why)
 +int incr_link_count(uint64_t inode_no, uint64_t referenced_from,
 + const char *why)
  {
   struct inode_info *ii = NULL;
  
 @@ -61,8 +65,8 @@ int increment_link(uint64_t inode_no, uint64_t 
 referenced_from,
   return 0;
  }
  
 -int decrement_link(uint64_t inode_no, uint64_t referenced_from,
 -const char *why)
 +int decr_link_count(uint64_t inode_no, uint64_t referenced_from,
 + const char *why)
  {
   struct inode_info *ii = NULL;
  
 diff --git a/gfs2/fsck/link.h b/gfs2/fsck/link.h
 index f890575..ad040e6 100644
 --- a/gfs2/fsck/link.h
 +++ b/gfs2/fsck/link.h
 @@ -1,10 +1,10 @@
  #ifndef _LINK_H
  #define _LINK_H
  
 -int set_link_count(uint64_t inode_no, uint32_t count);
 -int increment_link(uint64_t inode_no, uint64_t referenced_from,
 -const char *why);
 -int decrement_link(uint64_t inode_no, uint64_t referenced_from,
 -const char *why);
 +int set_di_nlink(struct gfs2_inode *ip);
 +int incr_link_count(uint64_t inode_no, uint64_t referenced_from,
 + const char *why);
 +int decr_link_count(uint64_t inode_no, uint64_t referenced_from,
 + const char *why);
  
  #endif /* _LINK_H */
 diff --git a/gfs2/fsck/lost_n_found.c b/gfs2/fsck/lost_n_found.c
 index 4eff83b..32f3c5c 100644
 --- a/gfs2/fsck/lost_n_found.c
 +++ b/gfs2/fsck/lost_n_found.c
 @@ -51,8 +51,7 @@ int add_inode_to_lf(struct gfs2_inode *ip){
  the root directory.  We must increment the nlink value
  in the hash table to keep them in sync so that pass4 can
  detect and fix any descrepancies. */
 - set_link_count(sdp-sd_sb.sb_root_dir.no_addr,
 -sdp-md.rooti-i_di.di_nlink);
 + set_di_nlink(sdp-md.rooti);
  
   q = block_type(lf_dip-i_di.di_num.no_addr);
   if (q != gfs2_inode_dir) {
 @@ -68,15 +67,15 @@ int

[Cluster-devel] [Patch 09/44] fsck.gfs2: Rename the nlink functions to make them more intuitive

2011-08-11 Thread Bob Peterson 
From 55e442c79adec0fa7f6d4e7f6700f14a630d4e3e Mon Sep 17 00:00:00 2001
From: Bob Peterson rpete...@redhat.com
Date: Mon, 8 Aug 2011 14:01:18 -0500
Subject: [PATCH 09/44] fsck.gfs2: Rename the nlink functions to make them
 more intuitive

Part of fsck's checks is to verify the count of links for directories,
but the variable names and function names are too confusing to understand
without in-depth analysis of what the code is doing.

This patch renames the structure variable link_count to something that
makes more intuitive sense: di_nlink, which matches the variable name in
the dinode.  That distinguishes it from the number of links that fsck is
trying to count manually.  It also renames the link count functions to
make them more intuitively obvious as well: set_di_nlink sets the di_nlink
based on the value passed in from the dinode, and incr_link_count and
decr_link_count increment and decrement the counted links respectively.

rhbz#675723
---
 gfs2/fsck/fsck.h |4 ++--
 gfs2/fsck/link.c |   16 ++--
 gfs2/fsck/link.h |   10 +-
 gfs2/fsck/lost_n_found.c |   29 ++---
 gfs2/fsck/pass1.c|2 +-
 gfs2/fsck/pass2.c|   20 ++--
 gfs2/fsck/pass3.c|4 ++--
 gfs2/fsck/pass4.c|   14 +++---
 8 files changed, 51 insertions(+), 48 deletions(-)

diff --git a/gfs2/fsck/fsck.h b/gfs2/fsck/fsck.h
index 25bc3b9..6353dfc 100644
--- a/gfs2/fsck/fsck.h
+++ b/gfs2/fsck/fsck.h
@@ -29,8 +29,8 @@ struct inode_info
 {
 struct osi_node node;
 uint64_t   inode;
-uint16_t   link_count;   /* the number of links the inode
-  * thinks it has */
+uint16_t   di_nlink;   /* the number of links the inode
+   * thinks it has */
 uint16_t   counted_links; /* the number of links we've found */
 };
 
diff --git a/gfs2/fsck/link.c b/gfs2/fsck/link.c
index 08ea94c..e49f3af 100644
--- a/gfs2/fsck/link.c
+++ b/gfs2/fsck/link.c
@@ -13,22 +13,26 @@
 #include inode_hash.h
 #include link.h
 
-int set_link_count(uint64_t inode_no, uint32_t count)
+int set_di_nlink(struct gfs2_inode *ip)
 {
struct inode_info *ii;
+   uint64_t inode_no = ip-i_di.di_num.no_addr;
+
+   /*log_debug( _(Setting link count to %u for % PRIu64
+  (0x% PRIx64 )\n), count, inode_no, inode_no);*/
/* If the list has entries, look for one that matches inode_no */
ii = inodetree_find(inode_no);
if (!ii)
ii = inodetree_insert(inode_no);
if (ii)
-   ii-link_count = count;
+   ii-di_nlink = ip-i_di.di_nlink;
else
return -1;
return 0;
 }
 
-int increment_link(uint64_t inode_no, uint64_t referenced_from,
-  const char *why)
+int incr_link_count(uint64_t inode_no, uint64_t referenced_from,
+   const char *why)
 {
struct inode_info *ii = NULL;
 
@@ -61,8 +65,8 @@ int increment_link(uint64_t inode_no, uint64_t 
referenced_from,
return 0;
 }
 
-int decrement_link(uint64_t inode_no, uint64_t referenced_from,
-  const char *why)
+int decr_link_count(uint64_t inode_no, uint64_t referenced_from,
+   const char *why)
 {
struct inode_info *ii = NULL;
 
diff --git a/gfs2/fsck/link.h b/gfs2/fsck/link.h
index f890575..ad040e6 100644
--- a/gfs2/fsck/link.h
+++ b/gfs2/fsck/link.h
@@ -1,10 +1,10 @@
 #ifndef _LINK_H
 #define _LINK_H
 
-int set_link_count(uint64_t inode_no, uint32_t count);
-int increment_link(uint64_t inode_no, uint64_t referenced_from,
-  const char *why);
-int decrement_link(uint64_t inode_no, uint64_t referenced_from,
-  const char *why);
+int set_di_nlink(struct gfs2_inode *ip);
+int incr_link_count(uint64_t inode_no, uint64_t referenced_from,
+   const char *why);
+int decr_link_count(uint64_t inode_no, uint64_t referenced_from,
+   const char *why);
 
 #endif /* _LINK_H */
diff --git a/gfs2/fsck/lost_n_found.c b/gfs2/fsck/lost_n_found.c
index 4eff83b..32f3c5c 100644
--- a/gfs2/fsck/lost_n_found.c
+++ b/gfs2/fsck/lost_n_found.c
@@ -51,8 +51,7 @@ int add_inode_to_lf(struct gfs2_inode *ip){
   the root directory.  We must increment the nlink value
   in the hash table to keep them in sync so that pass4 can
   detect and fix any descrepancies. */
-   set_link_count(sdp-sd_sb.sb_root_dir.no_addr,
-  sdp-md.rooti-i_di.di_nlink);
+   set_di_nlink(sdp-md.rooti);
 
q = block_type(lf_dip-i_di.di_num.no_addr);
if (q != gfs2_inode_dir) {
@@ -68,15 +67,15 @@ int add_inode_to_lf(struct gfs2_inode *ip){
  _(lost+found dinode),
  gfs2_inode_dir);
/* root inode links