mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Cody Maloney" <c...@mesosphere.io>
Subject Re: Review Request 26472: stout: Always use stout ABORT() rather than system abort()
Date Thu, 09 Oct 2014 16:57:51 GMT


> On Oct. 9, 2014, 3:49 a.m., Dominic Hamon wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/stringify.hpp, line 36
> > <https://reviews.apache.org/r/26472/diff/1/?file=716305#file716305line36>
> >
> >     it might be worth considering using stringstream here to build the same error
message we had before.
> >     
> >     we will have a stack trace, but having the thing that was attempting to stringify
could be useful too, i think.
> 
> Adam B wrote:
>     +1 on "having the thing that was attempting to stringify could be useful"
> 
> Cody Maloney wrote:
>     The thing is we would be doing the same operation which just failed in order to try
to print out T again. So in the failure case (Which is unlikely to fail unless you are OOM),
we have the operation fail, then try the failed thing again in order to provide debug information.
>     
>     I'm planning to make ABORT() give backtraces on debug builds, which should provide
a lot more information than this ever possibly did, without retrying something we know is
likely to fail because it just did.
> 
> Dominic Hamon wrote:
>     stringify would fail again, but we used to use ostream operators. We should then
be able to use stringstream to get some representation of t.
> 
> Cody Maloney wrote:
>     I can do typeid(t).name() which would give me the mangled name and use libcxxabi
(Available on all of our platforms), to demangle it to the human-friendly C++ type name, But
that is a lot of utility library to add for little benefit.
> 
> Dominic Hamon wrote:
>     std::ostringstream os;
>         os << t;
>         ABORT("Failed to stringify: " + os.str());
>         
>     is the equivalent of what was there.

That is what the function does, and what fails though (and causes the message to be printed).
The full function would be:
```
template <typename T>
std::string stringify(T t)
{
  std::ostringstream out;
  out << t;
  if (!out.good()) {
    ABORT("Failed to stringify!" + out.str());
  }
  return out.str();
}
```
I could do:
```
template <typename T>
std::string stringify(T t)
{
  std::ostringstream out;
  out << t;
  if (!out.good()) {
    ABORT(std::string("Failed to stringify!") + typeid(T).name());
  }
  return out.str();
}
```
Which would give the info people want probably, although the type name will be mangled per
http://mentorembedded.github.io/cxx-abi/abi.html#mangling


- Cody


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


On Oct. 9, 2014, 4:35 p.m., Cody Maloney wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26472/
> -----------------------------------------------------------
> 
> (Updated Oct. 9, 2014, 4:35 p.m.)
> 
> 
> Review request for mesos, Adam B and Dominic Hamon.
> 
> 
> Bugs: MESOS-1870
>     https://issues.apache.org/jira/browse/MESOS-1870
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> This makes it so any time there is an abort, we get a line number and at least a basic
message as to why there was an abort. If you want a clean(er) exit, use <stout/exit>.
> 
> Also adds an overload which takes a standard string and unwraps it to a const char *
automatically, since a lot of the time we are building strings to pass them to abort.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/abort.hpp 6b5b5d1faa488cb3048f6d59bae17123b1b05a33

>   3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp 9d244b2275f7ef84e8c486f2090b67a57ca8c670

>   3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp 7138bc24912f35bfd04d2341b2218ea349ab40b8

>   3rdparty/libprocess/3rdparty/stout/include/stout/os/fork.hpp 8aa21ed10293c3dd7f55b7d30f6014bd05fd7455

>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp ccf80a771c59092cb72d4dac0d868eec7df2f088

>   3rdparty/libprocess/3rdparty/stout/include/stout/result.hpp ce8dd9b6468fb36daceba60d8b43bf2fbfceab6c

>   3rdparty/libprocess/3rdparty/stout/include/stout/stringify.hpp ed0a1ef2b617bd87261bf4836706cedb80fdf043

>   3rdparty/libprocess/3rdparty/stout/include/stout/thread.hpp b1af74f517da75525d0dedacaf4a1c6f8cbf55f4

>   3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp 87c5fc8391d5213f0d76fa5980176726a0366f56

>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 9207c551170b747d25207efe7d3c03675b184953

> 
> Diff: https://reviews.apache.org/r/26472/diff/
> 
> 
> Testing
> -------
> 
> make distcheck
> 
> 
> Thanks,
> 
> Cody Maloney
> 
>


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