[Lldb-commits] [PATCH] D76111: Create basic SBEnvironment class

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

Just some header doc cleanup before we submit. Thanks for sticking with these changes!



================
Comment at: lldb/include/lldb/API/SBEnvironment.h:24
+
+  const lldb::SBEnvironment &operator=(const lldb::SBEnvironment &rhs);
+
----------------
const means nothing in these classes since the ivar will never get modified, so you can remote the leading "const" for the return value. 


================
Comment at: lldb/include/lldb/API/SBEnvironment.h:27
+  /// Return the value of a given environment variable.
+  ///
+  /// \return
----------------
need \param entries in header doc


================
Comment at: lldb/include/lldb/API/SBEnvironment.h:29
+  /// \return
+  ///     The value of the enviroment variable or null if not present.
+  const char *Get(const char *name);
----------------
Rephrase the return value docs a bit. Maybe:

```
/// The value of the environment variable or null if not present. If the environment variable has no value but is present a valid pointer to an empty string will be returned.
```

Is that what we are doing? Returning "" when variable is there but has no value?


================
Comment at: lldb/include/lldb/API/SBEnvironment.h:32
+
+  /// \return
+  ///     The name at the given index or null if the index is invalid.
----------------
need brief description and \param entries in header doc


================
Comment at: lldb/include/lldb/API/SBEnvironment.h:36
+
+  /// \return
+  ///     The value at the given index or null if the index is invalid.
----------------
need brief description and \param entries in header doc


================
Comment at: lldb/include/lldb/API/SBEnvironment.h:37
+  /// \return
+  ///     The value at the given index or null if the index is invalid.
+  const char *GetValueAtIndex(size_t index);
----------------
Need to clarify what happens when the env far is there but has no value. Should match the Get(const char*) function return value with what ever we say is the solution for that.


================
Comment at: lldb/include/lldb/API/SBEnvironment.h:40
+
+  size_t GetNumValues();
+
----------------
Move above the *AtIndex(size_t) calls so users see this first in API.


================
Comment at: lldb/include/lldb/API/SBEnvironment.h:44
+  /// If the variable exists, its value is updated only if overwrite is true.
+  ///
+  /// \return
----------------
need \param entries in header doc


================
Comment at: lldb/include/lldb/API/SBEnvironment.h:50
+  /// Unset an environment variable if exists.
+  ///
+  /// \return
----------------
need \param entries in header doc


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76111