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@xxxxxxxxxx> > > > > > > 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@xxxxxxxxxxxxx
