> On Jan. 25, 2016, 9:05 p.m., Neil Conway wrote: > > src/common/resources.cpp, line 66 > > <https://reviews.apache.org/r/42733/diff/1/?file=1219795#file1219795line66> > > > > This is actually correct as written, no? i.e., if principal isn't set, > > `principal()` will return the empty string. Since an empty string isn't a > > legal principle, the comparison should just work, I believe.
I believe we allow the empty string as a valid principal. - Greg ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42733/#review116166 ----------------------------------------------------------- On Jan. 25, 2016, 9:02 p.m., Greg Mann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42733/ > ----------------------------------------------------------- > > (Updated Jan. 25, 2016, 9:02 p.m.) > > > Review request for mesos, Jie Yu, Michael Park, and Neil Conway. > > > Repository: mesos > > > Description > ------- > > Added checks for presence of `ReservationInfo.principal`. > > The `ReservationInfo.principal` field was recently made `optional`. During > the deprecation cycle, requests that do not have the field set are > invalidated and rejected. However, there remain a couple places in the code > that assume this field is always set. This patch uses `has_principal()` > before this field is accessed to ensure safety. > > > Diffs > ----- > > src/common/resources.cpp 0fcf86014ab5c9908a1cdb3a57b7c5e70acd7737 > src/master/master.cpp 9ee56277c8a472be9e683d5db505becfb5f7c422 > src/v1/resources.cpp 126e5a2f567d2e281da3f99bc485f7960567eee5 > > Diff: https://reviews.apache.org/r/42733/diff/ > > > Testing > ------- > > `make check` > > > Thanks, > > Greg Mann > >
