[Cluster-devel] [PATCH 1/2] gfs2_convert: Fix misleading indentation warning

[Cluster-devel] [PATCH 1/2] gfs2_convert: Fix misleading indentation warning

[Andrew Price]

Caught by the new -Wmisleading-indentation warning in gcc 6:

    CC       gfs2_convert-gfs2_convert.o
  gfs2_convert.c: In function 'inode_renumber':
  gfs2_convert.c:1073:5: warning: this 'if' clause does not guard...
       if (block != rindex_addr && block != jindex_addr)
       ^~
  gfs2_convert.c:1075:6: note: ...this statement, but the latter is
  misleadingly indented as if it is guarded by the 'if'
        if (error) {
        ^~

Fortunately, this wasn't actually a bug, it just caused the value of
'error' to be checked when it couldn't possibly be non-zero as well as
the one time it could be.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 gfs2/convert/gfs2_convert.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gfs2/convert/gfs2_convert.c b/gfs2/convert/gfs2_convert.c
index aac4853..9845049 100644
--- a/gfs2/convert/gfs2_convert.c
+++ b/gfs2/convert/gfs2_convert.c
@@ -1070,11 +1070,11 @@ static int inode_renumber(struct gfs2_sbd *sbp, uint64_t root_inode_addr, osi_li
 			bh = bread(sbp, block);
 			if (!gfs2_check_meta(bh, GFS_METATYPE_DI)) {/* if it is an dinode */
 				/* Skip the rindex and jindex inodes for now. */
-				if (block != rindex_addr && block != jindex_addr)
+				if (block != rindex_addr && block != jindex_addr) {
 					error = adjust_inode(sbp, bh);
-					if (error) {
+					if (error)
 						return error;
-					}
+				}
 			} else { /* It's metadata, but not an inode, so fix the bitmap. */
 				int blk, buf_offset;
 				int bitmap_byte; /* byte within the bitmap to fix */
-- 
2.7.4

[Cluster-devel] [PATCH 2/2] gfs2-utils: Build with -D_FORTIFY_SOURCE=2

[Andrew Price]

A survey of popular Linux distros shows that they all compile their
packages with _FORTIFY_SOURCE=2 defined so it makes sense to enable it
upstream too, in order to fix any issues it highlights before they reach
the distros. It will also allow us to factor the overhead it adds into
perf measurements.

Briefly, using this flag adds buffer overflow detection to some
functions, and enables -Wunused-result warnings.

It requires the optimization level to be -O1 or higher so it is only
added to CPPFLAGS when --enable-debug is not used.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 configure.ac | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index b421fbe..cc49642 100644
--- a/configure.ac
+++ b/configure.ac
@@ -143,8 +143,10 @@ ENV_LDFLAGS="$LDFLAGS"
 if test "x${enable_debug}" = xyes; then
 	AC_DEFINE_UNQUOTED([DEBUG], [1], [Compiling Debugging code])
 	OPT_CFLAGS="-O0"
+	OPT_CPPFLAGS=""
 else
 	OPT_CFLAGS="-O2"
+	OPT_CPPFLAGS="-D_FORTIFY_SOURCE=2"
 fi
 
 # gdb flags
@@ -203,7 +205,7 @@ for j in $WARNLIST; do
 done
 
 CFLAGS="$ENV_CFLAGS $OPT_CFLAGS $GDB_FLAGS $EXTRA_WARNINGS $WERROR_CFLAGS"
-CPPFLAGS="-I\$(top_builddir)/make -I\$(top_srcdir)/make -I. $ENV_CPPFLAGS"
+CPPFLAGS="-I\$(top_builddir)/make -I\$(top_srcdir)/make -I. $ENV_CPPFLAGS $OPT_CPPFLAGS"
 LDFLAGS="$ENV_LDFLAGS"
 
 AC_CONFIG_TESTDIR([tests], [gfs2/libgfs2:gfs2/mkfs:gfs2/fsck:gfs2/edit:gfs2/convert:gfs2/tune:tests])
-- 
2.7.4

[Cluster-devel] [PATCH 1/2] gfs2_convert: Fix misleading indentation warning

[Bob Peterson]

----- Original Message -----
| Caught by the new -Wmisleading-indentation warning in gcc 6:
| 
|     CC       gfs2_convert-gfs2_convert.o
|   gfs2_convert.c: In function 'inode_renumber':
|   gfs2_convert.c:1073:5: warning: this 'if' clause does not guard...
|        if (block != rindex_addr && block != jindex_addr)
|        ^~
|   gfs2_convert.c:1075:6: note: ...this statement, but the latter is
|   misleadingly indented as if it is guarded by the 'if'
|         if (error) {
|         ^~
| 
| Fortunately, this wasn't actually a bug, it just caused the value of
| 'error' to be checked when it couldn't possibly be non-zero as well as
| the one time it could be.
| 
| Signed-off-by: Andrew Price <anprice@redhat.com>
| ---
|  gfs2/convert/gfs2_convert.c | 6 +++---
|  1 file changed, 3 insertions(+), 3 deletions(-)
| 
| diff --git a/gfs2/convert/gfs2_convert.c b/gfs2/convert/gfs2_convert.c
| index aac4853..9845049 100644
| --- a/gfs2/convert/gfs2_convert.c
| +++ b/gfs2/convert/gfs2_convert.c
| @@ -1070,11 +1070,11 @@ static int inode_renumber(struct gfs2_sbd *sbp,
| uint64_t root_inode_addr, osi_li
|  			bh = bread(sbp, block);
|  			if (!gfs2_check_meta(bh, GFS_METATYPE_DI)) {/* if it is an dinode */
|  				/* Skip the rindex and jindex inodes for now. */
| -				if (block != rindex_addr && block != jindex_addr)
| +				if (block != rindex_addr && block != jindex_addr) {
|  					error = adjust_inode(sbp, bh);
| -					if (error) {
| +					if (error)
|  						return error;
| -					}
| +				}
|  			} else { /* It's metadata, but not an inode, so fix the bitmap. */
|  				int blk, buf_offset;
|  				int bitmap_byte; /* byte within the bitmap to fix */
| --
| 2.7.4
| 
| 
Hi,

ACK

Bob Peterson
Red Hat File Systems