sandsmark added inline comments.

INLINE COMMENTS

> ConnectionMapping.cpp:157
> +        // Should be within the first 16 characters.
> +        size_t data_start = data.find(':');
> +        if (data_start >= 16) {

if we're going for ultra optimized, const everything I guess.

> ConnectionMapping.cpp:167
> +        // Check for IPv4.
> +        if (data.size() < data_start + 87 + 1) {
> +            continue;

instead of 87 + 1, maybe have a `constexpr int lineLength = strlen("0: 
00000000000000000000000001000000:0277 00000000000000000000000000000000:0000 0A 
00000000:00000000 00:00000000 00000000     0        0 16741");` above with the 
comment?
or:

  constexpr const char *exampleLine = "0: 00000000000000000000000001000000:0277 
00000000000000000000000000000000:0000 0A 00000000:00000000 00:00000000 00000000 
    0        0 16741";
      constexpr int lineLength = strlen(exampleLine);

both more understandable and less prone to accidentally putting in the wrong 
number.

same with all other magic string offset numbers here.

> ConnectionMapping.cpp:176
> +            char *const ipv4 = &data[data_start + 2];
> +            ipv4[8] = '\0';
> +            localAddress.address[3] = (uint32_t)strtoul(ipv4, nullptr, 16);

strtoul stops at the first non-digit, so is this really necessary?

> ConnectionMapping.cpp:203
> +                    ipv6[32] = '\0';
> +                    localAddress.address[3] = (uint32_t)strtoul(&ipv6[24], 
> nullptr, 16);
> +                } else {

I prefer the more c++-ish cast of `uint32_t(foo);` (I think I got that from the 
Qt code style guidelines).

REPOSITORY
  R106 KSysguard

REVISION DETAIL
  https://phabricator.kde.org/D29808

To: dkorth, ahiemstra
Cc: sandsmark, ahiemstra, jpalecek, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, 
ngraham, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

Reply via email to