[Cluster-devel] [PATCH 1/3] fsck.gfs2: Fix 'initializer element is not constant' build error

[Cluster-devel] [PATCH 1/3] fsck.gfs2: Fix 'initializer element is not constant' build error

[Andrew Price]

This error occurs when gfs2-utils is compiled with -std options more
recent than gnu89:

    CC       fsck_gfs2-main.o
  main.c:39:38: error: initializer element is not constant
   struct osi_root dup_blocks = (struct osi_root) { NULL, };
                                        ^
  main.c:40:35: error: initializer element is not constant
   struct osi_root dirtree = (struct osi_root) { NULL, };
                                     ^
  main.c:41:37: error: initializer element is not constant
   struct osi_root inodetree = (struct osi_root) { NULL, };
                                     ^
As far as I can tell, with C89/gnu89 the use of a cast in this context
is undefined behaviour and the later standards are more strict about it,
hence the error. As the standards specify that members of objects with
static storage duration are zeroed/NULLed anyway, the initializers can
be removed to achieve the intended result.

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

diff --git a/gfs2/fsck/main.c b/gfs2/fsck/main.c
index a4af25d..658cd17 100644
--- a/gfs2/fsck/main.c
+++ b/gfs2/fsck/main.c
@@ -36,9 +36,9 @@ const char *pass = "";
 uint64_t last_data_block;
 uint64_t first_data_block;
 int preen = 0, force_check = 0;
-struct osi_root dup_blocks = (struct osi_root) { NULL, };
-struct osi_root dirtree = (struct osi_root) { NULL, };
-struct osi_root inodetree = (struct osi_root) { NULL, };
+struct osi_root dup_blocks;
+struct osi_root dirtree;
+struct osi_root inodetree;
 int dups_found = 0, dups_found_first = 0;
 struct gfs_sb *sbd1 = NULL;
 int sb_fixed = 0;
-- 
1.9.3

[Cluster-devel] [PATCH 2/3] fsck.gfs2: Simplify bad_journalname

[Andrew Price]

Remove the need for a temporary string and strncpy call by passing the
length of the string to printf.

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

diff --git a/gfs2/fsck/initialize.c b/gfs2/fsck/initialize.c
index 1ab078b..c052205 100644
--- a/gfs2/fsck/initialize.c
+++ b/gfs2/fsck/initialize.c
@@ -1513,14 +1513,10 @@ static int init_rindex(struct gfs2_sbd *sdp)
 
 static void bad_journalname(const char *filename, int len)
 {
-	char tmp_name[64];
-
 	if (len >= 64)
 		len = 63;
-	strncpy(tmp_name, filename, len);
-	tmp_name[len] = '\0';
-	log_debug(_("Journal index entry '%s' has an invalid filename.\n"),
-		  tmp_name);
+	log_debug(_("Journal index entry '%.*s' has an invalid filename.\n"),
+	          len, filename);
 }
 
 /**
-- 
1.9.3

[Cluster-devel] [PATCH 2/3] fsck.gfs2: Simplify bad_journalname

[Bob Peterson]

----- Original Message -----
> Remove the need for a temporary string and strncpy call by passing the
> length of the string to printf.
> 
> Signed-off-by: Andrew Price <anprice@redhat.com>
> ---
>  gfs2/fsck/initialize.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/gfs2/fsck/initialize.c b/gfs2/fsck/initialize.c
> index 1ab078b..c052205 100644
> --- a/gfs2/fsck/initialize.c
> +++ b/gfs2/fsck/initialize.c
> @@ -1513,14 +1513,10 @@ static int init_rindex(struct gfs2_sbd *sdp)
>  
>  static void bad_journalname(const char *filename, int len)
>  {
> -	char tmp_name[64];
> -
>  	if (len >= 64)
>  		len = 63;
> -	strncpy(tmp_name, filename, len);
> -	tmp_name[len] = '\0';
> -	log_debug(_("Journal index entry '%s' has an invalid filename.\n"),
> -		  tmp_name);
> +	log_debug(_("Journal index entry '%.*s' has an invalid filename.\n"),
> +	          len, filename);
>  }
>  
>  /**
> --
> 1.9.3

Nice trick (strangely, I've never used that construct), but we could just as well get rid of the whole function altogether.

Bob Peterson
Red Hat File Systems

[Cluster-devel] [PATCH 2/3] fsck.gfs2: Simplify bad_journalname

[Andrew Price]

On 28/01/15 13:13, Bob Peterson wrote:
> ----- Original Message -----
>> Remove the need for a temporary string and strncpy call by passing the
>> length of the string to printf.
>>
>> Signed-off-by: Andrew Price <anprice@redhat.com>
>> ---
>>   gfs2/fsck/initialize.c | 8 ++------
>>   1 file changed, 2 insertions(+), 6 deletions(-)
>>
>> diff --git a/gfs2/fsck/initialize.c b/gfs2/fsck/initialize.c
>> index 1ab078b..c052205 100644
>> --- a/gfs2/fsck/initialize.c
>> +++ b/gfs2/fsck/initialize.c
>> @@ -1513,14 +1513,10 @@ static int init_rindex(struct gfs2_sbd *sdp)
>>
>>   static void bad_journalname(const char *filename, int len)
>>   {
>> -	char tmp_name[64];
>> -
>>   	if (len >= 64)
>>   		len = 63;
>> -	strncpy(tmp_name, filename, len);
>> -	tmp_name[len] = '\0';
>> -	log_debug(_("Journal index entry '%s' has an invalid filename.\n"),
>> -		  tmp_name);
>> +	log_debug(_("Journal index entry '%.*s' has an invalid filename.\n"),
>> +	          len, filename);
>>   }
>>
>>   /**
>> --
>> 1.9.3
>
> Nice trick (strangely, I've never used that construct), but we could just as well get rid of the whole function altogether.

I did consider removing it but, due to if-nesting and being called from 
two places, it makes things cleaner to leave it in a separate function. 
When fsck.gfs2 is fast enough that we need to worry about function call 
overhead then maybe we can inline it :)

Thanks,
Andy

[Cluster-devel] [PATCH 3/3] gfs2-utils build: Add a configure script summary

[Andrew Price]

Print a nicely formatted summary of some of the more interesting
configure options. Required some tweaking of earlier configure stages
for accuracy.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 configure.ac | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/configure.ac b/configure.ac
index a523be3..a41eb57 100644
--- a/configure.ac
+++ b/configure.ac
@@ -88,10 +88,10 @@ check_lib_no_libs() {
 # local options
 AC_ARG_ENABLE([debug],
 	AC_HELP_STRING([--enable-debug],[enable debug build [default=no]]),
-	[default="no"])
+	[], [enable_debug="no"])
 AC_ARG_ENABLE([gcov],
 	AC_HELP_STRING([--enable-gcov],[enable coverage instrumentation [default=no]]),
-	[default="no"])
+	[], [enable_gcov="no"])
 
 # We use the Check framework for unit tests
 PKG_CHECK_MODULES([check], [check >= 0.9.8],
@@ -175,6 +175,9 @@ if test "x${enable_gcov}" = xyes; then
 	if ! cc_supports_flag $GCOV_CFLAGS; then
 		AC_MSG_ERROR([your compiler does not support coverage instrumentation])
 	fi
+	if test "x${enable_debug}" = xyes; then
+		enable_debug="no (gcov enabled)"
+	fi
 	OPT_CFLAGS="-O0 $GCOV_CFLAGS"
 fi
 
@@ -206,10 +209,8 @@ for j in $WARNLIST; do
 	fi
 done
 
-CFLAGS="$ENV_CFLAGS $OPT_CFLAGS $GDB_FLAGS \
-	$EXTRA_WARNINGS $WERROR_CFLAGS"
-CPPFLAGS="-I\$(top_builddir)/make -I\$(top_srcdir)/make \
-	  -I. $ENV_CPPFLAGS"
+CFLAGS="$ENV_CFLAGS $OPT_CFLAGS $GDB_FLAGS $EXTRA_WARNINGS $WERROR_CFLAGS"
+CPPFLAGS="-I\$(top_builddir)/make -I\$(top_srcdir)/make -I. $ENV_CPPFLAGS"
 LDFLAGS="$ENV_LDFLAGS"
 
 AC_CONFIG_TESTDIR([tests], [gfs2/libgfs2:gfs2/mkfs:gfs2/fsck:gfs2/edit:gfs2/convert:gfs2/tune:tests])
@@ -232,9 +233,15 @@ AC_CONFIG_FILES([Makefile
 
 AC_OUTPUT
 
-if test x"${have_check}" != "xyes"; then
-	AC_MSG_NOTICE([package 'check' not found; unit tests will not be built])
-fi
-if test x"${enable_gcov}" = "xyes"; then
-	AC_MSG_NOTICE([code coverage enabled; optimization will be disabled])
-fi
+echo
+echo "  Configure summary"
+echo " ==================="
+echo " prefix            : $prefix"
+echo " exec_prefix       : $exec_prefix"
+echo " sbindir           : $sbindir"
+echo " ------------------"
+echo " debug build       : $enable_debug"
+echo " C unit tests      : $have_check"
+echo " gcov build        : $enable_gcov"
+echo
+echo "Now run 'make' to build and 'make check' to run tests"
-- 
1.9.3

[Cluster-devel] [PATCH 3/3] gfs2-utils build: Add a configure script summary

[Bob Peterson]

----- Original Message -----
> Print a nicely formatted summary of some of the more interesting
> configure options. Required some tweaking of earlier configure stages
> for accuracy.
> 
> Signed-off-by: Andrew Price <anprice@redhat.com>
> ---

Hi,

ACK

Bob Peterson
Red Hat File Systems

[Cluster-devel] [PATCH 1/3] fsck.gfs2: Fix 'initializer element is not constant' build error

[Bob Peterson]

----- Original Message -----
> This error occurs when gfs2-utils is compiled with -std options more
> recent than gnu89:
> 
>     CC       fsck_gfs2-main.o
>   main.c:39:38: error: initializer element is not constant
>    struct osi_root dup_blocks = (struct osi_root) { NULL, };
>                                         ^
>   main.c:40:35: error: initializer element is not constant
>    struct osi_root dirtree = (struct osi_root) { NULL, };
>                                      ^
>   main.c:41:37: error: initializer element is not constant
>    struct osi_root inodetree = (struct osi_root) { NULL, };
>                                      ^
> As far as I can tell, with C89/gnu89 the use of a cast in this context
> is undefined behaviour and the later standards are more strict about it,
> hence the error. As the standards specify that members of objects with
> static storage duration are zeroed/NULLed anyway, the initializers can
> be removed to achieve the intended result.
> 
> Signed-off-by: Andrew Price <anprice@redhat.com>
> ---

ACK

Bob Peterson
Red Hat File Systems