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

[Diana Picus via Phabricator via flang-commits]

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:
> > > 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<>.
It doesn't actually matter anymore, I removed NullTerminatedString and all other changes to memory.h.


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

https://reviews.llvm.org/D111785