diff mbox

Rename strdup uses

Message ID AM5PR0802MB26109E6D6FC34D37773D89F483450@AM5PR0802MB2610.eurprd08.prod.outlook.com
State Superseded
Headers

Commit Message

Wilco Dijkstra Feb. 9, 2017, 10:36 p.m. UTC 
  Adhemerval Zanella wrote:
On 09/02/2017 13:17, Wilco Dijkstra wrote:
> > Rename existing uses of str(n)dup to __str(n)dup so it no longer needs to be
> > redirected to a builtin.  Also building GLIBC with -Os now no longer shows localplt
> > or linkname space failures (partial fix for BZ #15105 and BZ #19463).  Although this
> > means a loss of inlining (based on current committed headers) in 2 cases, these
> > are both error messages so not performance critical.
>
> I would prefer to just clump both this patch and the one that removes strdup
> inlines [1] together and then just remove the strdup and strndup macros
> on string/string.h.  Also, it seems that this patch is not really complete,
> since by removing the defines on string.h I see the missing spot that
> triggers PLT failures:

I think you must have mispasted my patch as it starts like this:

--
diff --git a/inet/rcmd.c b/inet/rcmd.c

> With this change we can just remove the str{n}dup macros on string.h and
> simplify this a lot.

I'll remove that chunk from my other patch then. I specifically did this search and replace as
a separate patch since it's much easier to review and get approval for minor clean-ups.

Wilco

Comments

Adhemerval Zanella Feb. 9, 2017, 10:44 p.m. UTC | #1 
On 09/02/2017 20:36, Wilco Dijkstra wrote:
> Adhemerval Zanella wrote:
> On 09/02/2017 13:17, Wilco Dijkstra wrote:
>>> Rename existing uses of str(n)dup to __str(n)dup so it no longer needs to be
>>> redirected to a builtin.  Also building GLIBC with -Os now no longer shows localplt
>>> or linkname space failures (partial fix for BZ #15105 and BZ #19463).  Although this
>>> means a loss of inlining (based on current committed headers) in 2 cases, these
>>> are both error messages so not performance critical.
>>
>> I would prefer to just clump both this patch and the one that removes strdup
>> inlines [1] together and then just remove the strdup and strndup macros
>> on string/string.h.  Also, it seems that this patch is not really complete,
>> since by removing the defines on string.h I see the missing spot that
>> triggers PLT failures:
> 
> I think you must have mispasted my patch as it starts like this:
> 
> --
> diff --git a/elf/dl-cache.c b/elf/dl-cache.c
> index cfa335eb326cd04f3892a09d4c00cef8208a2f33..4c8afd0684b96b44ec069f907b9537d4fc2ff6d2 100644
> --- a/elf/dl-cache.c
> +++ b/elf/dl-cache.c
> @@ -302,7 +302,7 @@ _dl_load_cache_lookup (const char *name)
>    char *temp;
>    temp = alloca (strlen (best) + 1);
>    strcpy (temp, best);
> -  return strdup (temp);
> +  return __strdup (temp);
>  }

Indeed, my bad here.

>  
>  #ifndef MAP_COPY
> diff --git a/inet/rcmd.c b/inet/rcmd.c
> 
>> With this change we can just remove the str{n}dup macros on string.h and
>> simplify this a lot.
> 
> I'll remove that chunk from my other patch then. I specifically did this search and replace as
> a separate patch since it's much easier to review and get approval for minor clean-ups.
> 
> Wilco
> 

Ah right, although imho for such coupled patchset splitting is more confusing
than a single one.
diff mbox

Patch

diff --git a/elf/dl-cache.c b/elf/dl-cache.c
index cfa335eb326cd04f3892a09d4c00cef8208a2f33..4c8afd0684b96b44ec069f907b9537d4fc2ff6d2 100644
--- a/elf/dl-cache.c
+++ b/elf/dl-cache.c
@@ -302,7 +302,7 @@  _dl_load_cache_lookup (const char *name)
   char *temp;
   temp = alloca (strlen (best) + 1);
   strcpy (temp, best);
-  return strdup (temp);
+  return __strdup (temp);
 }
 
 #ifndef MAP_COPY