This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] elf/dl-load.c: Remove local_strdup.
- From: "Carlos O'Donell" <carlos at redhat dot com>
- To: GNU C Library <libc-alpha at sourceware dot org>
- Date: Wed, 29 Oct 2014 16:23:43 -0400
- Subject: Re: [PATCH] elf/dl-load.c: Remove local_strdup.
- Authentication-results: sourceware.org; auth=none
- References: <5449C26B dot 4070600 at redhat dot com>
On 10/23/2014 11:07 PM, Carlos O'Donell wrote:
> This patch removes the apparently superfluous local_strdup.
> I can find no reason for it to be here and the reason for
> it being there is not documented.
>
> It has been present since 1996-08-15 in a patch that simply
> refactored strlen/malloc/memcpy code into local_strdup without
> changing anything. Probably a conservative change. Why strdup
> was not called is not clear.
>
> I have verified that building dl-load.c uses the __strdup
> alias so the loader is self-consistent.
>
> Tested on x86_64 with no regressions.
>
> OK to commit?
v2
- Call __strdup
I checked this in.
elf/dl-load.c: Use __strdup.
During a refactoring pass several repeated blocks of code in dl-load.c
were turned into a call to a local function named local_strdup. There
is no need for local_strdup, and the routines should instead call
__strdup. This change does just that. We call the internal symbol
__strdup because calling strdup is unsafe. The user might be
using a standard that doesn't include strdup and may have defined this
symbol in their application. During a static link we might reference
the user defined symbol and crash if it doesn't implement a standards
conforming strdup. The resulting code is simpler to understand, and
makes it easier to debug.
No regressions on x86_64.
2014-10-28 Carlos O'Donell <carlos@redhat.com>
* dl-load.c (local_strdup): Remove.
(expand_dynamic_string_token): Use __strdup.
(decompose_rpath): Likewise.
(_dl_map_object): Likewise.
a/ChangeLog b/ChangeLog
index 5c7bef3..6874e54 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2014-10-28 Carlos O'Donell <carlos@redhat.com>
+
+ * dl-load.c (local_strdup): Remove.
+ (expand_dynamic_string_token): Use __strdup.
+ (decompose_rpath): Likewise.
+ (_dl_map_object): Likewise.
+
2014-10-28 Joseph Myers <joseph@codesourcery.com>
[BZ #14132]
diff --git a/elf/dl-load.c b/elf/dl-load.c
index 9dd40e3..ce5b626 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -112,20 +112,6 @@ static const size_t system_dirs_len[] =
(sizeof (system_dirs_len) / sizeof (system_dirs_len[0]))
-/* Local version of `strdup' function. */
-static char *
-local_strdup (const char *s)
-{
- size_t len = strlen (s) + 1;
- void *new = malloc (len);
-
- if (new == NULL)
- return NULL;
-
- return (char *) memcpy (new, s, len);
-}
-
-
static bool
is_trusted_path (const char *path, size_t len)
{
@@ -384,7 +370,7 @@ expand_dynamic_string_token (struct link_map *l, const char *s, int is_path)
/* If we do not have to replace anything simply copy the string. */
if (__glibc_likely (cnt == 0))
- return local_strdup (s);
+ return __strdup (s);
/* Determine the length of the substituted string. */
total = DL_DST_REQUIRED (l, s, strlen (s), cnt);
@@ -593,7 +579,7 @@ decompose_rpath (struct r_search_path_struct *sps,
}
/* Make a writable copy. */
- copy = local_strdup (rpath);
+ copy = __strdup (rpath);
if (copy == NULL)
{
errstring = N_("cannot create RUNPATH/RPATH copy");
@@ -2101,7 +2087,7 @@ _dl_map_object (struct link_map *loader, const char *name,
false);
if (__glibc_likely (fd != -1))
{
- realname = local_strdup (cached);
+ realname = __strdup (cached);
if (realname == NULL)
{
__close (fd);
@@ -2130,7 +2116,7 @@ _dl_map_object (struct link_map *loader, const char *name,
/* The path may contain dynamic string tokens. */
realname = (loader
? expand_dynamic_string_token (loader, name, 0)
- : local_strdup (name));
+ : __strdup (name));
if (realname == NULL)
fd = -1;
else
@@ -2164,7 +2150,7 @@ _dl_map_object (struct link_map *loader, const char *name,
static const Elf_Symndx dummy_bucket = STN_UNDEF;
/* Allocate a new object map. */
- if ((name_copy = local_strdup (name)) == NULL
+ if ((name_copy = __strdup (name)) == NULL
|| (l = _dl_new_object (name_copy, name, type, loader,
mode, nsid)) == NULL)
{
---
Cheers,
Carlos.