[Lldb-commits] [PATCH] D130939: [LLDB][NFC] Fix potential div by 0 "count" can be zero potentially causing div by 0

Wed Aug 3 01:02:01 PDT 2022

DavidSpickett added a comment.

I stopped being lazy and went and found where this is used. It's actually a command you can run from the interpreter that prints out the results of the testing.

  (lldb) process plugin packet speed-test -c 0
  Testing sending 0 packets of various sizes:
  <...>
  qSpeedTest(send=   1024, recv=   1024) in 0.000000120 s for      0.00 packets/s (0.000000 ms per packet) with standard deviation of 0.000000 ms
  Testing receiving 4.0MB of data using varying receive packet sizes:
  <...>
  qSpeedTest(send=      0, recv=   1024)   4096 packets needed to receive 4.0MB in 0.222097710 s for 18.01 MB/sec for  18442.33 packets/sec (0.054223 ms per packet)

So I think it is legitimate to specify 0 send packets here, if you just want to test receiving. So keep the check you've got and add the same sort of check for the instance above it.

(whether this command should even print anything if you're sending 0 packets, that's a matter for another time)

================
Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:2483
             ((float)num_packets) / duration<float>(total_time).count();
         auto average_per_packet = total_time / num_packets;
         const duration<float> standard_deviation =
----------------
fixathon wrote:
> DavidSpickett wrote:
> > This could have the same issue if `num_packets` parameter is 0.
> This instance of **average_per_packet** variable is not even being used anywhere in the code, in its local scope. Same for **packets_per_second** just above it.
> These 2 could possibly just be deleted?
It's used to print to the stream below.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130939