This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Add STATIC_GETENV macro.
- From: "Carlos O'Donell" <carlos at redhat dot com>
- To: OndÅej BÃlka <neleai at seznam dot cz>
- Cc: libc-alpha at sourceware dot org
- Date: Sun, 10 Nov 2013 18:01:42 -0500
- Subject: Re: [PATCH] Add STATIC_GETENV macro.
- Authentication-results: sourceware.org; auth=none
- References: <20131109103822 dot GA9173 at domone> <527F3B35 dot 7050108 at redhat dot com> <20131110085500 dot GA5991 at domone>
On 11/10/2013 03:55 AM, OndÅej BÃlka wrote:
> On Sun, Nov 10, 2013 at 02:52:21AM -0500, Carlos O'Donell wrote:
>> On 11/09/2013 05:38 AM, OndÅej BÃlka wrote:
>>> Hi,
>>>
>>> This adds a STATIC_GETENV macro for avoiding possible getenv bottlenecks as
>>> documented in 'Make getenv O(1)' thread.
>>>
>>> As this is useful addition this could be a gnu extension.
>>>
>>> An patch that converts internal getenv uses to STATIC_GETENV will
>>> follow.
>>>
>>> There is question if we should make STATIC_GETENV call a secure_getenv
>>> by default or add STATIC_SECURE_GETENV.
>>>
>>> This depends on if getenv calls in next patch could be replaced by
>>> secure getenv or not.
>>>
>>> So far both choices are possible.
>>>
>>> * stdlib/stdlib.h (STATIC_GETENV): Add.
>>> * manual/startup.texi (Environment Access): Document STATIC_GETENV.
>>> * NEWS: Mention STATIC_GETENV.
>>
>> I like where this is going, but new functions like this will need
>> quite a bit of discussion, and it isn't clear to me what uses users
>> would use this where they wouldn't just cache the value themselves?
>>
> How many will? When writing code getenv/STATIC_GETENV choice is mostly
> automatic, doing caching less so.
>
> It involves adding static variable and choosing appropriate name, if
> conditional and if you take standard as seriously as Rich using atomic
> operations to set result.
>
> Also when one want to refactoring with codebase containing twenty
> getenvs then with STATIC_GETENV its running a script and checking which
> parts makes sense.
> As user it is first checking which parts make sense and then rewrite
> needed parts which is a repetitive task so when one looks how much he
> should do he gives up.
Let me think about this for a while.
I'm not happy with the name STATIC_GETENV, I wish we could express
the behaviour rather than implementation as part of the name, but
that's just my opinion e.g. GETENV_CACHED or something similar.
The other issue is that we must support users compiling programs
with much older compilers than we support building glibc. When
were the __sync* builtins added? Can you always rely on them?
I thought they were being deprecated in favour of the atomic
builtins? I think you need several ifdefs there to account for
various ages of compilers.
>> One of the goals here is that we don't turn this into a "everything
>> and the kitchen sink" library.
>>
> There is no risk of that, it already happened.
That made me laugh :-)
Cheers,
Carlos.