You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Andrei Budnik <ab...@mesosphere.com> on 2017/08/21 20:54:20 UTC

Review Request 61798: Added _EXIT as alternative to ABORT.

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

Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Benjamin Mahler, and James Peach.


Bugs: MESOS-7791
    https://issues.apache.org/jira/browse/MESOS-7791


Repository: mesos


Description
-------

EXIT macro can't be used when async-signal safety is required.

Currently, ABORT is used for terminating a program after both
unexpected and expected errors, when async-signal safety is required.
In case of expected errors, `abort` causes coredumps, which might be
undesirable.

In addition, _EXIT takes variable number of arguments, thereby _EXIT
interface doesn't force users to concatenate strings, thus making its
usage more async-signal safe.


Diffs
-----

  3rdparty/stout/include/stout/exit.hpp e5c2d3440a85ab2d2cae551ee4094cd965e38dfc 


Diff: https://reviews.apache.org/r/61798/diff/1/


Testing
-------

sudo make check (mac os x, fedora 25)
internal CI


Thanks,

Andrei Budnik


Re: Review Request 61798: Added SAFE_EXIT as alternative to ABORT.

Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61798/#review184786
-----------------------------------------------------------


Ship it!




Ship It!

- James Peach


On Aug. 21, 2017, 8:54 p.m., Andrei Budnik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61798/
> -----------------------------------------------------------
> 
> (Updated Aug. 21, 2017, 8:54 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Alexander Rojas, Benjamin Bannier, Benjamin Mahler, and James Peach.
> 
> 
> Bugs: MESOS-7791
>     https://issues.apache.org/jira/browse/MESOS-7791
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> EXIT macro can't be used when async-signal safety is required.
> 
> Currently, ABORT is used for terminating a program after both
> unexpected and expected errors, when async-signal safety is required.
> In case of expected errors, `abort` causes coredumps, which might be
> undesirable.
> 
> In addition, SAFE_EXIT supports formatted output conversion,
> thereby SAFE_EXIT interface doesn't force users to concatenate strings,
> thus making its usage more async-signal safe.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/exit.hpp e5c2d3440a85ab2d2cae551ee4094cd965e38dfc 
> 
> 
> Diff: https://reviews.apache.org/r/61798/diff/6/
> 
> 
> Testing
> -------
> 
> sudo make check (mac os x, fedora 25)
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>


Re: Review Request 61798: Added _EXIT as alternative to ABORT.

Posted by Andrei Budnik <ab...@mesosphere.com>.

> On Aug. 24, 2017, 10:59 p.m., James Peach wrote:
> > 3rdparty/stout/include/stout/exit.hpp
> > Lines 39 (patched)
> > <https://reviews.apache.org/r/61798/diff/2/?file=1802341#file1802341line39>
> >
> >     Move this comment to the `_EXIT()` macro.

Done.


> On Aug. 24, 2017, 10:59 p.m., James Peach wrote:
> > 3rdparty/stout/include/stout/exit.hpp
> > Lines 42 (patched)
> > <https://reviews.apache.org/r/61798/diff/2/?file=1802341#file1802341line42>
> >
> >     The `EXIT` macro produces something like this:
> >     ```
> >     E0824 15:33:17.790952 32725 main.cpp:497] EXIT with status 1: Failed to create a master detector: Failed to parse '///////'
> >     ```
> >     
> >     It would be good to make `_EXIT` consistent, and I think you can use the `RAW_LOG_XXX` interfaces in `glog/raw_logging.h` for this.

Fixed: `_EXIT` uses `RAW_LOG_XXX`.


> On Aug. 24, 2017, 10:59 p.m., James Peach wrote:
> > 3rdparty/stout/include/stout/exit.hpp
> > Lines 77 (patched)
> > <https://reviews.apache.org/r/61798/diff/2/?file=1802341#file1802341line77>
> >
> >     Lets be consistent with other code by using `::_exit(status)` here.

Fixed.
BTW, `std::_Exit` was introduced in c++11.


- Andrei


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


On Aug. 21, 2017, 8:54 p.m., Andrei Budnik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61798/
> -----------------------------------------------------------
> 
> (Updated Aug. 21, 2017, 8:54 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Alexander Rojas, Benjamin Bannier, Benjamin Mahler, and James Peach.
> 
> 
> Bugs: MESOS-7791
>     https://issues.apache.org/jira/browse/MESOS-7791
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> EXIT macro can't be used when async-signal safety is required.
> 
> Currently, ABORT is used for terminating a program after both
> unexpected and expected errors, when async-signal safety is required.
> In case of expected errors, `abort` causes coredumps, which might be
> undesirable.
> 
> In addition, _EXIT supports formatted output conversion,
> thereby _EXIT interface doesn't force users to concatenate strings,
> thus making its usage more async-signal safe.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/exit.hpp e5c2d3440a85ab2d2cae551ee4094cd965e38dfc 
> 
> 
> Diff: https://reviews.apache.org/r/61798/diff/3/
> 
> 
> Testing
> -------
> 
> sudo make check (mac os x, fedora 25)
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>


Re: Review Request 61798: Added _EXIT as alternative to ABORT.

Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61798/#review183755
-----------------------------------------------------------




3rdparty/stout/include/stout/exit.hpp
Lines 39 (patched)
<https://reviews.apache.org/r/61798/#comment259809>

    Move this comment to the `_EXIT()` macro.



3rdparty/stout/include/stout/exit.hpp
Lines 42 (patched)
<https://reviews.apache.org/r/61798/#comment259851>

    The `EXIT` macro produces something like this:
    ```
    E0824 15:33:17.790952 32725 main.cpp:497] EXIT with status 1: Failed to create a master detector: Failed to parse '///////'
    ```
    
    It would be good to make `_EXIT` consistent, and I think you can use the `RAW_LOG_XXX` interfaces in `glog/raw_logging.h` for this.



3rdparty/stout/include/stout/exit.hpp
Lines 77 (patched)
<https://reviews.apache.org/r/61798/#comment259811>

    Lets be consistent with other code by using `::_exit(status)` here.


- James Peach


On Aug. 21, 2017, 8:54 p.m., Andrei Budnik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61798/
> -----------------------------------------------------------
> 
> (Updated Aug. 21, 2017, 8:54 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Alexander Rojas, Benjamin Bannier, Benjamin Mahler, and James Peach.
> 
> 
> Bugs: MESOS-7791
>     https://issues.apache.org/jira/browse/MESOS-7791
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> EXIT macro can't be used when async-signal safety is required.
> 
> Currently, ABORT is used for terminating a program after both
> unexpected and expected errors, when async-signal safety is required.
> In case of expected errors, `abort` causes coredumps, which might be
> undesirable.
> 
> In addition, _EXIT takes variable number of arguments, thereby _EXIT
> interface doesn't force users to concatenate strings, thus making its
> usage more async-signal safe.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/exit.hpp e5c2d3440a85ab2d2cae551ee4094cd965e38dfc 
> 
> 
> Diff: https://reviews.apache.org/r/61798/diff/2/
> 
> 
> Testing
> -------
> 
> sudo make check (mac os x, fedora 25)
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>


Re: Review Request 61798: Added _EXIT as alternative to ABORT.

Posted by Andrei Budnik <ab...@mesosphere.com>.

> On Aug. 22, 2017, 10:46 a.m., Alexander Rukletsov wrote:
> > 3rdparty/stout/include/stout/exit.hpp
> > Lines 42 (patched)
> > <https://reviews.apache.org/r/61798/diff/1/?file=1801033#file1801033line42>
> >
> >     Shouldn't we use `CRLF` on Windows?

I can't find a new line macro. BTW, `ABORT` writes "\n" to stderr.


> On Aug. 22, 2017, 10:46 a.m., Alexander Rukletsov wrote:
> > 3rdparty/stout/include/stout/exit.hpp
> > Lines 76 (patched)
> > <https://reviews.apache.org/r/61798/diff/1/?file=1801033#file1801033line76>
> >
> >     Why don't you choose `stdout` vs. `stderr` based on `status` value similar to `__Exit`?

Fixed.


- Andrei


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


On Aug. 21, 2017, 8:54 p.m., Andrei Budnik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61798/
> -----------------------------------------------------------
> 
> (Updated Aug. 21, 2017, 8:54 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Alexander Rojas, Benjamin Bannier, Benjamin Mahler, and James Peach.
> 
> 
> Bugs: MESOS-7791
>     https://issues.apache.org/jira/browse/MESOS-7791
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> EXIT macro can't be used when async-signal safety is required.
> 
> Currently, ABORT is used for terminating a program after both
> unexpected and expected errors, when async-signal safety is required.
> In case of expected errors, `abort` causes coredumps, which might be
> undesirable.
> 
> In addition, _EXIT takes variable number of arguments, thereby _EXIT
> interface doesn't force users to concatenate strings, thus making its
> usage more async-signal safe.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/exit.hpp e5c2d3440a85ab2d2cae551ee4094cd965e38dfc 
> 
> 
> Diff: https://reviews.apache.org/r/61798/diff/2/
> 
> 
> Testing
> -------
> 
> sudo make check (mac os x, fedora 25)
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>


Re: Review Request 61798: Added _EXIT as alternative to ABORT.

Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61798/#review183462
-----------------------------------------------------------




3rdparty/stout/include/stout/exit.hpp
Lines 42 (patched)
<https://reviews.apache.org/r/61798/#comment259475>

    Shouldn't we use `CRLF` on Windows?



3rdparty/stout/include/stout/exit.hpp
Lines 76 (patched)
<https://reviews.apache.org/r/61798/#comment259476>

    Why don't you choose `stdout` vs. `stderr` based on `status` value similar to `__Exit`?


- Alexander Rukletsov


On Aug. 21, 2017, 8:54 p.m., Andrei Budnik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61798/
> -----------------------------------------------------------
> 
> (Updated Aug. 21, 2017, 8:54 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Benjamin Mahler, and James Peach.
> 
> 
> Bugs: MESOS-7791
>     https://issues.apache.org/jira/browse/MESOS-7791
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> EXIT macro can't be used when async-signal safety is required.
> 
> Currently, ABORT is used for terminating a program after both
> unexpected and expected errors, when async-signal safety is required.
> In case of expected errors, `abort` causes coredumps, which might be
> undesirable.
> 
> In addition, _EXIT takes variable number of arguments, thereby _EXIT
> interface doesn't force users to concatenate strings, thus making its
> usage more async-signal safe.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/exit.hpp e5c2d3440a85ab2d2cae551ee4094cd965e38dfc 
> 
> 
> Diff: https://reviews.apache.org/r/61798/diff/1/
> 
> 
> Testing
> -------
> 
> sudo make check (mac os x, fedora 25)
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>


Re: Review Request 61798: Added SAFE_EXIT as alternative to ABORT.

Posted by Andrei Budnik <ab...@mesosphere.com>.

> On Aug. 30, 2017, 6:36 a.m., Alexander Rojas wrote:
> > 3rdparty/stout/include/stout/exit.hpp
> > Lines 42 (patched)
> > <https://reviews.apache.org/r/61798/diff/4/?file=1807241#file1807241line42>
> >
> >     I believe the name of this macro needs to be changed. From the C standard section [7.1.3](http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf):
> >     
> >     > * All identifiers that begin with an underscore and either an uppercase letter or another
> >     underscore are always reserved for any use.
> >     > * All identifiers that begin with an underscore are always reserved for use as identifiers
> >     with file scope in both the ordinary and tag name spaces.
> >     
> >     Likewise, from C++ [perspective](http://en.cppreference.com/w/cpp/keyword):
> >     
> >     > Also, all identifiers that contain a double underscore __ in any position and each identifier that begins with an underscore followed by an uppercase letter is always reserved and all identifiers that begin with an underscore are reserved for use as names in the global namespace. See identifiers for more details.

Thanks for good observation!
`_EXIT` was renamed to `SAFE_EXIT`.


- Andrei


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


On Aug. 21, 2017, 8:54 p.m., Andrei Budnik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61798/
> -----------------------------------------------------------
> 
> (Updated Aug. 21, 2017, 8:54 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Alexander Rojas, Benjamin Bannier, Benjamin Mahler, and James Peach.
> 
> 
> Bugs: MESOS-7791
>     https://issues.apache.org/jira/browse/MESOS-7791
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> EXIT macro can't be used when async-signal safety is required.
> 
> Currently, ABORT is used for terminating a program after both
> unexpected and expected errors, when async-signal safety is required.
> In case of expected errors, `abort` causes coredumps, which might be
> undesirable.
> 
> In addition, SAFE_EXIT supports formatted output conversion,
> thereby SAFE_EXIT interface doesn't force users to concatenate strings,
> thus making its usage more async-signal safe.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/exit.hpp e5c2d3440a85ab2d2cae551ee4094cd965e38dfc 
> 
> 
> Diff: https://reviews.apache.org/r/61798/diff/5/
> 
> 
> Testing
> -------
> 
> sudo make check (mac os x, fedora 25)
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>


Re: Review Request 61798: Added SAFE_EXIT as alternative to ABORT.

Posted by James Peach <jp...@apache.org>.

> On Aug. 30, 2017, 6:36 a.m., Alexander Rojas wrote:
> > 3rdparty/stout/include/stout/exit.hpp
> > Lines 42 (patched)
> > <https://reviews.apache.org/r/61798/diff/4/?file=1807241#file1807241line42>
> >
> >     I believe the name of this macro needs to be changed. From the C standard section [7.1.3](http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf):
> >     
> >     > * All identifiers that begin with an underscore and either an uppercase letter or another
> >     underscore are always reserved for any use.
> >     > * All identifiers that begin with an underscore are always reserved for use as identifiers
> >     with file scope in both the ordinary and tag name spaces.
> >     
> >     Likewise, from C++ [perspective](http://en.cppreference.com/w/cpp/keyword):
> >     
> >     > Also, all identifiers that contain a double underscore __ in any position and each identifier that begins with an underscore followed by an uppercase letter is always reserved and all identifiers that begin with an underscore are reserved for use as names in the global namespace. See identifiers for more details.
> 
> Andrei Budnik wrote:
>     Thanks for good observation!
>     `_EXIT` was renamed to `SAFE_EXIT`.

`SAFE_EXIT` is fine by me, but it is a little late to avoid leading underscore identifiers in Mesos :)


- James


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


On Aug. 21, 2017, 8:54 p.m., Andrei Budnik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61798/
> -----------------------------------------------------------
> 
> (Updated Aug. 21, 2017, 8:54 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Alexander Rojas, Benjamin Bannier, Benjamin Mahler, and James Peach.
> 
> 
> Bugs: MESOS-7791
>     https://issues.apache.org/jira/browse/MESOS-7791
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> EXIT macro can't be used when async-signal safety is required.
> 
> Currently, ABORT is used for terminating a program after both
> unexpected and expected errors, when async-signal safety is required.
> In case of expected errors, `abort` causes coredumps, which might be
> undesirable.
> 
> In addition, SAFE_EXIT supports formatted output conversion,
> thereby SAFE_EXIT interface doesn't force users to concatenate strings,
> thus making its usage more async-signal safe.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/exit.hpp e5c2d3440a85ab2d2cae551ee4094cd965e38dfc 
> 
> 
> Diff: https://reviews.apache.org/r/61798/diff/5/
> 
> 
> Testing
> -------
> 
> sudo make check (mac os x, fedora 25)
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>


Re: Review Request 61798: Added _EXIT as alternative to ABORT.

Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61798/#review184130
-----------------------------------------------------------




3rdparty/stout/include/stout/exit.hpp
Lines 42 (patched)
<https://reviews.apache.org/r/61798/#comment260194>

    I believe the name of this macro needs to be changed. From the C standard section [7.1.3](http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf):
    
    > * All identifiers that begin with an underscore and either an uppercase letter or another
    underscore are always reserved for any use.
    > * All identifiers that begin with an underscore are always reserved for use as identifiers
    with file scope in both the ordinary and tag name spaces.
    
    Likewise, from C++ [perspective](http://en.cppreference.com/w/cpp/keyword):
    
    > Also, all identifiers that contain a double underscore __ in any position and each identifier that begins with an underscore followed by an uppercase letter is always reserved and all identifiers that begin with an underscore are reserved for use as names in the global namespace. See identifiers for more details.


- Alexander Rojas


On Aug. 21, 2017, 10:54 p.m., Andrei Budnik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61798/
> -----------------------------------------------------------
> 
> (Updated Aug. 21, 2017, 10:54 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Alexander Rojas, Benjamin Bannier, Benjamin Mahler, and James Peach.
> 
> 
> Bugs: MESOS-7791
>     https://issues.apache.org/jira/browse/MESOS-7791
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> EXIT macro can't be used when async-signal safety is required.
> 
> Currently, ABORT is used for terminating a program after both
> unexpected and expected errors, when async-signal safety is required.
> In case of expected errors, `abort` causes coredumps, which might be
> undesirable.
> 
> In addition, _EXIT supports formatted output conversion,
> thereby _EXIT interface doesn't force users to concatenate strings,
> thus making its usage more async-signal safe.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/exit.hpp e5c2d3440a85ab2d2cae551ee4094cd965e38dfc 
> 
> 
> Diff: https://reviews.apache.org/r/61798/diff/4/
> 
> 
> Testing
> -------
> 
> sudo make check (mac os x, fedora 25)
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>


Re: Review Request 61798: Added _EXIT as alternative to ABORT.

Posted by Andrei Budnik <ab...@mesosphere.com>.

> On Aug. 29, 2017, 2:47 p.m., James Peach wrote:
> > 3rdparty/stout/include/stout/exit.hpp
> > Lines 37 (patched)
> > <https://reviews.apache.org/r/61798/diff/3/?file=1803749#file1803749line37>
> >
> >     What do you think about re-declaring `google::RawLog__` here with the [format attribute](https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-format-function-attribute)?
> >     
> >     It is a little gnarly, but worth it IME.

`RAW_LOG` (https://github.com/google/glog/blob/master/src/glog/raw_logging.h.in#L172) checks format attributes already.


> On Aug. 29, 2017, 2:47 p.m., James Peach wrote:
> > 3rdparty/stout/include/stout/exit.hpp
> > Lines 44 (patched)
> > <https://reviews.apache.org/r/61798/diff/3/?file=1803749#file1803749line44>
> >
> >     Can we make this emit the same format as `EXIT()`?
> >     
> >     ```
> >     #define _Exit(status, fmt, ...)
> >     ...
> >       RAW_LOG(level, "Exit with status %d: " fmt, status ...);
> >     ...
> >     ```

Done.


- Andrei


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


On Aug. 21, 2017, 8:54 p.m., Andrei Budnik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61798/
> -----------------------------------------------------------
> 
> (Updated Aug. 21, 2017, 8:54 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Alexander Rojas, Benjamin Bannier, Benjamin Mahler, and James Peach.
> 
> 
> Bugs: MESOS-7791
>     https://issues.apache.org/jira/browse/MESOS-7791
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> EXIT macro can't be used when async-signal safety is required.
> 
> Currently, ABORT is used for terminating a program after both
> unexpected and expected errors, when async-signal safety is required.
> In case of expected errors, `abort` causes coredumps, which might be
> undesirable.
> 
> In addition, _EXIT supports formatted output conversion,
> thereby _EXIT interface doesn't force users to concatenate strings,
> thus making its usage more async-signal safe.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/exit.hpp e5c2d3440a85ab2d2cae551ee4094cd965e38dfc 
> 
> 
> Diff: https://reviews.apache.org/r/61798/diff/4/
> 
> 
> Testing
> -------
> 
> sudo make check (mac os x, fedora 25)
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>


Re: Review Request 61798: Added _EXIT as alternative to ABORT.

Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61798/#review184054
-----------------------------------------------------------


Fix it, then Ship it!





3rdparty/stout/include/stout/exit.hpp
Lines 37 (patched)
<https://reviews.apache.org/r/61798/#comment260059>

    What do you think about re-declaring `google::RawLog__` here with the [format attribute](https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-format-function-attribute)?
    
    It is a little gnarly, but worth it IME.



3rdparty/stout/include/stout/exit.hpp
Lines 44 (patched)
<https://reviews.apache.org/r/61798/#comment260057>

    Can we make this emit the same format as `EXIT()`?
    
    ```
    #define _Exit(status, fmt, ...)
    ...
      RAW_LOG(level, "Exit with status %d: " fmt, status ...);
    ...
    ```


- James Peach


On Aug. 21, 2017, 8:54 p.m., Andrei Budnik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61798/
> -----------------------------------------------------------
> 
> (Updated Aug. 21, 2017, 8:54 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Alexander Rojas, Benjamin Bannier, Benjamin Mahler, and James Peach.
> 
> 
> Bugs: MESOS-7791
>     https://issues.apache.org/jira/browse/MESOS-7791
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> EXIT macro can't be used when async-signal safety is required.
> 
> Currently, ABORT is used for terminating a program after both
> unexpected and expected errors, when async-signal safety is required.
> In case of expected errors, `abort` causes coredumps, which might be
> undesirable.
> 
> In addition, _EXIT supports formatted output conversion,
> thereby _EXIT interface doesn't force users to concatenate strings,
> thus making its usage more async-signal safe.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/exit.hpp e5c2d3440a85ab2d2cae551ee4094cd965e38dfc 
> 
> 
> Diff: https://reviews.apache.org/r/61798/diff/3/
> 
> 
> Testing
> -------
> 
> sudo make check (mac os x, fedora 25)
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>