> On Jan. 6, 2020, 12:59 p.m., Benjamin Bannier wrote: > > 3rdparty/libprocess/include/process/address.hpp > > Lines 238 (patched) > > <https://reviews.apache.org/r/71947/diff/1/?file=2191675#file2191675line238> > > > > As discussed offline, let's add a `CHECK` here that `sun_path` is not > > empty.
As it turns out, I was mistaken in our offline discussion: a single null byte is a perfectly valid name for an abstract domain socket. > On Jan. 6, 2020, 12:59 p.m., Benjamin Bannier wrote: > > 3rdparty/libprocess/include/process/address.hpp > > Line 232 (original), 247 (patched) > > <https://reviews.apache.org/r/71947/diff/1/?file=2191675#file2191675line247> > > > > For unnamed sockets we would now return an empty string here which > > seems leaky. What do you think about changing the return type of > > `Address::path` to e.g., `Option<string>` instead and returning a `None` in > > that case? I've thought about it and an empty string actually uniquely identifies unnamed sockets. Pathname sockets always must have at least one character, and the difference between an empty string and an abstract domain socket whose name is a single null byte would be that the former has size 0, and the latter has size 1. So I think as long as we document this correspondence, this should be fine. - Benno ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71947/#review219127 ----------------------------------------------------------- On Jan. 10, 2020, 1:37 a.m., Benno Evers wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71947/ > ----------------------------------------------------------- > > (Updated Jan. 10, 2020, 1:37 a.m.) > > > Review request for mesos, Benjamin Bannier and Benjamin Mahler. > > > Repository: mesos > > > Description > ------- > > Address handling code for unix domain sockets assumed that > strlen() could be used to compute the name of a unix domain > socket, but that fails for unnamed sockets or in the case > where an abstract domain socket contains embedded null bytes. > > This patch adds a new `length` parameter to correctly handle > these special cases. > > > Diffs > ----- > > 3rdparty/libprocess/include/process/address.hpp > 749498056b52b916dfaf6c85f83ecc05e0d5406c > 3rdparty/libprocess/include/process/network.hpp > 8f48a4a78557309a9b1b00d7defb45eed454b077 > > > Diff: https://reviews.apache.org/r/71947/diff/2/ > > > Testing > ------- > > Ran existing unit tests and verified that the newly added `CHECK()` doesn't > trigger. > > > Thanks, > > Benno Evers > >
