----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18138/#review34908 -----------------------------------------------------------
Ship it! This new version of the fix does address my concern about both lhs/rhs being empty, but the logic can be simplified a bit. src/master/http.cpp <https://reviews.apache.org/r/18138/#comment65292> I like how clear the code is this way, making the case for both-empty very explicit. But I will point out that all this boolean logic can be simplified. If you check the false case first in both methods, you'll achieve the same thing (and the compiler may be smart enough to clean this up for you). ascending: if (rhsSize==0) return false; if (lhsSize==0) return true; // would have already returned false if both were empty. descending: if (lhsSize==0) return false; if (rhsSize==0) return true; // would have already returned false if both were empty. - Adam B On Feb. 19, 2014, 11:30 a.m., Niklas Nielsen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18138/ > ----------------------------------------------------------- > > (Updated Feb. 19, 2014, 11:30 a.m.) > > > Review request for mesos, Adam B, Ben Mahler, and Vinod Kone. > > > Bugs: MESOS-979 > https://issues.apache.org/jira/browse/MESOS-979 > > > Repository: mesos-git > > > Description > ------- > > Related to MESOS-979, the tasks.json endpoint would sporadically cause the > master process to SEGFAULT. The code failed during the subsequent sort > (after gathering running and completed tasks) deferencing garbage pointer > values. The issue turns out to be an issue with std::sort and strict weak- > ordering. Descending order (which was default) was implemented as !ascending() > which is not a strict greather-than equal. This patch expands the descending > implementation and reenables endpoint. > > > Diffs > ----- > > src/master/http.cpp 6aeb257 > src/master/master.cpp f4f5e04 > > Diff: https://reviews.apache.org/r/18138/diff/ > > > Testing > ------- > > make check and functional testing; can't trigger earlier repro steps for > segfault. > > > Thanks, > > Niklas Nielsen > >
