diff mbox

sln: Install as a hard link to ldconfig

Message ID 20160713121747.6F56A401AE80B@oldenburg.str.redhat.com
State New
Headers show

Commit Message

Florian Weimer July 13, 2016, 12:17 p.m. UTC
Implementing and sln and ldconfig with the same binary saves
around 850 KiB from a glibc installation.

The sln program is implicitly tested during the build, so no test
case is needed.

2016-07-13  Florian Weimer  <fweimer@redhat.com>

	Use the same binary for ldconfig and sln.
	* elf/sln.h: New file.
	* elf/sln.c (run_sln): New function.
	(PACKAGE): Remove.
	(sln_main): Renamed from main.
	* elf/ldconfig.c (main): Call sln_main if run_sln.
	* elf/Makefile (others): Remove sln.
	(others-static, install-rootsbin, sln-modules, extra-objs): Remove.
	(ldconfig-modules): Add sln.
	(install-others-programs): Likewise.
	(sln): Create as hard link to ldconfig.
	(/sbin/sln): Create as hard link to /sbin/ldconfig.

Comments

Andreas Schwab July 13, 2016, 12:42 p.m. UTC | #1
fweimer@redhat.com (Florian Weimer) writes:

> Implementing and sln and ldconfig with the same binary saves
> around 850 KiB from a glibc installation.

How about not installing it in the first place?

Andreas.
Florian Weimer July 13, 2016, 12:47 p.m. UTC | #2
On 07/13/2016 02:42 PM, Andreas Schwab wrote:
> fweimer@redhat.com (Florian Weimer) writes:
>
>> Implementing and sln and ldconfig with the same binary saves
>> around 850 KiB from a glibc installation.
>
> How about not installing it in the first place?

I assumed it was intended as a recovery tool if something is wrong with 
the DSO symbolic links, which is why I preserved it.  On the other hand, 
in this day and age, systems will certainly not boot if simple binaries 
like ln cannot run, and the system administrator will not be able to log 
in, so such recovery actions appear rather unrealistic without the help 
of a rescue system.

This leads to the next question: Do we still need to link ldconfig 
statically?

Florian
Joseph Myers July 20, 2016, 5:19 p.m. UTC | #3
On Wed, 13 Jul 2016, Florian Weimer wrote:

> I assumed it was intended as a recovery tool if something is wrong with the
> DSO symbolic links, which is why I preserved it.  On the other hand, in this
> day and age, systems will certainly not boot if simple binaries like ln cannot
> run, and the system administrator will not be able to log in, so such recovery
> actions appear rather unrealistic without the help of a rescue system.

I think sln / static ldconfig are recovery tools for *when already logged 
in* (and having just made a mistake with the links).
Jeff Law July 20, 2016, 5:20 p.m. UTC | #4
On 07/20/2016 11:19 AM, Joseph Myers wrote:
> On Wed, 13 Jul 2016, Florian Weimer wrote:
>
>> I assumed it was intended as a recovery tool if something is wrong with the
>> DSO symbolic links, which is why I preserved it.  On the other hand, in this
>> day and age, systems will certainly not boot if simple binaries like ln cannot
>> run, and the system administrator will not be able to log in, so such recovery
>> actions appear rather unrealistic without the help of a rescue system.
>
> I think sln / static ldconfig are recovery tools for *when already logged
> in* (and having just made a mistake with the links).
That would be my assumption as well -- and that's how I've used them in 
the past.
jeff
Aurelien Jarno July 20, 2016, 7:23 p.m. UTC | #5
On 2016-07-20 17:19, Joseph Myers wrote:
> On Wed, 13 Jul 2016, Florian Weimer wrote:
> 
> > I assumed it was intended as a recovery tool if something is wrong with the
> > DSO symbolic links, which is why I preserved it.  On the other hand, in this
> > day and age, systems will certainly not boot if simple binaries like ln cannot
> > run, and the system administrator will not be able to log in, so such recovery
> > actions appear rather unrealistic without the help of a rescue system.
> 
> I think sln / static ldconfig are recovery tools for *when already logged 
> in* (and having just made a mistake with the links).

I agree about that for ldconfig. Now I do wonder if it is the job of
the libc to provide sln, when alternatives like a statically linked
busybox can recover from many more breakages.

Aurelien
Mike Frysinger July 21, 2016, 6:18 a.m. UTC | #6
On 20 Jul 2016 21:23, Aurelien Jarno wrote:
> On 2016-07-20 17:19, Joseph Myers wrote:
> > On Wed, 13 Jul 2016, Florian Weimer wrote:
> > > I assumed it was intended as a recovery tool if something is wrong with the
> > > DSO symbolic links, which is why I preserved it.  On the other hand, in this
> > > day and age, systems will certainly not boot if simple binaries like ln cannot
> > > run, and the system administrator will not be able to log in, so such recovery
> > > actions appear rather unrealistic without the help of a rescue system.
> > 
> > I think sln / static ldconfig are recovery tools for *when already logged 
> > in* (and having just made a mistake with the links).
> 
> I agree about that for ldconfig. Now I do wonder if it is the job of
> the libc to provide sln, when alternatives like a statically linked
> busybox can recover from many more breakages.

agreed -- i think sln should die
-mike
Mike Frysinger July 21, 2016, 6:22 a.m. UTC | #7
On 13 Jul 2016 14:17, Florian Weimer wrote:
> +/* Check if we have to run sln.  */
> +bool
> +run_sln (const char *argv0)
> +{
> +  const char *slash = strrchr (argv0, '/');
> +  const char *progname;
> +  if (slash == NULL)
> +    progname = argv0;
> +  else
> +    progname = slash + 1;
> +  return strcmp (progname, "sln") == 0;
> +}

GNU programming conventions say to not rely on argv[0] to change
behavior:
	https://www.gnu.org/prep/standards/html_node/_002d_002dversion.html

as a practical example, it is trivial for argv[0] to be anything
(including NULL) which would confuse this test.

at the very least, you should use program_invocation_short_name
instead of parsing argv[0] yourself.  not that that addresses
the concern above since that internally is based on argv[0].
-mike
Adhemerval Zanella Netto July 21, 2016, 12:36 p.m. UTC | #8
On 21/07/2016 03:18, Mike Frysinger wrote:
> On 20 Jul 2016 21:23, Aurelien Jarno wrote:
>> On 2016-07-20 17:19, Joseph Myers wrote:
>>> On Wed, 13 Jul 2016, Florian Weimer wrote:
>>>> I assumed it was intended as a recovery tool if something is wrong with the
>>>> DSO symbolic links, which is why I preserved it.  On the other hand, in this
>>>> day and age, systems will certainly not boot if simple binaries like ln cannot
>>>> run, and the system administrator will not be able to log in, so such recovery
>>>> actions appear rather unrealistic without the help of a rescue system.
>>>
>>> I think sln / static ldconfig are recovery tools for *when already logged 
>>> in* (and having just made a mistake with the links).
>>
>> I agree about that for ldconfig. Now I do wonder if it is the job of
>> the libc to provide sln, when alternatives like a statically linked
>> busybox can recover from many more breakages.
> 
> agreed -- i think sln should die
> -mike
> 

Same, nowadays distros can provide better alternatives.
Maciej W. Rozycki July 26, 2016, 11:33 p.m. UTC | #9
On Wed, 13 Jul 2016, Florian Weimer wrote:

> I assumed it was intended as a recovery tool if something is wrong with the
> DSO symbolic links, which is why I preserved it.  On the other hand, in this
> day and age, systems will certainly not boot if simple binaries like ln cannot
> run, and the system administrator will not be able to log in, so such recovery
> actions appear rather unrealistic without the help of a rescue system.

 Why?

 E.g. booting Linux with `rw init=/bin/bash.static' has always worked for 
me, with the root filesystem mounted r/w and ready for any recovery 
actions, such as fixing up DSO symlinks.  And I see no reason for it to 
stop working even where an initial RAM disk is used, as an image of such a 
RAM disk is always self-contained and not used beyond early (pre-init) 
initialisation needed to pull the root and maybe console device drivers.

 Have I missed anything?

 This is to get the facts straight that is -- whether to keep or discard 
`sln' is of course another matter.

  Maciej
Florian Weimer Aug. 2, 2016, 10:04 a.m. UTC | #10
On 07/21/2016 08:22 AM, Mike Frysinger wrote:
> On 13 Jul 2016 14:17, Florian Weimer wrote:
>> +/* Check if we have to run sln.  */
>> +bool
>> +run_sln (const char *argv0)
>> +{
>> +  const char *slash = strrchr (argv0, '/');
>> +  const char *progname;
>> +  if (slash == NULL)
>> +    progname = argv0;
>> +  else
>> +    progname = slash + 1;
>> +  return strcmp (progname, "sln") == 0;
>> +}
>
> GNU programming conventions say to not rely on argv[0] to change
> behavior:

> 	https://www.gnu.org/prep/standards/html_node/_002d_002dversion.html

Can you provide an exact quote?  I don't see it.  coreutils supports 
this, and bash pretty much requires looking at argv[0].

Thanks,
Florian
Joseph Myers Aug. 2, 2016, 3:49 p.m. UTC | #11
On Tue, 2 Aug 2016, Florian Weimer wrote:

> Can you provide an exact quote?  I don't see it.  coreutils supports this, and
> bash pretty much requires looking at argv[0].

There are at least two relevant quotes in standards.texi:

  @node User Interfaces
  @section Standards for Interfaces Generally

  @cindex program name and its behavior
  @cindex behavior, dependent on program's name
  Please don't make the behavior of a utility depend on the name used
  to invoke it.  It is useful sometimes to make a link to a utility
  with a different name, and that should not change what it does.

  Instead, use a run time option or a compilation switch or both to
  select among the alternate behaviors.  You can also build two versions
  of the program, with different names and different default behaviors.

and, for --version:

  The program's name should be a constant string; @emph{don't} compute it
  from @code{argv[0]}.  The idea is to state the standard or canonical
  name for the program, not its file name.  There are other ways to find
  out the precise file name where a command is found in @code{PATH}.
Mike Frysinger Aug. 2, 2016, 4:19 p.m. UTC | #12
On 01 Aug 2016 15:37, Maciej W. Rozycki wrote:
> On Mon, 1 Aug 2016, Mike Frysinger wrote:
> > >  E.g. booting Linux with `rw init=/bin/bash.static' has always worked for 
> > > me, with the root filesystem mounted r/w and ready for any recovery 
> > > actions, such as fixing up DSO symlinks.  And I see no reason for it to 
> > > stop working even where an initial RAM disk is used, as an image of such a 
> > > RAM disk is always self-contained and not used beyond early (pre-init) 
> > > initialisation needed to pull the root and maybe console device drivers.
> > 
> > bash.static is just as much of a distro-specific convention.  i haven't
> > heard of this before, and Debian/Ubuntu/Gentoo don't have it.
> 
>  I find it a bit surprising for a general-purpose distribution not to 
> provide a static shell by default, but this is as you say a policy set by 
> individual distributions.  I would feel a bit nervous if I were a system 
> administrator and didn't have a static shell around on a system I was 
> taking care of.

i agree, but i don't think it's something we need to support.  having just
a static (bash) shell doesn't get you very far -- you've got a few bash
builtins and that's it.  trying to actually recover from things often need
far more commands.  sln only helps you with creating symlinks (e.g. like
resetting glibc links, assuming it's a glibc install error).  i think that
special casing this one scenario doesn't make sense nowadays.

in Gentoo, we ship a build of busybox with a lot of built-in commands.
you can recover form a wide range of issues, set up the network, fetch
files over the network, and decompress/unpack archives.
-mike
Mike Frysinger Aug. 2, 2016, 4:27 p.m. UTC | #13
On 02 Aug 2016 12:04, Florian Weimer wrote:
> On 07/21/2016 08:22 AM, Mike Frysinger wrote:
> > On 13 Jul 2016 14:17, Florian Weimer wrote:
> >> +/* Check if we have to run sln.  */
> >> +bool
> >> +run_sln (const char *argv0)
> >> +{
> >> +  const char *slash = strrchr (argv0, '/');
> >> +  const char *progname;
> >> +  if (slash == NULL)
> >> +    progname = argv0;
> >> +  else
> >> +    progname = slash + 1;
> >> +  return strcmp (progname, "sln") == 0;
> >> +}
> >
> > GNU programming conventions say to not rely on argv[0] to change
> > behavior:
> 
> > 	https://www.gnu.org/prep/standards/html_node/_002d_002dversion.html
> 
> Can you provide an exact quote?  I don't see it.

i was looking at this part:
  The program’s name should be a constant string; don’t compute it from argv[0].

but Jim's quotes are more relevant in this setup

> coreutils supports this,

i don't think that's all that accurate.  i'm assuming you're referring to
the "coreutils" multicall binary (which i was involved with merging).  it
was merged as a concession to many alternative projects (e.g. busybox),
with specific embedded scenarios in mind, is not the default build mode,
and it includes a configure option to install small scripts rather than
symlinks specifically to deal with the invocation issue (and avoiding the
use of argv[0]).  afaik, coreutils doesn't use argv[0] in other places.

> and bash pretty much requires looking at argv[0].

not really ... while it does have historical support for checking a "-"
prefix and changing its behavior based on that, it is not a requirement.
it works just fine with flags like --login and is the preferred method.

don't get me wrong -- personally, i often use argv[0] in multicall progs
and require proper symlinks to dtrt.  but GNU projects are strongly
discouraged from doing so, and glibc is a GNU project.
-mike
Maciej W. Rozycki Aug. 2, 2016, 4:28 p.m. UTC | #14
On Tue, 2 Aug 2016, Mike Frysinger wrote:

> i agree, but i don't think it's something we need to support.  having just
> a static (bash) shell doesn't get you very far -- you've got a few bash
> builtins and that's it.  trying to actually recover from things often need
> far more commands.  sln only helps you with creating symlinks (e.g. like
> resetting glibc links, assuming it's a glibc install error).  i think that
> special casing this one scenario doesn't make sense nowadays.

 FAOD, I wrote:

>  This is to get the facts straight that is -- whether to keep or discard
> `sln' is of course another matter.

which I think clarifies my statement was not meant to support the 
retaining of `sln', but just to clear any confusion there might be about 
ways of recovery provided by Linux.

> in Gentoo, we ship a build of busybox with a lot of built-in commands.
> you can recover form a wide range of issues, set up the network, fetch
> files over the network, and decompress/unpack archives.

 Coincidentally I thought it would be a good idea to have `busybox' as the 
static shell, so I find it nice that you have thought about it too.

  Maciej
Mike Frysinger Aug. 2, 2016, 5:23 p.m. UTC | #15
On 02 Aug 2016 17:28, Maciej W. Rozycki wrote:
> On Tue, 2 Aug 2016, Mike Frysinger wrote:
> > i agree, but i don't think it's something we need to support.  having just
> > a static (bash) shell doesn't get you very far -- you've got a few bash
> > builtins and that's it.  trying to actually recover from things often need
> > far more commands.  sln only helps you with creating symlinks (e.g. like
> > resetting glibc links, assuming it's a glibc install error).  i think that
> > special casing this one scenario doesn't make sense nowadays.
> 
>  FAOD, I wrote:
> 
> >  This is to get the facts straight that is -- whether to keep or discard
> > `sln' is of course another matter.
> 
> which I think clarifies my statement was not meant to support the 
> retaining of `sln', but just to clear any confusion there might be about 
> ways of recovery provided by Linux.

sorry, i misread that addendum to your first post.  it wasn't clear to me
that was the route you were going.
-mike
diff mbox

Patch

diff --git a/elf/Makefile b/elf/Makefile
index 593403c..38ef328 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -70,12 +70,8 @@  install-others	= $(inst_rtlddir)/$(rtld-installed-name)
 install-bin-script = ldd
 endif
 
-others		= sprof sln
+others		= sprof
 install-bin	= sprof
-others-static   = sln
-install-rootsbin = sln
-sln-modules	:= static-stubs
-extra-objs	+= $(sln-modules:=.o)
 
 ifeq (yes,$(use-ldconfig))
 ifeq (yes,$(build-shared))
@@ -83,8 +79,15 @@  others-static	+= ldconfig
 others		+= ldconfig
 install-rootsbin += ldconfig
 
-ldconfig-modules := cache readlib xmalloc xstrdup chroot_canon static-stubs
+ldconfig-modules := cache readlib xmalloc xstrdup chroot_canon static-stubs sln
 extra-objs	+= $(ldconfig-modules:=.o)
+
+# Install sln as a hard link to ldconfig.
+install-others-programs += $(inst_rootsbindir)/sln
+$(objpfx)sln: $(objpfx)ldconfig
+	ln -f $< $@
+$(inst_rootsbindir)/sln: $(inst_rootsbindir)/ldconfig
+	ln -f $< $@
 endif
 endif
 
@@ -466,8 +469,6 @@  $(objpfx)ldd: ldd.bash.in $(common-objpfx)soversions.mk \
 
 $(objpfx)sprof: $(libdl)
 
-$(objpfx)sln: $(sln-modules:%=$(objpfx)%.o)
-
 $(objpfx)ldconfig: $(ldconfig-modules:%=$(objpfx)%.o)
 
 SYSCONF-FLAGS := -D'SYSCONFDIR="$(sysconfdir)"'
diff --git a/elf/ldconfig.c b/elf/ldconfig.c
index 467ca82..972737c 100644
--- a/elf/ldconfig.c
+++ b/elf/ldconfig.c
@@ -44,6 +44,8 @@ 
 
 #include <dl-procinfo.h>
 
+#include "sln.h"
+
 #ifdef _DL_FIRST_PLATFORM
 # define _DL_FIRST_EXTRA (_DL_FIRST_PLATFORM + _DL_PLATFORMS_COUNT)
 #else
@@ -1275,6 +1277,9 @@  main (int argc, char **argv)
   /* Set the text message domain.  */
   textdomain (_libc_intl_domainname);
 
+  if (run_sln (argv[0]))
+    return sln_main (argc, argv);
+
   /* Parse and process arguments.  */
   int remaining;
   argp_parse (&argp, argc, argv, 0, &remaining, NULL);
diff --git a/elf/sln.c b/elf/sln.c
index fa4ccec..c6889d7 100644
--- a/elf/sln.c
+++ b/elf/sln.c
@@ -1,4 +1,4 @@ 
-/* `sln' program to create symbolic links between files.
+/* sln helper to create symbolic links between files, invoked from ldconfig.
    Copyright (C) 1998-2016 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
@@ -31,21 +31,29 @@ 
 
 #include "../version.h"
 
-#define PACKAGE _libc_intl_domainname
+#include "sln.h"
 
 static int makesymlink (const char *src, const char *dest);
 static int makesymlinks (const char *file);
 static void usage (void);
 
+/* Check if we have to run sln.  */
+bool
+run_sln (const char *argv0)
+{
+  const char *slash = strrchr (argv0, '/');
+  const char *progname;
+  if (slash == NULL)
+    progname = argv0;
+  else
+    progname = slash + 1;
+  return strcmp (progname, "sln") == 0;
+}
+
+/* Invoked from ldconfig.  */
 int
-main (int argc, char **argv)
+sln_main (int argc, char **argv)
 {
-  /* Set locale via LC_ALL.  */
-  setlocale (LC_ALL, "");
-
-  /* Set the text message domain.  */
-  textdomain (PACKAGE);
-
   switch (argc)
     {
     case 2:
diff --git a/elf/sln.h b/elf/sln.h
new file mode 100644
index 0000000..a3a16ab
--- /dev/null
+++ b/elf/sln.h
@@ -0,0 +1,30 @@ 
+/* Interface of the sln command-line tool.
+   Copyright (C) 2016 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef SLN_H
+#define SLN_H
+
+#include <stdbool.h>
+
+/* Return true if main should invoke sln_main.  */
+bool run_sln (const char *argv0);
+
+/* Main routine of the sln command.  */
+int sln_main (int argc, char **argv);
+
+#endif  /* SLN_H */