| Message ID | 20160713121747.6F56A401AE80B@oldenburg.str.redhat.com |
|---|---|
| State | New |
| Headers | show |
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.
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
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).
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
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
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
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
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.
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
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
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}.
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
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
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
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 --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 */