4 ms·

Call getenv() in a loop and you run out of memory.

Sprocklem 3y ago
I was suggesting that the buffer be invalidated by each subsequent call – like some other libc functions' internal buffers – although, as I noted in the edit this would need `getenv()` to be able to indicate errors (specifically ENOMEM). It currently cannot do this as currently described, because NULL is used to indicate an absent variable. You could also require callers free the returned memory when they're done, but that would be another change of API.
- alexey-salmin 3y ago
  > I was suggesting that the buffer be invalidated by each subsequent call This would break very simple single-threaded programs that e.g. print two env vars in one printf call.
- lokar 3y ago
  The solution to all problems like this was decided years ago: _r You provide the storage and free it The problem is these non-direct uses. They each need to switch to •_r and manage the buffer, or offer _r versions themselves and sort of pass through the problem
  - Sprocklem 3y ago
    Of course, *_r is a better option, but the existing API is used so pervasively that it needs to be made thread-safe to actually avoid thread-unsafe code in, e.g, libraries.
    - lokar 3y ago
      I don’t see how you can make: - return a pointer - the library owns the allocation - the state is global and mutable Thread safe
      - Sprocklem 3y ago
        A number of libc functions return a pointer to an internal thread-local buffer, which is invalidated on subsequent calls. If the function copies the environment variable's value to such a buffer while holding the mutex controlling access to the global state, then the returned value is guaranteed to remain unaffected by other threads. There are, however, other problems (discussed elsewhere in this thread) that complicate such an API in the context of getenv().
        lokar 3y ago
        Requiring you to hold a mutex to safely call the function is an API change
        Sprocklem 3y ago
        I was not suggesting the caller hold a mutex, but rather getenv(), which eould be transparent to the caller.
        lokar 3y ago
        Don’t you need C++11?
harerazer 3y ago
Then don't do that. Of all the footguns in POSIX/C programming, having to remember to free this is really not as bad as you seem to imply.
- thayne 3y ago
  And how would you free it? The current posix API doesn't have any way to reliably free the result returned by `getenv`.
- alexey-salmin 3y ago
  You miss the point. If you have full control over when and how getenv is called, there's no issue to begin with. The problem is that you don't, as OP demonstrates. It's perfectly natural to call getaddrinfo in a loop. We need a new API which is not broken like in NetBSD, and a multi-year migration of all core libraries to it. Well a pity it wasn't started years ago though, could've been 95% done by now.

I was suggesting that the buffer be invalidated by each subsequent call – like some other libc functions' internal buffers – although, as I noted in the edit this would need `getenv()` to be able to indicate errors (specifically ENOMEM). It currently cannot do this as currently described, because NULL is used to indicate an absent variable. You could also require callers free the returned memory when they're done, but that would be another change of API.

alexey-salmin 3y ago
> I was suggesting that the buffer be invalidated by each subsequent call This would break very simple single-threaded programs that e.g. print two env vars in one printf call.
lokar 3y ago
The solution to all problems like this was decided years ago: _r You provide the storage and free it The problem is these non-direct uses. They each need to switch to •_r and manage the buffer, or offer _r versions themselves and sort of pass through the problem
- Sprocklem 3y ago
  Of course, *_r is a better option, but the existing API is used so pervasively that it needs to be made thread-safe to actually avoid thread-unsafe code in, e.g, libraries.
  - lokar 3y ago
    I don’t see how you can make: - return a pointer - the library owns the allocation - the state is global and mutable Thread safe
    - Sprocklem 3y ago
      A number of libc functions return a pointer to an internal thread-local buffer, which is invalidated on subsequent calls. If the function copies the environment variable's value to such a buffer while holding the mutex controlling access to the global state, then the returned value is guaranteed to remain unaffected by other threads. There are, however, other problems (discussed elsewhere in this thread) that complicate such an API in the context of getenv().
      - lokar 3y ago
        Requiring you to hold a mutex to safely call the function is an API change
        Sprocklem 3y ago
        I was not suggesting the caller hold a mutex, but rather getenv(), which eould be transparent to the caller.
        lokar 3y ago
        Don’t you need C++11?

harerazer 3y ago

Then don't do that. Of all the footguns in POSIX/C programming, having to remember to free this is really not as bad as you seem to imply.

thayne 3y ago
And how would you free it? The current posix API doesn't have any way to reliably free the result returned by `getenv`.
alexey-salmin 3y ago
You miss the point. If you have full control over when and how getenv is called, there's no issue to begin with. The problem is that you don't, as OP demonstrates. It's perfectly natural to call getaddrinfo in a loop. We need a new API which is not broken like in NetBSD, and a multi-year migration of all core libraries to it. Well a pity it wasn't started years ago though, could've been 95% done by now.