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

[Diana Picus via Phabricator via llvm-commits]

rovka marked an inline comment as not done.
rovka added inline comments.


================
Comment at: flang/runtime/environment.cpp:98
+
+static OwningPtr<char[]> NullTerminatedString(
+    const char *name, std::size_t name_length) {
----------------
klausler wrote:
> 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.
> 
Yes, on the pointer, but not on the OwningPtr. For that you'd have to call .get() first, which imo doesn't look as nice as using the [] operator directly (which is only possible for OwningPtr<A[]>).


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

https://reviews.llvm.org/D111785