4 ms·

Here's my proposal: Introduce a new threadsafe API (`tgetenv` or whatever) which takes _two_ `char *`s, one of which is a return buffer. This leaves allocation

by cnity 3y ago

Here's my proposal: Introduce a new threadsafe API (`tgetenv` or whatever) which takes _two_ `char *`s, one of which is a return buffer. This leaves allocation as a responsibility of the caller. And then you can leave the existing syscalls as they are (thread unsafe) while having a separate thread safe version.

DSMan195276 3y ago
I agree that would be the way to do it, but now we're no longer talking about simply 'fixing' the implementation of the existing API but rather introducing a new function you have to use. `setenv()` would only be safe if your program never uses `getenv()`, and calls to `getenv()` are so numerous and all over the place that for most non-trivial programs it would be hard to ensure they never happen. There's also the rub that `setenv()` is not part of the C standard, it's POSIX. I don't think the C standard would ever introduce `tgetenv()` to fix a problem it doesn't have, so non-POSIX code would have to continue to call `getenv()` since that's all that is available to them.
- leoh 3y ago
  >you have to use I mean, why not just deprecate the old one; add a warning if it’s used
  - DSMan195276 3y ago
    That doesn't really help you determine whether a given library is using `getenv()` or not. That also requires that things are actually recompiled/updated, which for some C libraries is not that often. There's also the rub that many C programs do not target the latest standard (for a variety of reasons). I didn't realize `getenv_s` was added in C11 (though it's optional), but it doesn't really matter because programs/libraries that target C89 or C99 can't use it anyway.
    - leoh 3y ago
      That’s a good point. I would guess there are ways of doing static analysis to see if a given binary is making getenv() calls though, even if one doesn’t fully grok its source. Maybe some combo of that with sentenv() in your source or something Or do “live” analysis under integration and give a low priority warning But yeah it’s hairy, you’re right
- josefx 3y ago
  > I don't think the C standard would ever introduce `tgetenv()` to fix a problem it doesn't have The C standard has no problem acknowledging that getenv is subject to data races for most of its implementations. As far as I can tell that part was even added at the same time as threading support.
  - DSMan195276 3y ago
    Well actually I'll have to eat my words on that one - I didn't catch that Annex K in C11 includes `getenv_s` (even if it is optional).
salawat 3y ago
GNU convention is <funcname>_r for a reentrant version of a non reentrant function. I'm in the process of working on a tool in C at the moment, so for once I actually have some context on what's being grumped about here!
JohnFen 3y ago
Entirely this. It works so well that I've seen this in various utility libraries for decades.
- cnity 3y ago
  Yeah. Most of my code these days works like this, to the point where almost all of my allocation happens roughly in my main entry-point. Or I use arena allocators and pass those into my utilities. But there's basically never a `realloc` or `malloc` call deep in my code anymore.

by cnity 3y ago

DSMan195276 3y ago

I agree that would be the way to do it, but now we're no longer talking about simply 'fixing' the implementation of the existing API but rather introducing a new function you have to use. `setenv()` would only be safe if your program never uses `getenv()`, and calls to `getenv()` are so numerous and all over the place that for most non-trivial programs it would be hard to ensure they never happen. There's also the rub that `setenv()` is not part of the C standard, it's POSIX. I don't think the C standard would ever introduce `tgetenv()` to fix a problem it doesn't have, so non-POSIX code would have to continue to call `getenv()` since that's all that is available to them.

leoh 3y ago
>you have to use I mean, why not just deprecate the old one; add a warning if it’s used
- DSMan195276 3y ago
  That doesn't really help you determine whether a given library is using `getenv()` or not. That also requires that things are actually recompiled/updated, which for some C libraries is not that often. There's also the rub that many C programs do not target the latest standard (for a variety of reasons). I didn't realize `getenv_s` was added in C11 (though it's optional), but it doesn't really matter because programs/libraries that target C89 or C99 can't use it anyway.
  - leoh 3y ago
    That’s a good point. I would guess there are ways of doing static analysis to see if a given binary is making getenv() calls though, even if one doesn’t fully grok its source. Maybe some combo of that with sentenv() in your source or something Or do “live” analysis under integration and give a low priority warning But yeah it’s hairy, you’re right
josefx 3y ago
> I don't think the C standard would ever introduce `tgetenv()` to fix a problem it doesn't have The C standard has no problem acknowledging that getenv is subject to data races for most of its implementations. As far as I can tell that part was even added at the same time as threading support.
- DSMan195276 3y ago
  Well actually I'll have to eat my words on that one - I didn't catch that Annex K in C11 includes `getenv_s` (even if it is optional).

salawat 3y ago

GNU convention is <funcname>_r for a reentrant version of a non reentrant function. I'm in the process of working on a tool in C at the moment, so for once I actually have some context on what's being grumped about here!

JohnFen 3y ago

Entirely this. It works so well that I've seen this in various utility libraries for decades.

cnity 3y ago
Yeah. Most of my code these days works like this, to the point where almost all of my allocation happens roughly in my main entry-point. Or I use arena allocators and pass those into my utilities. But there's basically never a `realloc` or `malloc` call deep in my code anymore.