> On March 12, 2014, 11:36 p.m., Benjamin Hindman wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/os/permissions.hpp, line 23 > > <https://reviews.apache.org/r/19093/diff/2/?file=517092#file517092line23> > > > > How about introducing a new Permissions struct here? > > > > struct Permissions > > { > > explicit Permissions(mode_t mode) > > { > > owner.readable = mode & magic_to_determine_if_mode_is_readable; > > owner.writable = mode & magic_to_determine_if_mode_is_writable; > > owner.executable = mode & magic_to_determine_if_mode_is_executable; > > ... > > } > > > > struct { > > bool readable; > > bool writable; > > bool executable; > > } owner; > > > > struct { > > bool readable; > > bool writable; > > bool executable; > > } group; > > > > struct { > > bool readable; > > bool writable; > > bool executable; > > } others; > > > > bool setuid; > > bool setgid; > > }; > > > > Then in code you could do: > > > > Try<os::Permissions> permissions = os::permissions(path); > > > > if (!permissions.isError()) { > > if (permissions.get().others.readable || > > permissions.get().others.writable || > > permissions.get().others.executable) { > > LOG(WARNING) << "Permissions on credentials file are insufficient!"; > > } > > } > > > > The slight downside here is that we'll be spending some CPU cycles > > constructing a Permissions object, but that's highly unlikely to be a > > performance issue. The upside here is the code becomes very explicit and > > therefore maintainable! > > Ben Mahler wrote: > What about sharing the struct definition across 'owner' 'group' and > 'others'? > > struct { > bool readable; > bool writable; > bool executable; > } owner, group, others;
Definitely! - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19093/#review37012 ----------------------------------------------------------- On March 12, 2014, 11:07 a.m., Isabel Jimenez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/19093/ > ----------------------------------------------------------- > > (Updated March 12, 2014, 11:07 a.m.) > > > Review request for mesos, Benjamin Hindman and Vinod Kone. > > > Bugs: MESOS-1087 > https://issues.apache.org/jira/browse/MESOS-1087 > > > Repository: mesos-git > > > Description > ------- > > Adding permissions check on credentials file and deleting TODO comment > > > Diffs > ----- > > 3rdparty/libprocess/3rdparty/stout/Makefile.am 1075b46 > 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 3f475a4 > 3rdparty/libprocess/3rdparty/stout/include/stout/os/permissions.hpp > PRE-CREATION > src/master/master.cpp 2a40333 > > Diff: https://reviews.apache.org/r/19093/diff/ > > > Testing > ------- > > > Thanks, > > Isabel Jimenez > >
