You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Alex Clemmer <cl...@gmail.com> on 2016/02/10 07:36:48 UTC
Review Request 43410: Windows: Added support for dynamic library
loading.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43410/
-----------------------------------------------------------
Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
Repository: mesos
Description
-------
Originally review #40583, this was resurrected when Dario went on
paternity leave. Minor changes were made, the review itself remains
largely in tact.
Diffs
-----
3rdparty/libprocess/3rdparty/stout/include/stout/dynamiclibrary.hpp 166c97d3e4b07451b8b4c01092cecb69315c691a
3rdparty/libprocess/3rdparty/stout/include/stout/posix/dynamiclibrary.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/windows/dynamiclibrary.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/43410/diff/
Testing
-------
Thanks,
Alex Clemmer
Re: Review Request 43410: Windows: Added support for dynamic library
loading.
Posted by Alex Naparu <al...@outlook.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43410/#review119927
-----------------------------------------------------------
Ship it!
Ship It!
- Alex Naparu
On Feb. 18, 2016, 12:59 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43410/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2016, 12:59 a.m.)
>
>
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
>
>
> Bugs: MESOS-4496
> https://issues.apache.org/jira/browse/MESOS-4496
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Originally review #40583, this was resurrected when Dario went on
> paternity leave. Minor changes were made, the review itself remains
> largely in tact.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/dynamiclibrary.hpp 166c97d3e4b07451b8b4c01092cecb69315c691a
> 3rdparty/libprocess/3rdparty/stout/include/stout/posix/dynamiclibrary.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/dynamiclibrary.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/43410/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 43410: Windows: Added support for dynamic library
loading.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43410/
-----------------------------------------------------------
(Updated Feb. 25, 2016, 5:15 p.m.)
Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
Bugs: MESOS-4496
https://issues.apache.org/jira/browse/MESOS-4496
Repository: mesos
Description
-------
Originally review #40583, this was resurrected when Dario went on
paternity leave. Minor changes were made, the review itself remains
largely in tact.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/include/stout/dynamiclibrary.hpp 166c97d3e4b07451b8b4c01092cecb69315c691a
3rdparty/libprocess/3rdparty/stout/include/stout/posix/dynamiclibrary.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/windows/dynamiclibrary.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/43410/diff/
Testing
-------
Thanks,
Alex Clemmer
Re: Review Request 43410: Windows: Added support for dynamic library
loading.
Posted by Alex Clemmer <cl...@gmail.com>.
> On Feb. 24, 2016, 11:57 p.m., Michael Park wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/windows/dynamiclibrary.hpp, lines 67-68
> > <https://reviews.apache.org/r/43410/diff/3/?file=1253406#file1253406line67>
> >
> > Fits in one line.
Hmm, does it? I don't think so, am I missing something obvious?
- Alex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43410/#review120611
-----------------------------------------------------------
On Feb. 18, 2016, 12:59 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43410/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2016, 12:59 a.m.)
>
>
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
>
>
> Bugs: MESOS-4496
> https://issues.apache.org/jira/browse/MESOS-4496
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Originally review #40583, this was resurrected when Dario went on
> paternity leave. Minor changes were made, the review itself remains
> largely in tact.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/dynamiclibrary.hpp 166c97d3e4b07451b8b4c01092cecb69315c691a
> 3rdparty/libprocess/3rdparty/stout/include/stout/posix/dynamiclibrary.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/dynamiclibrary.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/43410/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 43410: Windows: Added support for dynamic library
loading.
Posted by Michael Park <mp...@apache.org>.
> On Feb. 24, 2016, 11:57 p.m., Michael Park wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/windows/dynamiclibrary.hpp, lines 67-68
> > <https://reviews.apache.org/r/43410/diff/3/?file=1253406#file1253406line67>
> >
> > Fits in one line.
>
> Alex Clemmer wrote:
> Hmm, does it? I don't think so, am I missing something obvious?
I meant to wrap it like this:
```
if (!FreeLibrary(handle_)) {
return WindowsError(
"Could not close library '" + (path_.isSome() ? path_.get() : ""));
}
```
I've committed this patch with this change.
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43410/#review120611
-----------------------------------------------------------
On Feb. 25, 2016, 5:15 p.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43410/
> -----------------------------------------------------------
>
> (Updated Feb. 25, 2016, 5:15 p.m.)
>
>
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
>
>
> Bugs: MESOS-4496
> https://issues.apache.org/jira/browse/MESOS-4496
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Originally review #40583, this was resurrected when Dario went on
> paternity leave. Minor changes were made, the review itself remains
> largely in tact.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/dynamiclibrary.hpp 166c97d3e4b07451b8b4c01092cecb69315c691a
> 3rdparty/libprocess/3rdparty/stout/include/stout/posix/dynamiclibrary.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/dynamiclibrary.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/43410/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 43410: Windows: Added support for dynamic library
loading.
Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43410/#review120611
-----------------------------------------------------------
Fix it, then Ship it!
3rdparty/libprocess/3rdparty/stout/include/stout/posix/dynamiclibrary.hpp (lines 50 - 52)
<https://reviews.apache.org/r/43410/#comment182079>
Not yours, but fits in one line.
3rdparty/libprocess/3rdparty/stout/include/stout/windows/dynamiclibrary.hpp (lines 50 - 51)
<https://reviews.apache.org/r/43410/#comment182077>
Fits in one line.
3rdparty/libprocess/3rdparty/stout/include/stout/windows/dynamiclibrary.hpp (line 65)
<https://reviews.apache.org/r/43410/#comment182076>
This C-style cast to `HMODULE` seems to indicate that `handle_` should be of type `HMODULE`. In fact `LoadLibrary`'s return type is `HMODULE`, so it seems to line up well?
3rdparty/libprocess/3rdparty/stout/include/stout/windows/dynamiclibrary.hpp (lines 67 - 68)
<https://reviews.apache.org/r/43410/#comment182078>
Fits in one line.
- Michael Park
On Feb. 18, 2016, 12:59 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43410/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2016, 12:59 a.m.)
>
>
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
>
>
> Bugs: MESOS-4496
> https://issues.apache.org/jira/browse/MESOS-4496
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Originally review #40583, this was resurrected when Dario went on
> paternity leave. Minor changes were made, the review itself remains
> largely in tact.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/dynamiclibrary.hpp 166c97d3e4b07451b8b4c01092cecb69315c691a
> 3rdparty/libprocess/3rdparty/stout/include/stout/posix/dynamiclibrary.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/dynamiclibrary.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/43410/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 43410: Windows: Added support for dynamic library
loading.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43410/
-----------------------------------------------------------
(Updated Feb. 18, 2016, 12:59 a.m.)
Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
Bugs: MESOS-4496
https://issues.apache.org/jira/browse/MESOS-4496
Repository: mesos
Description
-------
Originally review #40583, this was resurrected when Dario went on
paternity leave. Minor changes were made, the review itself remains
largely in tact.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/include/stout/dynamiclibrary.hpp 166c97d3e4b07451b8b4c01092cecb69315c691a
3rdparty/libprocess/3rdparty/stout/include/stout/posix/dynamiclibrary.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/windows/dynamiclibrary.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/43410/diff/
Testing
-------
Thanks,
Alex Clemmer
Re: Review Request 43410: Windows: Added support for dynamic library
loading.
Posted by Yi Sun <yi...@hotmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43410/#review119099
-----------------------------------------------------------
Ship it!
Ship It!
- Yi Sun
On Feb. 10, 2016, 6:01 p.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43410/
> -----------------------------------------------------------
>
> (Updated Feb. 10, 2016, 6:01 p.m.)
>
>
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
>
>
> Bugs: MESOS-4496
> https://issues.apache.org/jira/browse/MESOS-4496
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Originally review #40583, this was resurrected when Dario went on
> paternity leave. Minor changes were made, the review itself remains
> largely in tact.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/dynamiclibrary.hpp 166c97d3e4b07451b8b4c01092cecb69315c691a
> 3rdparty/libprocess/3rdparty/stout/include/stout/posix/dynamiclibrary.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/dynamiclibrary.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/43410/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 43410: Windows: Added support for dynamic library
loading.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43410/
-----------------------------------------------------------
(Updated Feb. 10, 2016, 6:01 p.m.)
Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
Bugs: MESOS-4496
https://issues.apache.org/jira/browse/MESOS-4496
Repository: mesos
Description
-------
Originally review #40583, this was resurrected when Dario went on
paternity leave. Minor changes were made, the review itself remains
largely in tact.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/include/stout/dynamiclibrary.hpp 166c97d3e4b07451b8b4c01092cecb69315c691a
3rdparty/libprocess/3rdparty/stout/include/stout/posix/dynamiclibrary.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/windows/dynamiclibrary.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/43410/diff/
Testing
-------
Thanks,
Alex Clemmer
Re: Review Request 43410: Windows: Added support for dynamic library
loading.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43410/
-----------------------------------------------------------
(Updated Feb. 10, 2016, 6:47 a.m.)
Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
Bugs: MESOS-4496
https://issues.apache.org/jira/browse/MESOS-4496
Repository: mesos
Description
-------
Originally review #40583, this was resurrected when Dario went on
paternity leave. Minor changes were made, the review itself remains
largely in tact.
Diffs
-----
3rdparty/libprocess/3rdparty/stout/include/stout/dynamiclibrary.hpp 166c97d3e4b07451b8b4c01092cecb69315c691a
3rdparty/libprocess/3rdparty/stout/include/stout/posix/dynamiclibrary.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/windows/dynamiclibrary.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/43410/diff/
Testing
-------
Thanks,
Alex Clemmer
Re: Review Request 43410: Windows: Added support for dynamic library
loading.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43410/
-----------------------------------------------------------
(Updated Feb. 10, 2016, 6:44 a.m.)
Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
Repository: mesos
Description
-------
Originally review #40583, this was resurrected when Dario went on
paternity leave. Minor changes were made, the review itself remains
largely in tact.
Diffs
-----
3rdparty/libprocess/3rdparty/stout/include/stout/dynamiclibrary.hpp 166c97d3e4b07451b8b4c01092cecb69315c691a
3rdparty/libprocess/3rdparty/stout/include/stout/posix/dynamiclibrary.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/windows/dynamiclibrary.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/43410/diff/
Testing
-------
Thanks,
Alex Clemmer