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 2017/01/15 09:12:38 UTC
Review Request 55544: Windows: Set `MAXHOSTNAMELEN` to an appropriate
value.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55544/
-----------------------------------------------------------
Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.
Bugs: MESOS-3398
https://issues.apache.org/jira/browse/MESOS-3398
Repository: mesos
Description
-------
The POSIX standard defines a function `getnameinfo`, but does not
specify a maximum length that a hostname can have. To simplify the logic
of choosing a reasonable buffer size to hold the hostname, most
implementations of this specification expose an additional symbol (e.g.,
NI_MAXHOST) that represents the maximum length a hostname can have.
Our code uses `MAXHOSTNAMELEN`, and so on platforms where this symbol is
not implemented (such as Windows), it is necessary to define it.
Currently, `stout/windows.hpp` defines this to be `64`. This value was
chosen because other popular codebases seemed to use this value. But, as
MESOS-3398 explains, we were never sure that this was correct.
Now we have compelling evidence that it isn't: the Windows documentation
for `getnameinfo` clearly states that the maximum length of a hostname
must is captured as `NI_MAXHOST`, and in all recent versions of the
standard libraries, the value of this symbol is 1025.
Hence, for machines with a hostname longer than 64 bytes, almost all the
tests will abort mysteriously, with a cryptic error that claims that the
"data area passed to a system call is too small", which essentially
means that this particular buffer is too small to hold the hostname.
This commit will cause this value to be correctly set on Windows.
Diffs
-----
3rdparty/stout/include/stout/windows.hpp d89c70902cf60544441608c2cb290b0727cbb45c
Diff: https://reviews.apache.org/r/55544/diff/
Testing
-------
Thanks,
Alex Clemmer
Re: Review Request 55544: Windows: Set `MAXHOSTNAMELEN` to an
appropriate value.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55544/#review162526
-----------------------------------------------------------
Ship it!
Ship It!
- Joseph Wu
On Jan. 15, 2017, 1:12 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55544/
> -----------------------------------------------------------
>
> (Updated Jan. 15, 2017, 1:12 a.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.
>
>
> Bugs: MESOS-3398
> https://issues.apache.org/jira/browse/MESOS-3398
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The POSIX standard defines a function `getnameinfo`, but does not
> specify a maximum length that a hostname can have. To simplify the logic
> of choosing a reasonable buffer size to hold the hostname, most
> implementations of this specification expose an additional symbol (e.g.,
> NI_MAXHOST) that represents the maximum length a hostname can have.
>
> Our code uses `MAXHOSTNAMELEN`, and so on platforms where this symbol is
> not implemented (such as Windows), it is necessary to define it.
>
> Currently, `stout/windows.hpp` defines this to be `64`. This value was
> chosen because other popular codebases seemed to use this value. But, as
> MESOS-3398 explains, we were never sure that this was correct.
>
> Now we have compelling evidence that it isn't: the Windows documentation
> for `getnameinfo` clearly states that the maximum length of a hostname
> must is captured as `NI_MAXHOST`, and in all recent versions of the
> standard libraries, the value of this symbol is 1025.
>
> Hence, for machines with a hostname longer than 64 bytes, almost all the
> tests will abort mysteriously, with a cryptic error that claims that the
> "data area passed to a system call is too small", which essentially
> means that this particular buffer is too small to hold the hostname.
>
> This commit will cause this value to be correctly set on Windows.
>
>
> Diffs
> -----
>
> 3rdparty/stout/include/stout/windows.hpp d89c70902cf60544441608c2cb290b0727cbb45c
>
> Diff: https://reviews.apache.org/r/55544/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 55544: Windows: Set `MAXHOSTNAMELEN` to an
appropriate value.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55544/#review161661
-----------------------------------------------------------
Bad patch!
Reviews applied: [55544, 55543, 55328, 55327, 55314, 55313, 55312, 55311, 55162, 55161, 55040, 55039, 55038, 55037, 55030, 55024, 55023, 55022]
Failed command: python support/apply-reviews.py -n -r 55022
Error:
2017-01-15 09:41:00 URL:https://reviews.apache.org/r/55022/diff/raw/ [1047/1047] -> "55022.patch" [1]
error: patch failed: 3rdparty/libprocess/src/io.cpp:79
error: 3rdparty/libprocess/src/io.cpp: patch does not apply
Full log: https://builds.apache.org/job/Mesos-Reviewbot/16729/console
- Mesos ReviewBot
On Jan. 15, 2017, 9:12 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55544/
> -----------------------------------------------------------
>
> (Updated Jan. 15, 2017, 9:12 a.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.
>
>
> Bugs: MESOS-3398
> https://issues.apache.org/jira/browse/MESOS-3398
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The POSIX standard defines a function `getnameinfo`, but does not
> specify a maximum length that a hostname can have. To simplify the logic
> of choosing a reasonable buffer size to hold the hostname, most
> implementations of this specification expose an additional symbol (e.g.,
> NI_MAXHOST) that represents the maximum length a hostname can have.
>
> Our code uses `MAXHOSTNAMELEN`, and so on platforms where this symbol is
> not implemented (such as Windows), it is necessary to define it.
>
> Currently, `stout/windows.hpp` defines this to be `64`. This value was
> chosen because other popular codebases seemed to use this value. But, as
> MESOS-3398 explains, we were never sure that this was correct.
>
> Now we have compelling evidence that it isn't: the Windows documentation
> for `getnameinfo` clearly states that the maximum length of a hostname
> must is captured as `NI_MAXHOST`, and in all recent versions of the
> standard libraries, the value of this symbol is 1025.
>
> Hence, for machines with a hostname longer than 64 bytes, almost all the
> tests will abort mysteriously, with a cryptic error that claims that the
> "data area passed to a system call is too small", which essentially
> means that this particular buffer is too small to hold the hostname.
>
> This commit will cause this value to be correctly set on Windows.
>
>
> Diffs
> -----
>
> 3rdparty/stout/include/stout/windows.hpp d89c70902cf60544441608c2cb290b0727cbb45c
>
> Diff: https://reviews.apache.org/r/55544/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>