> On Sept. 15, 2014, 7:15 p.m., Dominic Hamon wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp, line 111 > > <https://reviews.apache.org/r/25623/diff/2/?file=689080#file689080line111> > > > > what happens if i pass in (by accident) a NULL FILE pointer? it might > > end up here which will then attempt to open fd 0. > > > > In other words, be very careful when overloading pointer/int. > > > > do we need this overload? can we not just have a hard requirement that > > the user must pass in a FILE* or path?
https://gist.github.com/kdomanski/5e3aa6077c348541dc32 In the above example, passing a variable of pointer type to function taking an int is invalid conversion. Additionally GCC>=4.5.4 an Clang warn me when directly inserting NULL as parameter (GCC 4.4.7 does not). Thus overload resolution should be unambiguous. This particular overload is not indispensible, but removes the necessity to manually call ::fdopen on a pipe endpoint. - Kamil ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25623/#review53356 ----------------------------------------------------------- On Sept. 14, 2014, 8:25 p.m., Kamil Domanski wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25623/ > ----------------------------------------------------------- > > (Updated Sept. 14, 2014, 8:25 p.m.) > > > Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Vinod > Kone. > > > Bugs: MESOS-1794 > https://issues.apache.org/jira/browse/MESOS-1794 > > > Repository: mesos-git > > > Description > ------- > > This divides {{Try<int> net::download(const std::string& url, const > std::string& path)}} into three overloads: > > 1. Try<int> net::download(const std::string& url, FILE* file) downloads a > file and sends it to FILE stream > 2. Try<int> net::download(const std::string& url, int fdout) opens a file > stream based on a file descriptor and calls 1) > 3. Try<int> net::download(const std::string& url, const std::string& path) > opens a file at _path_ and passes the descriptor to 2), effectively working > as before this change > > This will allow to download into any file descriptor, such as a pipe, e.g. > directly to extraction, as proposed by Bernd Mathiske in MESOS-1667. > > > Diffs > ----- > > 3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp 7138bc2 > > Diff: https://reviews.apache.org/r/25623/diff/ > > > Testing > ------- > > `cd 3rdparty/libprocess/ && make check` for compilation test only > > https://gist.github.com/kdomanski/f1d39266f6abc0f14f58 for operation test > > > Thanks, > > Kamil Domanski > >
