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