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

[Peter Klausler via Phabricator via flang-commits]

klausler added inline comments.


================
Comment at: flang/runtime/environment.cpp:98
+
+static OwningPtr<char[]> NullTerminatedString(
+    const char *name, std::size_t name_length) {
----------------
rovka wrote:
> 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[]>).
> 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[]>).

If you insist on using [] and can't get yourself to call get(), then feel free to define operator= in OwningPtr<>.


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

https://reviews.llvm.org/D111785