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

[Dave Chinner <david@fromorbit.com>]

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