labath accepted this revision.
labath added inline comments.

================
Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp:763
+template <typename T, typename U>
+void fill_clamp(T &dest, U src, typename T::value_type fallback) {
+  dest = dest <= std::numeric_limits<typename T::value_type>::max() ? src
----------------
static void


================
Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp:764
+void fill_clamp(T &dest, U src, typename T::value_type fallback) {
+  dest = dest <= std::numeric_limits<typename T::value_type>::max() ? src
+                                                                    : fallback;
----------------
`src <=`


================
Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp:776
+
+  std::array<char, 64> data;
+  DataEncoder encoder{data.data(), data.size(), lldb::eByteOrderBig,
----------------
mgorny wrote:
> labath wrote:
> > consider:
> > ```
> > struct GdbStat {
> >   llvm::support::ubig32_t st_dev;
> >   llvm::support::ubig32_t st_ino;
> >   ...
> > };
> > 
> > ...
> > 
> > translate(gdb_stats.st_dev, file_stats.st_dev, 0); // I'm not sure that 
> > this clamping is really necessary.
> > ...
> > ```
> > 
> > Seems like it could be nicer, particularly as the vFile_Stat function will 
> > need to do the same thing...
> What's this `translate()` thing? I don't see anything matching in LLVM or 
> LLDB. Or are you saying I should define a helper function?
> 
> As for clamping, I think it's better if we send 0 (a "clearly invalid value") 
> than e.g. truncated `st_dev` that would be simply wrong.
Yeah, that was a new helper function.


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

https://reviews.llvm.org/D107840

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to