public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Rename progname as it is provided by libc
@ 2017-09-02 21:54 Khem Raj
  2017-09-02 22:20 ` Dave Chinner
  2017-09-08 16:25 ` Eric Sandeen
  0 siblings, 2 replies; 8+ messages in thread
From: Khem Raj @ 2017-09-02 21:54 UTC (permalink / raw)
  To: linux-xfs; +Cc: Khem Raj

Rename local variable progname to avoid a clash with libc
global symbols

Signed-off-by: Khem Raj <raj.khem@gmail.com>
---
 io/init.c                 | 10 +++++-----
 mdrestore/xfs_mdrestore.c | 10 +++++-----
 quota/init.c              | 10 +++++-----
 spaceman/init.c           |  8 ++++----
 4 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/io/init.c b/io/init.c
index 20d5f80..e82e101 100644
--- a/io/init.c
+++ b/io/init.c
@@ -23,7 +23,7 @@
 #include "init.h"
 #include "io.h"
 
-char	*progname;
+char	*io_progname;
 int	exitcode;
 int	expert;
 int	idlethread;
@@ -35,7 +35,7 @@ usage(void)
 {
 	fprintf(stderr,
 _("Usage: %s [-adfinrRstVx] [-m mode] [-p prog] [[-c|-C] cmd]... file\n"),
-		progname);
+		io_progname);
 	exit(1);
 }
 
@@ -142,7 +142,7 @@ init(
 	xfs_fsop_geom_t	geometry = { 0 };
 	struct fs_path	fsp;
 
-	progname = basename(argv[0]);
+	io_progname = basename(argv[0]);
 	setlocale(LC_ALL, "");
 	bindtextdomain(PACKAGE, LOCALEDIR);
 	textdomain(PACKAGE);
@@ -186,7 +186,7 @@ init(
 			flags |= IO_NONBLOCK;
 			break;
 		case 'p':
-			progname = optarg;
+			io_progname = optarg;
 			break;
 		case 'r':
 			flags |= IO_READONLY;
@@ -207,7 +207,7 @@ init(
 			expert = 1;
 			break;
 		case 'V':
-			printf(_("%s version %s\n"), progname, VERSION);
+			printf(_("%s version %s\n"), io_progname, VERSION);
 			exit(0);
 		default:
 			usage();
diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c
index 9d1b4e8..b840a54 100644
--- a/mdrestore/xfs_mdrestore.c
+++ b/mdrestore/xfs_mdrestore.c
@@ -19,7 +19,7 @@
 #include "libxfs.h"
 #include "xfs_metadump.h"
 
-char 		*progname;
+char 		*mdrestore_progname;
 int		show_progress = 0;
 int		show_info = 0;
 int		progress_since_warning = 0;
@@ -30,7 +30,7 @@ fatal(const char *msg, ...)
 	va_list		args;
 
 	va_start(args, msg);
-	fprintf(stderr, "%s: ", progname);
+	fprintf(stderr, "%s: ", mdrestore_progname);
 	vfprintf(stderr, msg, args);
 	exit(1);
 }
@@ -194,7 +194,7 @@ perform_restore(
 static void
 usage(void)
 {
-	fprintf(stderr, "Usage: %s [-V] [-g] source target\n", progname);
+	fprintf(stderr, "Usage: %s [-V] [-g] source target\n", mdrestore_progname);
 	exit(1);
 }
 
@@ -212,7 +212,7 @@ main(
 	struct stat	statbuf;
 	int		is_target_file;
 
-	progname = basename(argv[0]);
+	mdrestore_progname = basename(argv[0]);
 
 	while ((c = getopt(argc, argv, "giV")) != EOF) {
 		switch (c) {
@@ -223,7 +223,7 @@ main(
 				show_info = 1;
 				break;
 			case 'V':
-				printf("%s version %s\n", progname, VERSION);
+				printf("%s version %s\n", mdrestore_progname, VERSION);
 				exit(0);
 			default:
 				usage();
diff --git a/quota/init.c b/quota/init.c
index d45dc4c..46403de 100644
--- a/quota/init.c
+++ b/quota/init.c
@@ -21,7 +21,7 @@
 #include "input.h"
 #include "init.h"
 
-char	*progname;
+char	*quota_progname;
 int	exitcode;
 int	expert;
 bool	foreign_allowed = false;
@@ -47,7 +47,7 @@ usage(void)
 {
 	fprintf(stderr,
 		_("Usage: %s [-V] [-x] [-f] [-p prog] [-c cmd]... [-d project]... [path]\n"),
-		progname);
+		quota_progname);
 	exit(1);
 }
 
@@ -147,7 +147,7 @@ init(
 {
 	int		c;
 
-	progname = basename(argv[0]);
+	quota_progname = basename(argv[0]);
 	setlocale(LC_ALL, "");
 	bindtextdomain(PACKAGE, LOCALEDIR);
 	textdomain(PACKAGE);
@@ -173,13 +173,13 @@ init(
 			projid_file = optarg;
 			break;
 		case 'p':
-			progname = optarg;
+			quota_progname = optarg;
 			break;
 		case 'x':
 			expert++;
 			break;
 		case 'V':
-			printf(_("%s version %s\n"), progname, VERSION);
+			printf(_("%s version %s\n"), quota_progname, VERSION);
 			exit(0);
 		default:
 			usage();
diff --git a/spaceman/init.c b/spaceman/init.c
index b3eface..bedf112 100644
--- a/spaceman/init.c
+++ b/spaceman/init.c
@@ -23,7 +23,7 @@
 #include "path.h"
 #include "space.h"
 
-char	*progname;
+char	*spaceman_progname;
 int	exitcode;
 
 void
@@ -31,7 +31,7 @@ usage(void)
 {
 	fprintf(stderr,
 		_("Usage: %s [-c cmd] file\n"),
-		progname);
+		spaceman_progname);
 	exit(1);
 }
 
@@ -74,7 +74,7 @@ init(
 	xfs_fsop_geom_t	geometry = { 0 };
 	struct fs_path	fsp;
 
-	progname = basename(argv[0]);
+	spaceman_progname = basename(argv[0]);
 	setlocale(LC_ALL, "");
 	bindtextdomain(PACKAGE, LOCALEDIR);
 	textdomain(PACKAGE);
@@ -86,7 +86,7 @@ init(
 			add_user_command(optarg);
 			break;
 		case 'V':
-			printf(_("%s version %s\n"), progname, VERSION);
+			printf(_("%s version %s\n"), spaceman_progname, VERSION);
 			exit(0);
 		default:
 			usage();
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] Rename progname as it is provided by libc
  2017-09-02 21:54 [PATCH] Rename progname as it is provided by libc Khem Raj
@ 2017-09-02 22:20 ` Dave Chinner
  2017-09-02 23:42   ` Khem Raj
                     ` (2 more replies)
  2017-09-08 16:25 ` Eric Sandeen
  1 sibling, 3 replies; 8+ messages in thread
From: Dave Chinner @ 2017-09-02 22:20 UTC (permalink / raw)
  To: Khem Raj; +Cc: linux-xfs

On Sat, Sep 02, 2017 at 02:54:51PM -0700, Khem Raj wrote:
> Rename local variable progname to avoid a clash with libc
> global symbols

What libc is causing this problem?

And why is it considered ok for that libc to pollute the global
variable namespace with it's own internal variables?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Rename progname as it is provided by libc
  2017-09-02 22:20 ` Dave Chinner
@ 2017-09-02 23:42   ` Khem Raj
  2017-09-03  7:21   ` Khem Raj
  2017-09-03  7:48   ` Christoph Hellwig
  2 siblings, 0 replies; 8+ messages in thread
From: Khem Raj @ 2017-09-02 23:42 UTC (permalink / raw)
  To: Dave Chinner; +Cc: linux-xfs

On Sat, Sep 2, 2017 at 3:20 PM, Dave Chinner <david@fromorbit.com> wrote:
> On Sat, Sep 02, 2017 at 02:54:51PM -0700, Khem Raj wrote:
>> Rename local variable progname to avoid a clash with libc
>> global symbols
>
> What libc is causing this problem?
>
> And why is it considered ok for that libc to pollute the global
> variable namespace with it's own internal variables?
>

I am building with -staitc -pie options

xfs_mdrestore.o: relocation R_ARM_REL32 against external or undefined
symbol `progname' can not be used when making a PIE executable;
recompile with -fPIC

libcs have __progname, So I will have to check where is progname coming from
probably an external header. I think renaming to avoid conflict is
good thing regardless.

> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Rename progname as it is provided by libc
  2017-09-02 22:20 ` Dave Chinner
  2017-09-02 23:42   ` Khem Raj
@ 2017-09-03  7:21   ` Khem Raj
  2017-09-03  7:48   ` Christoph Hellwig
  2 siblings, 0 replies; 8+ messages in thread
From: Khem Raj @ 2017-09-03  7:21 UTC (permalink / raw)
  To: Dave Chinner; +Cc: linux-xfs

On Sun, Sep 03, 2017 at 08:20:03AM +1000, Dave Chinner wrote:
> On Sat, Sep 02, 2017 at 02:54:51PM -0700, Khem Raj wrote:
> > Rename local variable progname to avoid a clash with libc
> > global symbols
> 
> What libc is causing this problem?
> 
> And why is it considered ok for that libc to pollute the global
> variable namespace with it's own internal variables?

A closer look reveals that its getting the function from another local library which is used
on the linker commandline. below is full cmdline

../arm-bec-linux-musleabi-libtool --quiet --tag=CC --mode=link ../../recipe-sysroot-native/usr/bin/arm-bec-linux-musleabi/arm-bec-linux-musleabi-clang -march=armv7ve -mthumb -mfpu=neon-vfpv4 -mfloat-abi=hard -mcpu=cortex-a7 -mlittle-endian -D__extern_always_inline=inline -no-integrated-as -Wno-error=unused-command-line-argument -Qunused-arguments -fstack-protector-strong  -D_FORTIFY_SOURCE=2 -Wformat -Wformat-security -Werror=format-security --sysroot=/mnt/a/oe/build/tmp/work/cortexa7t2hf-neon-vfpv4-bec-linux-musleabi/xfsprogs/4.12.0-r0/recipe-sysroot -o xfs_io -Wl,-O1 -Wl,--hash-style=gnu -Wl,--as-needed -fstack-protector-strong -Wl,-z,relro,-z,now -Wl,-O1 -Wl,--hash-style=gnu -Wl,--as-needed -fstack-protector-strong -Wl,-z,relro,-z,now  -Wl,-O1 -Wl,--hash-style=gnu -Wl,--as-needed -fstack-protector-strong -Wl,-z,relro,-z,now  -Wl,-O1 -Wl,--hash-style=gnu -Wl,--as-needed -fstack-protector-strong -Wl,-z,relro,-z,now -static-libtool-libs  init.o attr.o bmap.o cowextsize.o encrypt.o file.o freeze.o fsync.o getrusage.o imap.o link.o mmap.o open.o parent.o pread.o prealloc.o pwrite.o reflink.o seek.o shutdown.o stat.o sync.o truncate.o utimes.o fadvise.o madvise.o mincore.o sendfile.o fiemap.o inject.o resblks.o copy_file_range.o sync_file_range.o readdir.o fsmap.o   ../libxcmd/libxcmd.la ../libhandle/libhandle.la -luuid -lpthread -lreadline


libxfs.so.0                   init.o (progname)

and also from init.o object from current directory

$ readelf -sW init.o | grep progname                                                                                                                                                                                            (09-03 00:17)
    56: 00000004     4 OBJECT  GLOBAL DEFAULT  COM progname

readelf -sW ../libxfs/.libs/libxfs.so.0 | grep progname                                                                                                                                                                       (09-03 00:19)
   214: 0005d004     4 OBJECT  GLOBAL DEFAULT   18 progname

Thats what is causing the conflict.

> 
> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Rename progname as it is provided by libc
  2017-09-02 22:20 ` Dave Chinner
  2017-09-02 23:42   ` Khem Raj
  2017-09-03  7:21   ` Khem Raj
@ 2017-09-03  7:48   ` Christoph Hellwig
  2 siblings, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2017-09-03  7:48 UTC (permalink / raw)
  To: Dave Chinner; +Cc: Khem Raj, linux-xfs

On Sun, Sep 03, 2017 at 08:20:03AM +1000, Dave Chinner wrote:
> On Sat, Sep 02, 2017 at 02:54:51PM -0700, Khem Raj wrote:
> > Rename local variable progname to avoid a clash with libc
> > global symbols
> 
> What libc is causing this problem?
> 
> And why is it considered ok for that libc to pollute the global
> variable namespace with it's own internal variables?

I don't think it's libc.  It's our own little mess where we
want to set a global progname for libxfs use in the tools.

The right interface is to have a private libxfs_progname routine
that isn't accessible by the tools..

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Rename progname as it is provided by libc
  2017-09-02 21:54 [PATCH] Rename progname as it is provided by libc Khem Raj
  2017-09-02 22:20 ` Dave Chinner
@ 2017-09-08 16:25 ` Eric Sandeen
  2017-09-08 16:29   ` Khem Raj
  2017-09-08 18:13   ` Khem Raj
  1 sibling, 2 replies; 8+ messages in thread
From: Eric Sandeen @ 2017-09-08 16:25 UTC (permalink / raw)
  To: Khem Raj, linux-xfs

On 9/2/17 4:54 PM, Khem Raj wrote:
> Rename local variable progname to avoid a clash with libc
> global symbols
> 
> Signed-off-by: Khem Raj <raj.khem@gmail.com>
> ---
>  io/init.c                 | 10 +++++-----
>  mdrestore/xfs_mdrestore.c | 10 +++++-----
>  quota/init.c              | 10 +++++-----
>  spaceman/init.c           |  8 ++++----
>  4 files changed, 19 insertions(+), 19 deletions(-)
>

First of all, this doesn't build.  Please don't send untested
patches...

    [LD]     xfs_io
attr.o: In function `chattr_f':
/mnt/test2/git/xfsprogs-maint/io/attr.c:316: undefined reference to `progname'
/mnt/test2/git/xfsprogs-maint/io/attr.c:310: undefined reference to `progname'
/mnt/test2/git/xfsprogs-maint/io/attr.c:332: undefined reference to `progname'
/mnt/test2/git/xfsprogs-maint/io/attr.c:326: undefined reference to `progname'
attr.o: In function `chattr_callback':
/mnt/test2/git/xfsprogs-maint/io/attr.c:252: undefined reference to `progname'
attr.o:/mnt/test2/git/xfsprogs-maint/io/attr.c:255: more undefined references to `progname' follow
collect2: ld returned 1 exit status
gmake[3]: *** [xfs_io] Error 1
gmake[2]: *** [io] Error 2
make[1]: *** [default] Error 2
make: *** [default] Error 2



second: is progname really a global symbol?  I know of __progname, but not progname.

i.e. this works:

#include <stdio.h>

extern const char *__progname;

int
main(
        int     argc,
        char    **argv)
{
        printf("progname: %s\n", __progname);
}

# gcc -o test test.c
# ./test
progname: test

but this doesn't:

#include <stdio.h>

extern const char *progname;

int
main(
        int     argc,
        char    **argv)
{
	printf("progname %s\n", progname);
}

# gcc -o test test.c
/tmp/ccjaZrXZ.o: In function `main':
test.c:(.text+0x12): undefined reference to `progname'
collect2: ld returned 1 exit status


-Eric

> diff --git a/io/init.c b/io/init.c
> index 20d5f80..e82e101 100644
> --- a/io/init.c
> +++ b/io/init.c
> @@ -23,7 +23,7 @@
>  #include "init.h"
>  #include "io.h"
>  
> -char	*progname;
> +char	*io_progname;
>  int	exitcode;
>  int	expert;
>  int	idlethread;
> @@ -35,7 +35,7 @@ usage(void)
>  {
>  	fprintf(stderr,
>  _("Usage: %s [-adfinrRstVx] [-m mode] [-p prog] [[-c|-C] cmd]... file\n"),
> -		progname);
> +		io_progname);
>  	exit(1);
>  }
>  
> @@ -142,7 +142,7 @@ init(
>  	xfs_fsop_geom_t	geometry = { 0 };
>  	struct fs_path	fsp;
>  
> -	progname = basename(argv[0]);
> +	io_progname = basename(argv[0]);
>  	setlocale(LC_ALL, "");
>  	bindtextdomain(PACKAGE, LOCALEDIR);
>  	textdomain(PACKAGE);
> @@ -186,7 +186,7 @@ init(
>  			flags |= IO_NONBLOCK;
>  			break;
>  		case 'p':
> -			progname = optarg;
> +			io_progname = optarg;
>  			break;
>  		case 'r':
>  			flags |= IO_READONLY;
> @@ -207,7 +207,7 @@ init(
>  			expert = 1;
>  			break;
>  		case 'V':
> -			printf(_("%s version %s\n"), progname, VERSION);
> +			printf(_("%s version %s\n"), io_progname, VERSION);
>  			exit(0);
>  		default:
>  			usage();
> diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c
> index 9d1b4e8..b840a54 100644
> --- a/mdrestore/xfs_mdrestore.c
> +++ b/mdrestore/xfs_mdrestore.c
> @@ -19,7 +19,7 @@
>  #include "libxfs.h"
>  #include "xfs_metadump.h"
>  
> -char 		*progname;
> +char 		*mdrestore_progname;
>  int		show_progress = 0;
>  int		show_info = 0;
>  int		progress_since_warning = 0;
> @@ -30,7 +30,7 @@ fatal(const char *msg, ...)
>  	va_list		args;
>  
>  	va_start(args, msg);
> -	fprintf(stderr, "%s: ", progname);
> +	fprintf(stderr, "%s: ", mdrestore_progname);
>  	vfprintf(stderr, msg, args);
>  	exit(1);
>  }
> @@ -194,7 +194,7 @@ perform_restore(
>  static void
>  usage(void)
>  {
> -	fprintf(stderr, "Usage: %s [-V] [-g] source target\n", progname);
> +	fprintf(stderr, "Usage: %s [-V] [-g] source target\n", mdrestore_progname);

line > 80 cols, we try to avoid that

>  	exit(1);
>  }
>  
> @@ -212,7 +212,7 @@ main(
>  	struct stat	statbuf;
>  	int		is_target_file;
>  
> -	progname = basename(argv[0]);
> +	mdrestore_progname = basename(argv[0]);
>  
>  	while ((c = getopt(argc, argv, "giV")) != EOF) {
>  		switch (c) {
> @@ -223,7 +223,7 @@ main(
>  				show_info = 1;
>  				break;
>  			case 'V':
> -				printf("%s version %s\n", progname, VERSION);
> +				printf("%s version %s\n", mdrestore_progname, VERSION);

80 col

>  				exit(0);
>  			default:
>  				usage();
> diff --git a/quota/init.c b/quota/init.c
> index d45dc4c..46403de 100644
> --- a/quota/init.c
> +++ b/quota/init.c
> @@ -21,7 +21,7 @@
>  #include "input.h"
>  #include "init.h"
>  
> -char	*progname;
> +char	*quota_progname;
>  int	exitcode;
>  int	expert;
>  bool	foreign_allowed = false;
> @@ -47,7 +47,7 @@ usage(void)
>  {
>  	fprintf(stderr,
>  		_("Usage: %s [-V] [-x] [-f] [-p prog] [-c cmd]... [-d project]... [path]\n"),
> -		progname);
> +		quota_progname);
>  	exit(1);
>  }
>  
> @@ -147,7 +147,7 @@ init(
>  {
>  	int		c;
>  
> -	progname = basename(argv[0]);
> +	quota_progname = basename(argv[0]);
>  	setlocale(LC_ALL, "");
>  	bindtextdomain(PACKAGE, LOCALEDIR);
>  	textdomain(PACKAGE);
> @@ -173,13 +173,13 @@ init(
>  			projid_file = optarg;
>  			break;
>  		case 'p':
> -			progname = optarg;
> +			quota_progname = optarg;
>  			break;
>  		case 'x':
>  			expert++;
>  			break;
>  		case 'V':
> -			printf(_("%s version %s\n"), progname, VERSION);
> +			printf(_("%s version %s\n"), quota_progname, VERSION);
>  			exit(0);
>  		default:
>  			usage();
> diff --git a/spaceman/init.c b/spaceman/init.c
> index b3eface..bedf112 100644
> --- a/spaceman/init.c
> +++ b/spaceman/init.c
> @@ -23,7 +23,7 @@
>  #include "path.h"
>  #include "space.h"
>  
> -char	*progname;
> +char	*spaceman_progname;
>  int	exitcode;
>  
>  void
> @@ -31,7 +31,7 @@ usage(void)
>  {
>  	fprintf(stderr,
>  		_("Usage: %s [-c cmd] file\n"),
> -		progname);
> +		spaceman_progname);
>  	exit(1);
>  }
>  
> @@ -74,7 +74,7 @@ init(
>  	xfs_fsop_geom_t	geometry = { 0 };
>  	struct fs_path	fsp;
>  
> -	progname = basename(argv[0]);
> +	spaceman_progname = basename(argv[0]);
>  	setlocale(LC_ALL, "");
>  	bindtextdomain(PACKAGE, LOCALEDIR);
>  	textdomain(PACKAGE);
> @@ -86,7 +86,7 @@ init(
>  			add_user_command(optarg);
>  			break;
>  		case 'V':
> -			printf(_("%s version %s\n"), progname, VERSION);
> +			printf(_("%s version %s\n"), spaceman_progname, VERSION);

80 col

>  			exit(0);
>  		default:
>  			usage();
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Rename progname as it is provided by libc
  2017-09-08 16:25 ` Eric Sandeen
@ 2017-09-08 16:29   ` Khem Raj
  2017-09-08 18:13   ` Khem Raj
  1 sibling, 0 replies; 8+ messages in thread
From: Khem Raj @ 2017-09-08 16:29 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: linux-xfs

On Fri, Sep 8, 2017 at 9:25 AM, Eric Sandeen <sandeen@sandeen.net> wrote:
> On 9/2/17 4:54 PM, Khem Raj wrote:
>> Rename local variable progname to avoid a clash with libc
>> global symbols
>>
>> Signed-off-by: Khem Raj <raj.khem@gmail.com>
>> ---
>>  io/init.c                 | 10 +++++-----
>>  mdrestore/xfs_mdrestore.c | 10 +++++-----
>>  quota/init.c              | 10 +++++-----
>>  spaceman/init.c           |  8 ++++----
>>  4 files changed, 19 insertions(+), 19 deletions(-)
>>
>
> First of all, this doesn't build.  Please don't send untested
> patches...
>
>     [LD]     xfs_io
> attr.o: In function `chattr_f':
> /mnt/test2/git/xfsprogs-maint/io/attr.c:316: undefined reference to `progname'
> /mnt/test2/git/xfsprogs-maint/io/attr.c:310: undefined reference to `progname'
> /mnt/test2/git/xfsprogs-maint/io/attr.c:332: undefined reference to `progname'
> /mnt/test2/git/xfsprogs-maint/io/attr.c:326: undefined reference to `progname'
> attr.o: In function `chattr_callback':
> /mnt/test2/git/xfsprogs-maint/io/attr.c:252: undefined reference to `progname'
> attr.o:/mnt/test2/git/xfsprogs-maint/io/attr.c:255: more undefined references to `progname' follow
> collect2: ld returned 1 exit status
> gmake[3]: *** [xfs_io] Error 1
> gmake[2]: *** [io] Error 2
> make[1]: *** [default] Error 2
> make: *** [default] Error 2
>
>
>
> second: is progname really a global symbol?  I know of __progname, but not progname.
>
> i.e. this works:
>
> #include <stdio.h>
>
> extern const char *__progname;
>
> int
> main(
>         int     argc,
>         char    **argv)
> {
>         printf("progname: %s\n", __progname);
> }
>
> # gcc -o test test.c
> # ./test
> progname: test
>
> but this doesn't:
>
> #include <stdio.h>
>
> extern const char *progname;
>
> int
> main(
>         int     argc,
>         char    **argv)
> {
>         printf("progname %s\n", progname);
> }
>
> # gcc -o test test.c
> /tmp/ccjaZrXZ.o: In function `main':
> test.c:(.text+0x12): undefined reference to `progname'
> collect2: ld returned 1 exit status
>
>
> -Eric
>
>> diff --git a/io/init.c b/io/init.c
>> index 20d5f80..e82e101 100644
>> --- a/io/init.c
>> +++ b/io/init.c
>> @@ -23,7 +23,7 @@
>>  #include "init.h"
>>  #include "io.h"
>>
>> -char *progname;
>> +char *io_progname;
>>  int  exitcode;
>>  int  expert;
>>  int  idlethread;
>> @@ -35,7 +35,7 @@ usage(void)
>>  {
>>       fprintf(stderr,
>>  _("Usage: %s [-adfinrRstVx] [-m mode] [-p prog] [[-c|-C] cmd]... file\n"),
>> -             progname);
>> +             io_progname);
>>       exit(1);
>>  }
>>
>> @@ -142,7 +142,7 @@ init(
>>       xfs_fsop_geom_t geometry = { 0 };
>>       struct fs_path  fsp;
>>
>> -     progname = basename(argv[0]);
>> +     io_progname = basename(argv[0]);
>>       setlocale(LC_ALL, "");
>>       bindtextdomain(PACKAGE, LOCALEDIR);
>>       textdomain(PACKAGE);
>> @@ -186,7 +186,7 @@ init(
>>                       flags |= IO_NONBLOCK;
>>                       break;
>>               case 'p':
>> -                     progname = optarg;
>> +                     io_progname = optarg;
>>                       break;
>>               case 'r':
>>                       flags |= IO_READONLY;
>> @@ -207,7 +207,7 @@ init(
>>                       expert = 1;
>>                       break;
>>               case 'V':
>> -                     printf(_("%s version %s\n"), progname, VERSION);
>> +                     printf(_("%s version %s\n"), io_progname, VERSION);
>>                       exit(0);
>>               default:
>>                       usage();
>> diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c
>> index 9d1b4e8..b840a54 100644
>> --- a/mdrestore/xfs_mdrestore.c
>> +++ b/mdrestore/xfs_mdrestore.c
>> @@ -19,7 +19,7 @@
>>  #include "libxfs.h"
>>  #include "xfs_metadump.h"
>>
>> -char                 *progname;
>> +char                 *mdrestore_progname;
>>  int          show_progress = 0;
>>  int          show_info = 0;
>>  int          progress_since_warning = 0;
>> @@ -30,7 +30,7 @@ fatal(const char *msg, ...)
>>       va_list         args;
>>
>>       va_start(args, msg);
>> -     fprintf(stderr, "%s: ", progname);
>> +     fprintf(stderr, "%s: ", mdrestore_progname);
>>       vfprintf(stderr, msg, args);
>>       exit(1);
>>  }
>> @@ -194,7 +194,7 @@ perform_restore(
>>  static void
>>  usage(void)
>>  {
>> -     fprintf(stderr, "Usage: %s [-V] [-g] source target\n", progname);
>> +     fprintf(stderr, "Usage: %s [-V] [-g] source target\n", mdrestore_progname);
>
> line > 80 cols, we try to avoid that
>
>>       exit(1);
>>  }
>>
>> @@ -212,7 +212,7 @@ main(
>>       struct stat     statbuf;
>>       int             is_target_file;
>>
>> -     progname = basename(argv[0]);
>> +     mdrestore_progname = basename(argv[0]);
>>
>>       while ((c = getopt(argc, argv, "giV")) != EOF) {
>>               switch (c) {
>> @@ -223,7 +223,7 @@ main(
>>                               show_info = 1;
>>                               break;
>>                       case 'V':
>> -                             printf("%s version %s\n", progname, VERSION);
>> +                             printf("%s version %s\n", mdrestore_progname, VERSION);
>
> 80 col
>
>>                               exit(0);
>>                       default:
>>                               usage();
>> diff --git a/quota/init.c b/quota/init.c
>> index d45dc4c..46403de 100644
>> --- a/quota/init.c
>> +++ b/quota/init.c
>> @@ -21,7 +21,7 @@
>>  #include "input.h"
>>  #include "init.h"
>>
>> -char *progname;
>> +char *quota_progname;
>>  int  exitcode;
>>  int  expert;
>>  bool foreign_allowed = false;
>> @@ -47,7 +47,7 @@ usage(void)
>>  {
>>       fprintf(stderr,
>>               _("Usage: %s [-V] [-x] [-f] [-p prog] [-c cmd]... [-d project]... [path]\n"),
>> -             progname);
>> +             quota_progname);
>>       exit(1);
>>  }
>>
>> @@ -147,7 +147,7 @@ init(
>>  {
>>       int             c;
>>
>> -     progname = basename(argv[0]);
>> +     quota_progname = basename(argv[0]);
>>       setlocale(LC_ALL, "");
>>       bindtextdomain(PACKAGE, LOCALEDIR);
>>       textdomain(PACKAGE);
>> @@ -173,13 +173,13 @@ init(
>>                       projid_file = optarg;
>>                       break;
>>               case 'p':
>> -                     progname = optarg;
>> +                     quota_progname = optarg;
>>                       break;
>>               case 'x':
>>                       expert++;
>>                       break;
>>               case 'V':
>> -                     printf(_("%s version %s\n"), progname, VERSION);
>> +                     printf(_("%s version %s\n"), quota_progname, VERSION);
>>                       exit(0);
>>               default:
>>                       usage();
>> diff --git a/spaceman/init.c b/spaceman/init.c
>> index b3eface..bedf112 100644
>> --- a/spaceman/init.c
>> +++ b/spaceman/init.c
>> @@ -23,7 +23,7 @@
>>  #include "path.h"
>>  #include "space.h"
>>
>> -char *progname;
>> +char *spaceman_progname;
>>  int  exitcode;
>>
>>  void
>> @@ -31,7 +31,7 @@ usage(void)
>>  {
>>       fprintf(stderr,
>>               _("Usage: %s [-c cmd] file\n"),
>> -             progname);
>> +             spaceman_progname);
>>       exit(1);
>>  }
>>
>> @@ -74,7 +74,7 @@ init(
>>       xfs_fsop_geom_t geometry = { 0 };
>>       struct fs_path  fsp;
>>
>> -     progname = basename(argv[0]);
>> +     spaceman_progname = basename(argv[0]);
>>       setlocale(LC_ALL, "");
>>       bindtextdomain(PACKAGE, LOCALEDIR);
>>       textdomain(PACKAGE);
>> @@ -86,7 +86,7 @@ init(
>>                       add_user_command(optarg);
>>                       break;
>>               case 'V':
>> -                     printf(_("%s version %s\n"), progname, VERSION);
>> +                     printf(_("%s version %s\n"), spaceman_progname, VERSION);
>
> 80 col
>
>>                       exit(0);
>>               default:
>>                       usage();
>>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Rename progname as it is provided by libc
  2017-09-08 16:25 ` Eric Sandeen
  2017-09-08 16:29   ` Khem Raj
@ 2017-09-08 18:13   ` Khem Raj
  1 sibling, 0 replies; 8+ messages in thread
From: Khem Raj @ 2017-09-08 18:13 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: linux-xfs

Hi Eric

On Fri, Sep 8, 2017 at 9:25 AM, Eric Sandeen <sandeen@sandeen.net> wrote:
> On 9/2/17 4:54 PM, Khem Raj wrote:
>> Rename local variable progname to avoid a clash with libc
>> global symbols
>>
>> Signed-off-by: Khem Raj <raj.khem@gmail.com>
>> ---
>>  io/init.c                 | 10 +++++-----
>>  mdrestore/xfs_mdrestore.c | 10 +++++-----
>>  quota/init.c              | 10 +++++-----
>>  spaceman/init.c           |  8 ++++----
>>  4 files changed, 19 insertions(+), 19 deletions(-)
>>
>
> First of all, this doesn't build.  Please don't send untested
> patches...

I am cross building with clang and for musl systems and the cross
build succeeds for me, I realized, I should have done native builds
too sorry about that
as discussed in previous replies, the symbols are conflicting from
local libraries in xfsprogs
unlike it coming from libc as I was thinking earlier.

I am building with --disable-static thats the root cause of issue I am
seeing because its finding two progname symbols
during link one from libxfs.so and another one from init.o when
linking the binary e.g. xfs_io I wonder if building with
--disable-static is valid/supported configuration
for the project or not.

here is error I was seeing
 /mnt/a/oe/build/tmp/work/cortexa7t2hf-neon-vfpv4-bec-linux-musleabi/xfsprogs/4.12.0-r0/recipe-sysroot-native/usr/bin/arm-bec-linux-musleabi/arm-bec-linux-musl
eabi-ld: xfs_mdrestore.o: relocation R_ARM_REL32 against external or
undefined symbol `progname' can not be used when making a PIE
executable; recompile with -f
PIC
| /mnt/a/oe/build/tmp/work/cortexa7t2hf-neon-vfpv4-bec-linux-musleabi/xfsprogs/4.12.0-r0/recipe-sysroot-native/usr/bin/arm-bec-linux-musleabi/arm-bec-linux-musl
eabi-ld: xfs_mdrestore.o(.text+0x64c): unresolvable R_ARM_REL32
relocation against symbol `progname'

shared libs e.g. libxfs.so are not built with -fPIC if I added -fPIC
to CFLAGS and now compiler is happy.

>
>     [LD]     xfs_io
> attr.o: In function `chattr_f':
> /mnt/test2/git/xfsprogs-maint/io/attr.c:316: undefined reference to `progname'
> /mnt/test2/git/xfsprogs-maint/io/attr.c:310: undefined reference to `progname'
> /mnt/test2/git/xfsprogs-maint/io/attr.c:332: undefined reference to `progname'
> /mnt/test2/git/xfsprogs-maint/io/attr.c:326: undefined reference to `progname'
> attr.o: In function `chattr_callback':
> /mnt/test2/git/xfsprogs-maint/io/attr.c:252: undefined reference to `progname'
> attr.o:/mnt/test2/git/xfsprogs-maint/io/attr.c:255: more undefined references to `progname' follow
> collect2: ld returned 1 exit status
> gmake[3]: *** [xfs_io] Error 1
> gmake[2]: *** [io] Error 2
> make[1]: *** [default] Error 2
> make: *** [default] Error 2
>
>
>
> second: is progname really a global symbol?  I know of __progname, but not progname.
>
> i.e. this works:
>
> #include <stdio.h>
>
> extern const char *__progname;
>
> int
> main(
>         int     argc,
>         char    **argv)
> {
>         printf("progname: %s\n", __progname);
> }
>
> # gcc -o test test.c
> # ./test
> progname: test
>
> but this doesn't:
>
> #include <stdio.h>
>
> extern const char *progname;
>
> int
> main(
>         int     argc,
>         char    **argv)
> {
>         printf("progname %s\n", progname);
> }
>
> # gcc -o test test.c
> /tmp/ccjaZrXZ.o: In function `main':
> test.c:(.text+0x12): undefined reference to `progname'
> collect2: ld returned 1 exit status
>
>
> -Eric
>
>> diff --git a/io/init.c b/io/init.c
>> index 20d5f80..e82e101 100644
>> --- a/io/init.c
>> +++ b/io/init.c
>> @@ -23,7 +23,7 @@
>>  #include "init.h"
>>  #include "io.h"
>>
>> -char *progname;
>> +char *io_progname;
>>  int  exitcode;
>>  int  expert;
>>  int  idlethread;
>> @@ -35,7 +35,7 @@ usage(void)
>>  {
>>       fprintf(stderr,
>>  _("Usage: %s [-adfinrRstVx] [-m mode] [-p prog] [[-c|-C] cmd]... file\n"),
>> -             progname);
>> +             io_progname);
>>       exit(1);
>>  }
>>
>> @@ -142,7 +142,7 @@ init(
>>       xfs_fsop_geom_t geometry = { 0 };
>>       struct fs_path  fsp;
>>
>> -     progname = basename(argv[0]);
>> +     io_progname = basename(argv[0]);
>>       setlocale(LC_ALL, "");
>>       bindtextdomain(PACKAGE, LOCALEDIR);
>>       textdomain(PACKAGE);
>> @@ -186,7 +186,7 @@ init(
>>                       flags |= IO_NONBLOCK;
>>                       break;
>>               case 'p':
>> -                     progname = optarg;
>> +                     io_progname = optarg;
>>                       break;
>>               case 'r':
>>                       flags |= IO_READONLY;
>> @@ -207,7 +207,7 @@ init(
>>                       expert = 1;
>>                       break;
>>               case 'V':
>> -                     printf(_("%s version %s\n"), progname, VERSION);
>> +                     printf(_("%s version %s\n"), io_progname, VERSION);
>>                       exit(0);
>>               default:
>>                       usage();
>> diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c
>> index 9d1b4e8..b840a54 100644
>> --- a/mdrestore/xfs_mdrestore.c
>> +++ b/mdrestore/xfs_mdrestore.c
>> @@ -19,7 +19,7 @@
>>  #include "libxfs.h"
>>  #include "xfs_metadump.h"
>>
>> -char                 *progname;
>> +char                 *mdrestore_progname;
>>  int          show_progress = 0;
>>  int          show_info = 0;
>>  int          progress_since_warning = 0;
>> @@ -30,7 +30,7 @@ fatal(const char *msg, ...)
>>       va_list         args;
>>
>>       va_start(args, msg);
>> -     fprintf(stderr, "%s: ", progname);
>> +     fprintf(stderr, "%s: ", mdrestore_progname);
>>       vfprintf(stderr, msg, args);
>>       exit(1);
>>  }
>> @@ -194,7 +194,7 @@ perform_restore(
>>  static void
>>  usage(void)
>>  {
>> -     fprintf(stderr, "Usage: %s [-V] [-g] source target\n", progname);
>> +     fprintf(stderr, "Usage: %s [-V] [-g] source target\n", mdrestore_progname);
>
> line > 80 cols, we try to avoid that
>
>>       exit(1);
>>  }
>>
>> @@ -212,7 +212,7 @@ main(
>>       struct stat     statbuf;
>>       int             is_target_file;
>>
>> -     progname = basename(argv[0]);
>> +     mdrestore_progname = basename(argv[0]);
>>
>>       while ((c = getopt(argc, argv, "giV")) != EOF) {
>>               switch (c) {
>> @@ -223,7 +223,7 @@ main(
>>                               show_info = 1;
>>                               break;
>>                       case 'V':
>> -                             printf("%s version %s\n", progname, VERSION);
>> +                             printf("%s version %s\n", mdrestore_progname, VERSION);
>
> 80 col
>
>>                               exit(0);
>>                       default:
>>                               usage();
>> diff --git a/quota/init.c b/quota/init.c
>> index d45dc4c..46403de 100644
>> --- a/quota/init.c
>> +++ b/quota/init.c
>> @@ -21,7 +21,7 @@
>>  #include "input.h"
>>  #include "init.h"
>>
>> -char *progname;
>> +char *quota_progname;
>>  int  exitcode;
>>  int  expert;
>>  bool foreign_allowed = false;
>> @@ -47,7 +47,7 @@ usage(void)
>>  {
>>       fprintf(stderr,
>>               _("Usage: %s [-V] [-x] [-f] [-p prog] [-c cmd]... [-d project]... [path]\n"),
>> -             progname);
>> +             quota_progname);
>>       exit(1);
>>  }
>>
>> @@ -147,7 +147,7 @@ init(
>>  {
>>       int             c;
>>
>> -     progname = basename(argv[0]);
>> +     quota_progname = basename(argv[0]);
>>       setlocale(LC_ALL, "");
>>       bindtextdomain(PACKAGE, LOCALEDIR);
>>       textdomain(PACKAGE);
>> @@ -173,13 +173,13 @@ init(
>>                       projid_file = optarg;
>>                       break;
>>               case 'p':
>> -                     progname = optarg;
>> +                     quota_progname = optarg;
>>                       break;
>>               case 'x':
>>                       expert++;
>>                       break;
>>               case 'V':
>> -                     printf(_("%s version %s\n"), progname, VERSION);
>> +                     printf(_("%s version %s\n"), quota_progname, VERSION);
>>                       exit(0);
>>               default:
>>                       usage();
>> diff --git a/spaceman/init.c b/spaceman/init.c
>> index b3eface..bedf112 100644
>> --- a/spaceman/init.c
>> +++ b/spaceman/init.c
>> @@ -23,7 +23,7 @@
>>  #include "path.h"
>>  #include "space.h"
>>
>> -char *progname;
>> +char *spaceman_progname;
>>  int  exitcode;
>>
>>  void
>> @@ -31,7 +31,7 @@ usage(void)
>>  {
>>       fprintf(stderr,
>>               _("Usage: %s [-c cmd] file\n"),
>> -             progname);
>> +             spaceman_progname);
>>       exit(1);
>>  }
>>
>> @@ -74,7 +74,7 @@ init(
>>       xfs_fsop_geom_t geometry = { 0 };
>>       struct fs_path  fsp;
>>
>> -     progname = basename(argv[0]);
>> +     spaceman_progname = basename(argv[0]);
>>       setlocale(LC_ALL, "");
>>       bindtextdomain(PACKAGE, LOCALEDIR);
>>       textdomain(PACKAGE);
>> @@ -86,7 +86,7 @@ init(
>>                       add_user_command(optarg);
>>                       break;
>>               case 'V':
>> -                     printf(_("%s version %s\n"), progname, VERSION);
>> +                     printf(_("%s version %s\n"), spaceman_progname, VERSION);
>
> 80 col
>
>>                       exit(0);
>>               default:
>>                       usage();
>>

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-09-08 18:14 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-02 21:54 [PATCH] Rename progname as it is provided by libc Khem Raj
2017-09-02 22:20 ` Dave Chinner
2017-09-02 23:42   ` Khem Raj
2017-09-03  7:21   ` Khem Raj
2017-09-03  7:48   ` Christoph Hellwig
2017-09-08 16:25 ` Eric Sandeen
2017-09-08 16:29   ` Khem Raj
2017-09-08 18:13   ` Khem Raj

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox