mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Benjamin Bannier" <benjamin.bann...@mesosphere.io>
Subject Re: Review Request 39005: stout: Added thread-safe replacement for strerror.
Date Wed, 04 Nov 2015 14:38:26 GMT


> On Nov. 4, 2015, 12:48 p.m., Bernd Mathiske wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/strerror.hpp, line 36
> > <https://reviews.apache.org/r/39005/diff/6/?file=1115248#file1115248line36>
> >
> >     How is this relevant?

Agreed, remove reference (which might go stale).


> On Nov. 4, 2015, 12:48 p.m., Bernd Mathiske wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/strerror.hpp, line 64
> > <https://reviews.apache.org/r/39005/diff/6/?file=1115248#file1115248line64>
> >
> >     a) This is an obscure reference. Pleae provide a pointer. b) Do you mean "similar
to" or "attributed to" instead of "due to"? The latter indicates that you need to program
this way because of something in heimdal's fcache code affecting strerror_r. That seems implausible.

Removed the reference which was both obscure and not very helpful, and in danger of going
stale.


> On Nov. 4, 2015, 12:48 p.m., Bernd Mathiske wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/strerror.hpp, line 53
> > <https://reviews.apache.org/r/39005/diff/6/?file=1115248#file1115248line53>
> >
> >     How would str_error run into errors "itself"? We don't seem to be handling any
such erros here. But if there are any, shouldn't we? And if there aren't any, would the whole
point be moot then?

Error scenarios would be (a) an invalid errnum, or (b) a buffer to small to contain the error
message from `strerror_r`.

For (a) `strerror_r` will still return some error message string so we do not need to do anything.

For (b) we would receive a truncated error message string. The buffer size we use "should
be big enough" for any conceivable error message. I added a note to the docstring so users
are aware of this limitation.

In both cases we would receive some string representation of the given `errnum`.


I also expanded the comment on the problems <glibc-2.16 has; I believe providing a complete
and safe workaround for all possible use cases of this header provides diminishing returns.


- Benjamin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39005/#review105058
-----------------------------------------------------------


On Nov. 4, 2015, 2:37 p.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39005/
> -----------------------------------------------------------
> 
> (Updated Nov. 4, 2015, 2:37 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Ben Mahler, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3551
>     https://issues.apache.org/jira/browse/MESOS-3551
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This commit also adds test cases for os::strerror (from stout) which can
> only be committed with a libprocess commit -- the corresponding Makefile
> lives there.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 76e1674e08bbe65a4fdf86731823a61f231d6d12

>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 67b142bbc2d80f40a1e893cc5813d58dd2aa8381

>   3rdparty/libprocess/3rdparty/stout/include/stout/os/strerror.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 26e1377ee625748b7fdbec327fef9ac602535f83

>   3rdparty/libprocess/3rdparty/stout/tests/os/strerror_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39005/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message