Mike Percy has posted comments on this change. Change subject: Add Google Breakpad support to Kudu ......................................................................
Patch Set 27: (3 comments) http://gerrit.cloudera.org:8080/#/c/5620/27/src/kudu/client/symbols.map File src/kudu/client/symbols.map: Line 26: # breakpad > Can we wildcard some of these? Maybe ConvertUTF* and my_*? Done http://gerrit.cloudera.org:8080/#/c/5620/27/src/kudu/util/minidump.cc File src/kudu/util/minidump.cc: Line 217: google::InstallFailureFunction(&AbortFailureFunction); > Do you even need the redirection through AbortFailureFunction? abort() has Yeah, just passing abort works, but I thought it was helpful to have it jump through AbortFailureFunction so it would be obvious that it was triggered through this code path. http://gerrit.cloudera.org:8080/#/c/5620/27/src/kudu/util/subprocess.cc File src/kudu/util/subprocess.cc: Line 432: ResetSigPipeHandlerToDefault(); > Why do we only reset SIGPIPE to SIG_DFL and not every signal? What about SI I added a short comment, but it's for the following reasons: 1. The shell or parent process invoking Kudu may set the signal disposition to ignore and they might know better than us what they want, e.g. for HUP. 2. As far as I could find, we only explicitly set the signal disposition to IGN for PIPE; we don't ignore any other signals. 3. Setting the signal handler to DFL is only necessary if it was previously set to IGN; if it was set to be caught then execve() automatically sets it to DFL. -- To view, visit http://gerrit.cloudera.org:8080/5620 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I495695cc38b75377f20b0497093a81ed5baa887f Gerrit-PatchSet: 27 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Dinesh Bhat <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
