[PATCH] xfsprogs: do not redeclare globals provided by libraries

[PATCH] xfsprogs: do not redeclare globals provided by libraries

[Eric Sandeen]

From: Eric Sandeen <sandeen@redhat.com>

In each of these cases, db, logprint, and mdrestore are redeclaring
as a global variable something which was already provided by a
library they link with.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/db/init.c b/db/init.c
index 455220a..0ac3736 100644
--- a/db/init.c
+++ b/db/init.c
@@ -27,7 +27,6 @@ static int		force;
  static struct xfs_mount	xmount;
  struct xfs_mount	*mp;
  static struct xlog	xlog;
-libxfs_init_t		x;
  xfs_agnumber_t		cur_agno = NULLAGNUMBER;

  static void
diff --git a/logprint/logprint.c b/logprint/logprint.c
index 7754a2a..511a32a 100644
--- a/logprint/logprint.c
+++ b/logprint/logprint.c
@@ -24,7 +24,6 @@ int	print_buffer;
  int	print_overwrite;
  int     print_no_data;
  int     print_no_print;
-int     print_exit = 1; /* -e is now default. specify -c to override */
  static int	print_operation = OP_PRINT;

  static void
@@ -132,6 +131,7 @@ main(int argc, char **argv)
  	bindtextdomain(PACKAGE, LOCALEDIR);
  	textdomain(PACKAGE);
  	memset(&mount, 0, sizeof(mount));
+	print_exit = 1; /* -e is now default. specify -c to override */

  	progname = basename(argv[0]);
  	while ((c = getopt(argc, argv, "bC:cdefl:iqnors:tDVv")) != EOF) {
@@ -152,7 +152,7 @@ main(int argc, char **argv)
  			case 'e':
  			    /* -e is now default
  			     */
-				print_exit++;
+				print_exit = 1;
  				break;
  			case 'C':
  				print_operation = OP_COPY;
diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c
index 3375e08..1cd399d 100644
--- a/mdrestore/xfs_mdrestore.c
+++ b/mdrestore/xfs_mdrestore.c
@@ -7,7 +7,6 @@
  #include "libxfs.h"
  #include "xfs_metadump.h"

-char 		*progname;
  static int	show_progress = 0;
  static int	show_info = 0;
  static int	progress_since_warning = 0;

Re: [PATCH] xfsprogs: do not redeclare globals provided by libraries

[Darrick J. Wong]

On Mon, Jan 27, 2020 at 04:56:02PM -0600, Eric Sandeen wrote:
> From: Eric Sandeen <sandeen@redhat.com>
> 
> In each of these cases, db, logprint, and mdrestore are redeclaring
> as a global variable something which was already provided by a
> library they link with.

Er... which library?

Also, uh...maybe we shouldn't be exporting globals across libraries?

(He says having not looked for how many there are lurki... ye gods)

--D

> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> diff --git a/db/init.c b/db/init.c
> index 455220a..0ac3736 100644
> --- a/db/init.c
> +++ b/db/init.c
> @@ -27,7 +27,6 @@ static int		force;
>  static struct xfs_mount	xmount;
>  struct xfs_mount	*mp;
>  static struct xlog	xlog;
> -libxfs_init_t		x;
>  xfs_agnumber_t		cur_agno = NULLAGNUMBER;
> 
>  static void
> diff --git a/logprint/logprint.c b/logprint/logprint.c
> index 7754a2a..511a32a 100644
> --- a/logprint/logprint.c
> +++ b/logprint/logprint.c
> @@ -24,7 +24,6 @@ int	print_buffer;
>  int	print_overwrite;
>  int     print_no_data;
>  int     print_no_print;
> -int     print_exit = 1; /* -e is now default. specify -c to override */
>  static int	print_operation = OP_PRINT;
> 
>  static void
> @@ -132,6 +131,7 @@ main(int argc, char **argv)
>  	bindtextdomain(PACKAGE, LOCALEDIR);
>  	textdomain(PACKAGE);
>  	memset(&mount, 0, sizeof(mount));
> +	print_exit = 1; /* -e is now default. specify -c to override */
> 
>  	progname = basename(argv[0]);
>  	while ((c = getopt(argc, argv, "bC:cdefl:iqnors:tDVv")) != EOF) {
> @@ -152,7 +152,7 @@ main(int argc, char **argv)
>  			case 'e':
>  			    /* -e is now default
>  			     */
> -				print_exit++;
> +				print_exit = 1;
>  				break;
>  			case 'C':
>  				print_operation = OP_COPY;
> diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c
> index 3375e08..1cd399d 100644
> --- a/mdrestore/xfs_mdrestore.c
> +++ b/mdrestore/xfs_mdrestore.c
> @@ -7,7 +7,6 @@
>  #include "libxfs.h"
>  #include "xfs_metadump.h"
> 
> -char 		*progname;
>  static int	show_progress = 0;
>  static int	show_info = 0;
>  static int	progress_since_warning = 0;
> 

Re: [PATCH] xfsprogs: do not redeclare globals provided by libraries

[Eric Sandeen]

On 1/27/20 9:29 PM, Darrick J. Wong wrote:
> On Mon, Jan 27, 2020 at 04:56:02PM -0600, Eric Sandeen wrote:
>> From: Eric Sandeen <sandeen@redhat.com>
>>
>> In each of these cases, db, logprint, and mdrestore are redeclaring
>> as a global variable something which was already provided by a
>> library they link with.
> 
> Er... which library?

libxfs and libxlog ...


   File                Line
0 libxlog/util.c      10 int print_exit;
1 logprint/logprint.c 27 int print_exit = 1;

   File           Line
0 db/init.c      30 libxfs_init_t x;
1 libxlog/util.c 13 libxfs_init_t x;

   File                      Line
0 fsr/xfs_fsr.c              31 char *progname;
1 io/init.c                  14 char *progname;
2 libxfs/init.c              28 char *progname = "libxfs";
3 mdrestore/xfs_mdrestore.c  10 char *progname;

(fsr & io don't link w/ libxfs; mdrestore does)


> 
> Also, uh...maybe we shouldn't be exporting globals across libraries?
> 
> (He says having not looked for how many there are lurki... ye gods)

Well, it's ugly for sure.

We could either try to re-architect this to

1) pass stuff like progname all over the place, or
2) consistently make the library provide it as a global, or
3) consistently make utils provide it to the library as a global (?)

choose your poison?

> 
> --D
> 
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>
>> diff --git a/db/init.c b/db/init.c
>> index 455220a..0ac3736 100644
>> --- a/db/init.c
>> +++ b/db/init.c
>> @@ -27,7 +27,6 @@ static int		force;
>>   static struct xfs_mount	xmount;
>>   struct xfs_mount	*mp;
>>   static struct xlog	xlog;
>> -libxfs_init_t		x;
>>   xfs_agnumber_t		cur_agno = NULLAGNUMBER;
>>
>>   static void
>> diff --git a/logprint/logprint.c b/logprint/logprint.c
>> index 7754a2a..511a32a 100644
>> --- a/logprint/logprint.c
>> +++ b/logprint/logprint.c
>> @@ -24,7 +24,6 @@ int	print_buffer;
>>   int	print_overwrite;
>>   int     print_no_data;
>>   int     print_no_print;
>> -int     print_exit = 1; /* -e is now default. specify -c to override */
>>   static int	print_operation = OP_PRINT;
>>
>>   static void
>> @@ -132,6 +131,7 @@ main(int argc, char **argv)
>>   	bindtextdomain(PACKAGE, LOCALEDIR);
>>   	textdomain(PACKAGE);
>>   	memset(&mount, 0, sizeof(mount));
>> +	print_exit = 1; /* -e is now default. specify -c to override */
>>
>>   	progname = basename(argv[0]);
>>   	while ((c = getopt(argc, argv, "bC:cdefl:iqnors:tDVv")) != EOF) {
>> @@ -152,7 +152,7 @@ main(int argc, char **argv)
>>   			case 'e':
>>   			    /* -e is now default
>>   			     */
>> -				print_exit++;
>> +				print_exit = 1;
>>   				break;
>>   			case 'C':
>>   				print_operation = OP_COPY;
>> diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c
>> index 3375e08..1cd399d 100644
>> --- a/mdrestore/xfs_mdrestore.c
>> +++ b/mdrestore/xfs_mdrestore.c
>> @@ -7,7 +7,6 @@
>>   #include "libxfs.h"
>>   #include "xfs_metadump.h"
>>
>> -char 		*progname;
>>   static int	show_progress = 0;
>>   static int	show_info = 0;
>>   static int	progress_since_warning = 0;
>>
> 

Re: [PATCH] xfsprogs: do not redeclare globals provided by libraries

[Eric Sandeen]

On 1/27/20 4:56 PM, Eric Sandeen wrote:
> From: Eric Sandeen <sandeen@redhat.com>
> 
> In each of these cases, db, logprint, and mdrestore are redeclaring
> as a global variable something which was already provided by a
> library they link with.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---

(sigh, it appears I have whitespace mangling gremlins back again,
sorry - was temporarily on a different system w/ my normal email tools
absent.)

-Eric

Re: [PATCH] xfsprogs: do not redeclare globals provided by libraries

[Dave Chinner]

On Tue, Jan 28, 2020 at 08:48:40AM -0600, Eric Sandeen wrote:
> On 1/27/20 9:29 PM, Darrick J. Wong wrote:
> > On Mon, Jan 27, 2020 at 04:56:02PM -0600, Eric Sandeen wrote:
> > > From: Eric Sandeen <sandeen@redhat.com>
> > > 
> > > In each of these cases, db, logprint, and mdrestore are redeclaring
> > > as a global variable something which was already provided by a
> > > library they link with.
> > 
> > Er... which library?
> 
> libxfs and libxlog ...
> 
> 
>   File                Line
> 0 libxlog/util.c      10 int print_exit;
> 1 logprint/logprint.c 27 int print_exit = 1;
> 
>   File           Line
> 0 db/init.c      30 libxfs_init_t x;
> 1 libxlog/util.c 13 libxfs_init_t x;
> 
>   File                      Line
> 0 fsr/xfs_fsr.c              31 char *progname;
> 1 io/init.c                  14 char *progname;
> 2 libxfs/init.c              28 char *progname = "libxfs";
> 3 mdrestore/xfs_mdrestore.c  10 char *progname;
> 
> (fsr & io don't link w/ libxfs; mdrestore does)
> 
> 
> > 
> > Also, uh...maybe we shouldn't be exporting globals across libraries?
> > 
> > (He says having not looked for how many there are lurki... ye gods)
> 
> Well, it's ugly for sure.
> 
> We could either try to re-architect this to
> 
> 1) pass stuff like progname all over the place, or
> 2) consistently make the library provide it as a global, or
> 3) consistently make utils provide it to the library as a global (?)

IIRC, I chose #2 way back when I was consolidating all the libxfs
library code. There was code that declared libxfs_init_t x; on
stack, as local globals, in the libraries, etc. So I simply made the
library global the One True Global, and then had everyone use it
everywhere.

That was just the simplest solution at the time to minimise the
amount of work to get userspace up to date with the kernel to allow
integration of the CRC work (userspace was years out of date at that
point). It was not pretty (like a lot of my code), but it worked.

Feel free to do what you think is best :)

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] xfsprogs: do not redeclare globals provided by libraries

[Eric Sandeen]

On 1/28/20 4:26 PM, Dave Chinner wrote:
> On Tue, Jan 28, 2020 at 08:48:40AM -0600, Eric Sandeen wrote:
>> On 1/27/20 9:29 PM, Darrick J. Wong wrote:
>>> On Mon, Jan 27, 2020 at 04:56:02PM -0600, Eric Sandeen wrote:
>>>> From: Eric Sandeen <sandeen@redhat.com>
>>>>
>>>> In each of these cases, db, logprint, and mdrestore are redeclaring
>>>> as a global variable something which was already provided by a
>>>> library they link with.
>>>
>>> Er... which library?
>>
>> libxfs and libxlog ...
>>
>>
>>   File                Line
>> 0 libxlog/util.c      10 int print_exit;
>> 1 logprint/logprint.c 27 int print_exit = 1;
>>
>>   File           Line
>> 0 db/init.c      30 libxfs_init_t x;
>> 1 libxlog/util.c 13 libxfs_init_t x;
>>
>>   File                      Line
>> 0 fsr/xfs_fsr.c              31 char *progname;
>> 1 io/init.c                  14 char *progname;
>> 2 libxfs/init.c              28 char *progname = "libxfs";
>> 3 mdrestore/xfs_mdrestore.c  10 char *progname;
>>
>> (fsr & io don't link w/ libxfs; mdrestore does)
>>
>>
>>>
>>> Also, uh...maybe we shouldn't be exporting globals across libraries?
>>>
>>> (He says having not looked for how many there are lurki... ye gods)
>>
>> Well, it's ugly for sure.
>>
>> We could either try to re-architect this to
>>
>> 1) pass stuff like progname all over the place, or
>> 2) consistently make the library provide it as a global, or
>> 3) consistently make utils provide it to the library as a global (?)
> 
> IIRC, I chose #2 way back when I was consolidating all the libxfs
> library code. There was code that declared libxfs_init_t x; on
> stack, as local globals, in the libraries, etc. So I simply made the
> library global the One True Global, and then had everyone use it
> everywhere.
> 
> That was just the simplest solution at the time to minimise the
> amount of work to get userspace up to date with the kernel to allow
> integration of the CRC work (userspace was years out of date at that
> point). It was not pretty (like a lot of my code), but it worked.
> 
> Feel free to do what you think is best :)

I think the patch I sent is best - not quite sure how the stray globals
snuck in as problems (or maybe got missed the first time) but here we are.

gcc 10 complains about them btw, I should have mentioned that.

All I need now is a review.  :D

-Eric

[PATCH V2] xfsprogs: do not redeclare globals provided by libraries

[Eric Sandeen]

In each of these cases, db, logprint, and mdrestore are redeclaring
as a global variable something which was already provided by a
library they link with. 

gcc now defaults to -fno-common and trips over these global variables
which are declared in utilities as well as in libxfs and libxlog, and
it causes the build to fail.

Signed-off-by: Eric Sandeen <sandeen@redhat.com> 
---

V2: unmangle whitespace, I'm a n00b.

diff --git a/db/init.c b/db/init.c
index 455220a..0ac3736 100644
--- a/db/init.c
+++ b/db/init.c
@@ -27,7 +27,6 @@ static int		force;
 static struct xfs_mount	xmount;
 struct xfs_mount	*mp;
 static struct xlog	xlog;
-libxfs_init_t		x;
 xfs_agnumber_t		cur_agno = NULLAGNUMBER;
 
 static void
diff --git a/logprint/logprint.c b/logprint/logprint.c
index 7754a2a..5809af9 100644
--- a/logprint/logprint.c
+++ b/logprint/logprint.c
@@ -24,7 +24,6 @@ int	print_buffer;
 int	print_overwrite;
 int     print_no_data;
 int     print_no_print;
-int     print_exit = 1; /* -e is now default. specify -c to override */
 static int	print_operation = OP_PRINT;
 
 static void
@@ -132,6 +131,7 @@ main(int argc, char **argv)
 	bindtextdomain(PACKAGE, LOCALEDIR);
 	textdomain(PACKAGE);
 	memset(&mount, 0, sizeof(mount));
+	print_exit = 1; /* -e is now default. specify -c to override */ 
 
 	progname = basename(argv[0]);
 	while ((c = getopt(argc, argv, "bC:cdefl:iqnors:tDVv")) != EOF) {
@@ -152,7 +152,7 @@ main(int argc, char **argv)
 			case 'e':
 			    /* -e is now default
 			     */
-				print_exit++;
+				print_exit = 1;
 				break;
 			case 'C':
 				print_operation = OP_COPY;
diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c
index 3375e08..1cd399d 100644
--- a/mdrestore/xfs_mdrestore.c
+++ b/mdrestore/xfs_mdrestore.c
@@ -7,7 +7,6 @@
 #include "libxfs.h"
 #include "xfs_metadump.h"
 
-char 		*progname;
 static int	show_progress = 0;
 static int	show_info = 0;
 static int	progress_since_warning = 0;

Re: [PATCH V2] xfsprogs: do not redeclare globals provided by libraries

[Darrick J. Wong]

On Wed, Jan 29, 2020 at 09:16:58AM -0600, Eric Sandeen wrote:
> In each of these cases, db, logprint, and mdrestore are redeclaring
> as a global variable something which was already provided by a
> library they link with. 
> 
> gcc now defaults to -fno-common and trips over these global variables
> which are declared in utilities as well as in libxfs and libxlog, and
> it causes the build to fail.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com> 
> ---
> 
> V2: unmangle whitespace, I'm a n00b.

BOFH FTW

> diff --git a/db/init.c b/db/init.c
> index 455220a..0ac3736 100644
> --- a/db/init.c
> +++ b/db/init.c
> @@ -27,7 +27,6 @@ static int		force;
>  static struct xfs_mount	xmount;
>  struct xfs_mount	*mp;
>  static struct xlog	xlog;
> -libxfs_init_t		x;
>  xfs_agnumber_t		cur_agno = NULLAGNUMBER;
>  
>  static void
> diff --git a/logprint/logprint.c b/logprint/logprint.c
> index 7754a2a..5809af9 100644
> --- a/logprint/logprint.c
> +++ b/logprint/logprint.c
> @@ -24,7 +24,6 @@ int	print_buffer;
>  int	print_overwrite;
>  int     print_no_data;
>  int     print_no_print;
> -int     print_exit = 1; /* -e is now default. specify -c to override */
>  static int	print_operation = OP_PRINT;
>  
>  static void
> @@ -132,6 +131,7 @@ main(int argc, char **argv)
>  	bindtextdomain(PACKAGE, LOCALEDIR);
>  	textdomain(PACKAGE);
>  	memset(&mount, 0, sizeof(mount));
> +	print_exit = 1; /* -e is now default. specify -c to override */ 

With the trailing whitespace after the comment fixed,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

Though given your earlier comment on IRC, maybe we should investigate
why -fno-common would be useful (since Fedora turned it on??) or if it
should be in the regular build to catch multiply defined global vars?

--D

>  
>  	progname = basename(argv[0]);
>  	while ((c = getopt(argc, argv, "bC:cdefl:iqnors:tDVv")) != EOF) {
> @@ -152,7 +152,7 @@ main(int argc, char **argv)
>  			case 'e':
>  			    /* -e is now default
>  			     */
> -				print_exit++;
> +				print_exit = 1;
>  				break;
>  			case 'C':
>  				print_operation = OP_COPY;
> diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c
> index 3375e08..1cd399d 100644
> --- a/mdrestore/xfs_mdrestore.c
> +++ b/mdrestore/xfs_mdrestore.c
> @@ -7,7 +7,6 @@
>  #include "libxfs.h"
>  #include "xfs_metadump.h"
>  
> -char 		*progname;
>  static int	show_progress = 0;
>  static int	show_info = 0;
>  static int	progress_since_warning = 0;
> 
> 

Re: [PATCH V2] xfsprogs: do not redeclare globals provided by libraries

[Eric Sandeen]

On 1/29/20 10:01 AM, Darrick J. Wong wrote:
> On Wed, Jan 29, 2020 at 09:16:58AM -0600, Eric Sandeen wrote:
>> In each of these cases, db, logprint, and mdrestore are redeclaring
>> as a global variable something which was already provided by a
>> library they link with. 
>>
>> gcc now defaults to -fno-common and trips over these global variables
>> which are declared in utilities as well as in libxfs and libxlog, and
>> it causes the build to fail.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com> 
>> ---

...

>> +	print_exit = 1; /* -e is now default. specify -c to override */ 
> 
> With the trailing whitespace after the comment fixed,

oh burn. ;)

> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Though given your earlier comment on IRC, maybe we should investigate
> why -fno-common would be useful (since Fedora turned it on??) or if it
> should be in the regular build to catch multiply defined global vars?

That's a good idea.  FWIW my understanding is that it's a new default in
gcc10.

Docs say w.r.t. -fcommon: "This behavior is inconsistent with C++, and on many targets implies a speed and code size penalty on global variable references. It is mainly useful to enable legacy code to link without errors."

Want me to send a V3 w/ -fno-common explicitly set?

-Eric

Re: [PATCH] xfsprogs: do not redeclare globals provided by libraries

[Christoph Hellwig]

On Mon, Jan 27, 2020 at 04:56:02PM -0600, Eric Sandeen wrote:
> From: Eric Sandeen <sandeen@redhat.com>
> 
> In each of these cases, db, logprint, and mdrestore are redeclaring
> as a global variable something which was already provided by a
> library they link with.

Independent of any better way to handle the library interaction this
looks like an improvement on its own:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH V2] xfsprogs: do not redeclare globals provided by libraries

[Christoph Hellwig]

And with unmangled whitespaces it looks even better :)

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH V2] xfsprogs: do not redeclare globals provided by libraries

[Darrick J. Wong]

On Wed, Jan 29, 2020 at 10:57:48AM -0600, Eric Sandeen wrote:
> On 1/29/20 10:01 AM, Darrick J. Wong wrote:
> > On Wed, Jan 29, 2020 at 09:16:58AM -0600, Eric Sandeen wrote:
> >> In each of these cases, db, logprint, and mdrestore are redeclaring
> >> as a global variable something which was already provided by a
> >> library they link with. 
> >>
> >> gcc now defaults to -fno-common and trips over these global variables
> >> which are declared in utilities as well as in libxfs and libxlog, and
> >> it causes the build to fail.
> >>
> >> Signed-off-by: Eric Sandeen <sandeen@redhat.com> 
> >> ---
> 
> ...
> 
> >> +	print_exit = 1; /* -e is now default. specify -c to override */ 
> > 
> > With the trailing whitespace after the comment fixed,
> 
> oh burn. ;)
> 
> > Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Though given your earlier comment on IRC, maybe we should investigate
> > why -fno-common would be useful (since Fedora turned it on??) or if it
> > should be in the regular build to catch multiply defined global vars?
> 
> That's a good idea.  FWIW my understanding is that it's a new default in
> gcc10.
> 
> Docs say w.r.t. -fcommon: "This behavior is inconsistent with C++, and on many targets implies a speed and code size penalty on global variable references. It is mainly useful to enable legacy code to link without errors."
> 
> Want me to send a V3 w/ -fno-common explicitly set?

Yes, that sounds like a better place to start a conversation about ...
whatever that option is. :)

--D

> -Eric

[PATCH V3] xfsprogs: do not redeclare globals provided by libraries

[Eric Sandeen]

In each of these cases, db, logprint, and mdrestore are redeclaring
as a global variable something which was already provided by a
library they link with. 

gcc now defaults to -fno-common and trips over these global variables
which are declared in utilities as well as in libxfs and libxlog, and
it causes the build to fail.

Signed-off-by: Eric Sandeen <sandeen@redhat.com> 
---

V2: unmangle whitespace, I'm a n00b.
V3: ditto, plus enforce -fno-common so older gcc users will play
    by the same rules as gcc 10

diff --git a/db/init.c b/db/init.c
index 455220a8..0ac37368 100644
--- a/db/init.c
+++ b/db/init.c
@@ -27,7 +27,6 @@ static int		force;
 static struct xfs_mount	xmount;
 struct xfs_mount	*mp;
 static struct xlog	xlog;
-libxfs_init_t		x;
 xfs_agnumber_t		cur_agno = NULLAGNUMBER;
 
 static void
diff --git a/include/builddefs.in b/include/builddefs.in
index 4700b527..0a5cce32 100644
--- a/include/builddefs.in
+++ b/include/builddefs.in
@@ -13,7 +13,7 @@ OPTIMIZER = @opt_build@
 MALLOCLIB = @malloc_lib@
 LOADERFLAGS = @LDFLAGS@
 LTLDFLAGS = @LDFLAGS@
-CFLAGS = @CFLAGS@ -D_FILE_OFFSET_BITS=64
+CFLAGS = @CFLAGS@ -D_FILE_OFFSET_BITS=64 -fno-common
 BUILD_CFLAGS = @BUILD_CFLAGS@ -D_FILE_OFFSET_BITS=64
 
 LIBRT = @librt@
diff --git a/logprint/logprint.c b/logprint/logprint.c
index 7754a2a6..11100569 100644
--- a/logprint/logprint.c
+++ b/logprint/logprint.c
@@ -24,7 +24,6 @@ int	print_buffer;
 int	print_overwrite;
 int     print_no_data;
 int     print_no_print;
-int     print_exit = 1; /* -e is now default. specify -c to override */
 static int	print_operation = OP_PRINT;
 
 static void
@@ -132,6 +131,7 @@ main(int argc, char **argv)
 	bindtextdomain(PACKAGE, LOCALEDIR);
 	textdomain(PACKAGE);
 	memset(&mount, 0, sizeof(mount));
+	print_exit = 1;	/* -e is now default. specify -c to override */
 
 	progname = basename(argv[0]);
 	while ((c = getopt(argc, argv, "bC:cdefl:iqnors:tDVv")) != EOF) {
@@ -152,7 +152,7 @@ main(int argc, char **argv)
 			case 'e':
 			    /* -e is now default
 			     */
-				print_exit++;
+				print_exit = 1;
 				break;
 			case 'C':
 				print_operation = OP_COPY;
diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c
index 3375e080..1cd399db 100644
--- a/mdrestore/xfs_mdrestore.c
+++ b/mdrestore/xfs_mdrestore.c
@@ -7,7 +7,6 @@
 #include "libxfs.h"
 #include "xfs_metadump.h"
 
-char 		*progname;
 static int	show_progress = 0;
 static int	show_info = 0;
 static int	progress_since_warning = 0;

Re: [PATCH V3] xfsprogs: do not redeclare globals provided by libraries

[Christoph Hellwig]

On Wed, Jan 29, 2020 at 01:47:49PM -0600, Eric Sandeen wrote:
> V3: ditto, plus enforce -fno-common so older gcc users will play
>     by the same rules as gcc 10

I don't mind adding -fno-common, but it has aboslutely nothing to do
with removing pointless duplicate variable defintions.

I'd say apply v2 and make this a separate patch.

> @@ -13,7 +13,7 @@ OPTIMIZER = @opt_build@
>  MALLOCLIB = @malloc_lib@
>  LOADERFLAGS = @LDFLAGS@
>  LTLDFLAGS = @LDFLAGS@
> -CFLAGS = @CFLAGS@ -D_FILE_OFFSET_BITS=64
> +CFLAGS = @CFLAGS@ -D_FILE_OFFSET_BITS=64 -fno-common

Are there any compilers not supporting -fno-common?  (And do we care?)

Re: [PATCH V3] xfsprogs: do not redeclare globals provided by libraries

[Eric Sandeen]

On 1/29/20 3:32 PM, Christoph Hellwig wrote:
> On Wed, Jan 29, 2020 at 01:47:49PM -0600, Eric Sandeen wrote:
>> V3: ditto, plus enforce -fno-common so older gcc users will play
>>     by the same rules as gcc 10
> 
> I don't mind adding -fno-common, but it has aboslutely nothing to do
> with removing pointless duplicate variable defintions.
> 
> I'd say apply v2 and make this a separate patch.

ok

>> @@ -13,7 +13,7 @@ OPTIMIZER = @opt_build@
>>  MALLOCLIB = @malloc_lib@
>>  LOADERFLAGS = @LDFLAGS@
>>  LTLDFLAGS = @LDFLAGS@
>> -CFLAGS = @CFLAGS@ -D_FILE_OFFSET_BITS=64
>> +CFLAGS = @CFLAGS@ -D_FILE_OFFSET_BITS=64 -fno-common
> 
> Are there any compilers not supporting -fno-common?  (And do we care?)

not sure if it'd vary by version or by arch, but

gcc version 4.4.7

has it on RHEL7

-Eric