This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Free errstring if _dl_addr returns 0
- From: "H.J. Lu" <hjl dot tools at gmail dot com>
- To: Roland McGrath <roland at hack dot frob dot com>
- Cc: GNU C Library <libc-alpha at sourceware dot org>
- Date: Tue, 9 Oct 2012 14:41:52 -0700
- Subject: Re: [PATCH] Free errstring if _dl_addr returns 0
- References: <20120907184737.GA14053@intel.com><20120907202424.CB3C72C095@topped-with-meat.com><CAMe9rOr3LGvtUjDpEA0nUmkWvF_ewYCwe4qJ9w-GORMR68WRkg@mail.gmail.com><20120907231329.A383A2C0CB@topped-with-meat.com><CAMe9rOryFB9QOXCevoSRG8A1zNgUu_stAASayPfJb6EX6Yy-ww@mail.gmail.com>
On Sat, Sep 8, 2012 at 7:24 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Fri, Sep 7, 2012 at 4:13 PM, Roland McGrath <roland@hack.frob.com> wrote:
>>> -- * dlfcn/dlerror.c (check_free): Free errstring if _dl_addr
>>> returns 0. Don't check if map is NULL.
>>
>> MAP in caps.
>>
>>> - always the C library in the base namespave. */
>>> + always the C library in the base namespace. _dl_addr retuns 0
>>
>> Typo: "returns"
>>
>>> + when check_free isn't called from any loaded object. This
>>> + indicates static executable and we can free the string. */
>>
>> "This indicates we're in a static executable, in which case it's always
>> safe to free the string."
>>
>> However, it occurs to me that we probably want a sanity check here. That
>> is, if _dl_addr returns 0 for some unexpected reason, we should not call
>> free. It should never be possible #ifdef SHARED, right?
>>
>> Hmm. Why do this test dynamically rather than simply use #ifndef SHARED?
>> i.e.:
>> /* We can free the string only if the allocation happened in the
>> C library used by the dynamic linker. This means, it is
>> always the C library in the base namespace. When we're statically
>> linked, the dynamic linker is part of the program and so always
>> uses the same C library we use here. */
>> #ifndef SHARED
> ^^^^^^^^^^^ A typo.
>
>> struct link_map *map = NULL;
>> Dl_info info;
>> if (_dl_addr (check_free, &info, &map, NULL) != 0 && map->l_ns == 0)
>> #endif
>> free ((char *) rec->errstring);
>>
>> Is there any case where that would be wrong?
>>
>>
>
> This works for me. Is this OK?
>
> Thanks.
>
> --
> H.J.
> ----
> 012-09-05 Roland McGrath <roland@hack.frob.com>
>
> * dlfcn/dlerror.c (check_free): Call _dl_addr only if SHARED is
> defined. Don't check if MAP is NULL.
>
> diff --git a/dlfcn/dlerror.c b/dlfcn/dlerror.c
> index 8138cc2..7597703 100644
> --- a/dlfcn/dlerror.c
> +++ b/dlfcn/dlerror.c
> @@ -190,11 +190,14 @@ check_free (struct dl_action_result *rec)
> {
> /* We can free the string only if the allocation happened in the
> C library used by the dynamic linker. This means, it is
> - always the C library in the base namespave. */
> + always the C library in the base namespace. When we're statically
> + linked, the dynamic linker is part of the program and so always
> + uses the same C library we use here. */
> +#ifdef SHARED
> struct link_map *map = NULL;
> Dl_info info;
> - if (_dl_addr (check_free, &info, &map, NULL) != 0
> - && map != NULL && map->l_ns == 0)
> + if (_dl_addr (check_free, &info, &map, NULL) != 0 && map->l_ns == 0)
> +#endif
> free ((char *) rec->errstring);
> }
> }
PING.
--
H.J.