[Lldb-commits] [PATCH] D25391: Remove usages of TimeValue from gdb-remote process plugin
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Sat Oct 8 09:40:33 PDT 2016
labath added inline comments.
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp:192
+ packet_result = ReadPacket(
+ response, std::chrono::duration_cast<std::chrono::microseconds>(
> `using namespace std` is generally frowned upon, but I wonder if we could be more lenient about `using namespace std::chrono`? Would make a lot of those code easier on the eyes.
I started using it in some tighter scopes, which are chrono-heavy. I'd be fine with the using directive at file level though.
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp:266-267
> Same here. You should just be able to pass the `duration` straight through, and update `ReadPacket` to do the cast.
These should go away, when everything is updated to use chrono, but the hierarchy there is multiple levels deep, so I cannot do it in a single pass (ReadPacket just calls `WaitForPacketWithTimeoutMicroSecondsNoLock`, which would also need to be updated... the real cast should only happen somewhere in the Connection classes, once you actually start issuing system calls).
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h:86
+ ScopedTimeout(GDBRemoteCommunication &gdb_comm,
+ std::chrono::seconds timeout);
> To make this more generic, this could be a `std::chrono::duration<int>`. If for any reason someone wanted `3.2` seconds, they could then specify it.
I don't think that works the way you think it does. If you don't specify the ratio template argument, it will just get defaulted to `std::ratio<1>`, meaning seconds. To have it generic you'd have to make this a template function, with the ratio as the parameter, but at that point you might as well just choose a type with the highest precision you are willing to accept. So, I could make this take a milli or micro second durations, but I don't think that's necessary for a packet timeout.
More information about the lldb-commits