Re: [PATCH] D20903: Make system_error::message() thread safe. Fixes PR25598.

2016-06-14 Thread Erik Kessler via cfe-commits
erik65536 added a comment. I don't see any other issues. Thanks for fixing this. http://reviews.llvm.org/D20903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20903: Make system_error::message() thread safe. Fixes PR25598.

2016-06-14 Thread Eric Fiselier via cfe-commits
EricWF added a comment. I checked in tests for the reported bug in r272642. They aren't the most portable so hopefully the pass on all supported platforms. http://reviews.llvm.org/D20903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D20903: Make system_error::message() thread safe. Fixes PR25598.

2016-06-14 Thread Eric Fiselier via cfe-commits
EricWF added a comment. In http://reviews.llvm.org/D20903#457252, @erik65536 wrote: > > The POSIX version of strerror_r() returns 0 on success, and any other value > > indicates an error (Reference > > ). > > > I should

Re: [PATCH] D20903: Make system_error::message() thread safe. Fixes PR25598.

2016-06-14 Thread Erik Kessler via cfe-commits
erik65536 added a comment. > The POSIX version of strerror_r() returns 0 on success, and any other value > indicates an error (Reference > ). I should have been more explicit when I wrote this recommendation. Checking if

Re: [PATCH] D20903: Make system_error::message() thread safe. Fixes PR25598.

2016-06-13 Thread Eric Fiselier via cfe-commits
EricWF updated this revision to Diff 60649. EricWF added a comment. Use named variable `strerror_buff_size` variable instead of literal `1024`. http://reviews.llvm.org/D20903 Files: src/system_error.cpp

Re: [PATCH] D20903: Make system_error::message() thread safe. Fixes PR25598.

2016-06-13 Thread Marshall Clow via cfe-commits
mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. LGTM. http://reviews.llvm.org/D20903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D20903: Make system_error::message() thread safe. Fixes PR25598.

2016-06-13 Thread Eric Fiselier via cfe-commits
EricWF updated this revision to Diff 60642. EricWF added a comment. - Use a local buffer instead of a `static` `thread_local` one. - Save and restore errno in the POSIX implementation. http://reviews.llvm.org/D20903 Files: src/system_error.cpp

Re: [PATCH] D20903: Make system_error::message() thread safe. Fixes PR25598.

2016-06-13 Thread Eric Fiselier via cfe-commits
EricWF added a comment. In http://reviews.llvm.org/D20903#446922, @mclow.lists wrote: > In general, I'm OK with this - but I'm concerned about that there's not > really any provision for the case where `strerror_r` does not exist. > > Also, there's no reason to have a thread local static array

Re: [PATCH] D20903: Make system_error::message() thread safe. Fixes PR25598.

2016-06-02 Thread Erik Kessler via cfe-commits
erik65536 added a subscriber: erik65536. erik65536 added a comment. The C++ spec states that `error_category::message()` shall not change the value of `errno` (See section 19.5). So `errno` will have to be saved and restored if `strerror_r()` fails. The POSIX version of `strerror_r()` returns

Re: [PATCH] D20903: Make system_error::message() thread safe. Fixes PR25598.

2016-06-02 Thread Marshall Clow via cfe-commits
mclow.lists added a comment. In general, I'm OK with this - but I'm concerned about that there's not really any provision for the case where `strerror_r` does not exist. Also, there's no reason to have a thread local static array here, if you're going to immediately copy it into a

[PATCH] D20903: Make system_error::message() thread safe. Fixes PR25598.

2016-06-02 Thread Eric Fiselier via cfe-commits
EricWF created this revision. EricWF added reviewers: mclow.lists, majnemer. EricWF added a subscriber: cfe-commits. Herald added a subscriber: emaste. system_error::message() uses `strerror` for the generic and system categories. This function is not thread safe. The fix is to use