Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12657 )
Change subject: KUDU-2721: support ranges in CPU lists ...................................................................... Patch Set 8: (4 comments) http://gerrit.cloudera.org:8080/#/c/12657/8/src/kudu/gutil/sysinfo.cc File src/kudu/gutil/sysinfo.cc: http://gerrit.cloudera.org:8080/#/c/12657/8/src/kudu/gutil/sysinfo.cc@177 PS8, Line 177: int ParseMaxCpuIndex(const char* str) { Should early out if 'str' is nullptr, or DCHECK on that. http://gerrit.cloudera.org:8080/#/c/12657/8/src/kudu/gutil/sysinfo.cc@202 PS8, Line 202: num_start == pos It's not really clear under what circumstances num_start == pos. Is it for illegal open-ended ranges (i.e. "5-"?) http://gerrit.cloudera.org:8080/#/c/12657/8/src/kudu/gutil/sysinfo.cc@202 PS8, Line 202: dash != nullptr Isn't this implied by the next condition? That is, if dash == range_start, aren't we guaranteed that dash != nullptr? http://gerrit.cloudera.org:8080/#/c/12657/8/src/kudu/gutil/sysinfo.cc@205 PS8, Line 205: long start_idx = strtol(range_start, nullptr, 10); Why strtol and not strtoul here? Wouldn't a negative number be interpreted as a dash and break the parsing earlier? -- To view, visit http://gerrit.cloudera.org:8080/12657 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I97311bfbcf70bea069e941b6e7f4f015fb781b3f Gerrit-Change-Number: 12657 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Tue, 05 Mar 2019 21:34:44 +0000 Gerrit-HasComments: Yes
