Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/8546 )
Change subject: IMPALA-5624: Replace "ls -l" with opendir() in ProcessStateInfo ...................................................................... Patch Set 4: (7 comments) http://gerrit.cloudera.org:8080/#/c/8546/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8546/4//COMMIT_MSG@11 PS4, Line 11: api > nit: acronyms are usually all caps Done http://gerrit.cloudera.org:8080/#/c/8546/4/be/src/util/process-state-info.h File be/src/util/process-state-info.h: http://gerrit.cloudera.org:8080/#/c/8546/4/be/src/util/process-state-info.h@77 PS4, Line 77: /// File Descriptors information will be read from /proc/pid/fd. > nit: /proc/self/fd Done http://gerrit.cloudera.org:8080/#/c/8546/4/be/src/util/process-state-info.cc File be/src/util/process-state-info.cc: http://gerrit.cloudera.org:8080/#/c/8546/4/be/src/util/process-state-info.cc@155 PS4, Line 155: does > nit: do not count. Done http://gerrit.cloudera.org:8080/#/c/8546/4/be/src/util/process-state-info.cc@156 PS4, Line 156: self > nit: either use getpid() here to be consistent with the rest of the file, o Done http://gerrit.cloudera.org:8080/#/c/8546/4/be/src/util/process-state-info.cc@159 PS4, Line 159: dir > Can you try to think of better variable names? You have a DIR called d, and Done http://gerrit.cloudera.org:8080/#/c/8546/4/be/src/util/process-state-info.cc@160 PS4, Line 160: while ((dir = readdir(d)) != nullptr) ++fd_count; > Please add a brief comment why your usage of readdir is thread-safe here. Done http://gerrit.cloudera.org:8080/#/c/8546/4/be/src/util/process-state-info.cc@163 PS4, Line 163: std:: > You shouldn't need std:: here Done -- To view, visit http://gerrit.cloudera.org:8080/8546 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibffae8069a62e100abbfa7d558b49040b095ddc0 Gerrit-Change-Number: 8546 Gerrit-PatchSet: 4 Gerrit-Owner: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com> Gerrit-Comment-Date: Sat, 18 Nov 2017 00:44:27 +0000 Gerrit-HasComments: Yes
