[flang-commits] [PATCH] D111785: [flang] runtime: Read environment variables directly

[Peter Klausler via Phabricator via flang-commits]

klausler added a comment.

In D111785#3083551 <https://reviews.llvm.org/D111785#3083551>, @rovka wrote:

> Updated to crash if we fail to allocate the c-style string. Introducing the crash behavior made NullTerminatedString look pretty much the same as SaveDefaultCharacter, so I used that one instead and completely removed the former, along with the specialization of OwningPtr<char []>.
>
> I haven't changed anything with regards to envp versus getenv. My thoughts are that if envp can become stale, we should remove it completely and use only getenv, but that should probably be a separate patch. What do you think?

I think ignoring envp is the right approach, and you might as well do it now.



================
Comment at: flang/runtime/environment.cpp:98
+
+static OwningPtr<char[]> NullTerminatedString(
+    const char *name, std::size_t name_length) {
----------------
rovka wrote:
> klausler wrote:
> > `char` can be used here  instead of `char]` and would be more idiomatic.
> > 
> > This function might have use cases elsewhere; consider exposing it in tools.h or memory.h.
> > `char` can be used here  instead of `char]` and would be more idiomatic.
> 
> But then we won't have operator[].
> 
> > This function might have use cases elsewhere; consider exposing it in tools.h or memory.h.
> 
> Will do.
> 
> > `char` can be used here  instead of `char]` and would be more idiomatic.
> 
> But then we won't have operator[].

Yes, it'll work.  In C and C++, `x[y]` is defined to be `*((x)+(y))` so you can offset a pointer like `p[1]` to get the following element.



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111785/new/

https://reviews.llvm.org/D111785