On Thu, Oct 8, 2020 at 4:49 PM Chris Johns <chr...@rtems.org> wrote:
>
> On 9/10/20 3:28 am, Gedare Bloom wrote:
> > On Thu, Oct 8, 2020 at 2:19 AM <chr...@rtems.org> wrote:
> >>
> >> From: Chris Johns <chr...@rtems.org>
> >>
> >> - Do not start threads detached
> >> ---
> >> cpukit/include/rtems/thread.hpp | 4 ++++
> >> cpukit/librtemscxx/thread.cpp | 22 ++++++++++++++++++----
> >> 2 files changed, 22 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/cpukit/include/rtems/thread.hpp
> >> b/cpukit/include/rtems/thread.hpp
> >> index e90e664dfa..2c4899dc0b 100644
> >> --- a/cpukit/include/rtems/thread.hpp
> >> +++ b/cpukit/include/rtems/thread.hpp
> >> @@ -321,6 +321,10 @@ namespace rtems
> >>
> >> bool joinable() const noexcept;
> >>
> >> + void join() noexcept;
> >> +
> >> + void detach() noexcept;
>
> Hmmm I the `noexcept` is wrong here as an exception may be raised. I will
> remove
> these.
>
> >> +
> >> /*
> >> * Constrain use. These are not available.
> >> */
> >> diff --git a/cpukit/librtemscxx/thread.cpp b/cpukit/librtemscxx/thread.cpp
> >> index 11bf3df230..ae13201bd4 100644
> >> --- a/cpukit/librtemscxx/thread.cpp
> >> +++ b/cpukit/librtemscxx/thread.cpp
> >> @@ -346,6 +346,24 @@ namespace rtems
> >> return !(id_ == id());
> >> }
> >>
> >> + void
> >> + thread::join() noexcept
> >> + {
> >> + if (!joinable())
> > explicit { for 1-liners is preferred
>
> Yes ... old habits ...
>
> >> + system_error_check (ENOMEM, "join");
> > }
> >
> > In general, this file is not entirely adhering to RTEMS coding style.
> > What coding style is it? Sorry I missed this in the first patch(es)
> > adding the cxx iface
>
> It does not entirely follow the coding style as it is C++.
>
> The code was first posted in December 2019 and goes back further as I played.
> The C++ makes use of advanced template techniques, nesting classes and
> structs,
> namespace, exceptions and more. Some of the coding standard is fine and other
> parts do not work and do not relate. I have not checked in detail.
>
I don't mind having some C++ specific rules. Maybe we can derive
something later if this keeps growing :)
> >> + system_error_check (::pthread_join (id_.id_, nullptr), "join");
> > prefer to not have a space before the ( for function calls so that
> > function-like macros appear consistently
>
> Interesting, this is something the GNU coding standard recommends as being
> more
> readable [1]. I did not know it was a requirement in the coding standard. I am
> not fused and can fix this but it would take a little time so let me know what
> you would like me to do? Also if you have other places that need attention
> please let me know.
You can push this with your corrections since it is self-consistent.
Maybe open a ticket for yourself to review the style later.
>
> Thanks
> Chris
>
> [1] https://www.gnu.org/prep/standards/html_node/Formatting.html#Formatting
> It is interesting how we directly conflict with some of what it says
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
