Re: [PATCH] score: Document thread queue operations

[Sebastian Huber]

Hello Chris,

thanks for the review, I will send a v2 of the patch.

On 02/09/2021 08:38, Chris Johns wrote:

 */
Thread_queue_Enqueue_operation enqueue;
  
/**

-   * @brief Thread queue extract operation.
+   * @brief This operation is used to extract the thread from the thread queue.
 *
-   * Called by object routines to extract a thread from a thread queue.
+   * The extract operation is intended for timeouts, thread restarts, and
+   * thread cancellation.  The surrender operation should be used to dequeue a
+   * thread from the thread queue.

 From the start, end, or where it is in the queue?



I think this is clear:

"This operation is used to extract the thread from the thread queue."

A thread is extracted. It doesn't matter at which position it is.

--
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.hu...@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

[PATCH v2] score: Document thread queue operations

[Sebastian Huber]

---
 cpukit/include/rtems/score/threadq.h | 48 +++-
 cpukit/include/rtems/score/threadqimpl.h | 21 +++
 cpukit/score/src/threadqops.c|  4 +-
 3 files changed, 62 insertions(+), 11 deletions(-)

diff --git a/cpukit/include/rtems/score/threadq.h 
b/cpukit/include/rtems/score/threadq.h
index 10476888d4..9d1ad84e81 100644
--- a/cpukit/include/rtems/score/threadq.h
+++ b/cpukit/include/rtems/score/threadq.h
@@ -406,6 +406,7 @@ typedef struct _Thread_queue_Heads {
 } Thread_queue_Heads;
 
 struct Thread_queue_Queue {
+#if defined(RTEMS_SMP)
   /**
* @brief Lock to protect this thread queue.
*
@@ -418,7 +419,6 @@ struct Thread_queue_Queue {
* @see _Thread_queue_Acquire(), _Thread_queue_Acquire_critical() and
* _Thread_queue_Release().
*/
-#if defined(RTEMS_SMP)
   SMP_ticket_lock_Control Lock;
 #endif
 
@@ -517,37 +517,67 @@ typedef Thread_Control *( *Thread_queue_First_operation )(
 );
 
 /**
- * @brief Thread queue operations.
+ * @brief The thread queue operations are used to manage the threads of a
+ *   thread queue.
+ *
+ * The standard thread queue operation variants are:
+ *
+ * * ::_Thread_queue_Operations_default
+ *
+ * * ::_Thread_queue_Operations_FIFO
+ *
+ * * ::_Thread_queue_Operations_priority
+ *
+ * * ::_Thread_queue_Operations_priority_inherit
  *
  * @see _Thread_wait_Set_operations().
  */
 struct Thread_queue_Operations {
   /**
-   * @brief Thread queue priority actions operation.
+   * @brief This operation performs the thread queue priority actions.
+   *
+   * Priority actions are produced and processed during enqueue, extract, and
+   * surrender operations.
*/
   Thread_queue_Priority_actions_operation priority_actions;
 
   /**
-   * @brief Thread queue enqueue operation.
+   * @brief This operation is used to enqueue the thread on the thread queue.
*
-   * Called by object routines to enqueue the thread.
+   * The enqueue order is defined by the operations variant.
*/
   Thread_queue_Enqueue_operation enqueue;
 
   /**
-   * @brief Thread queue extract operation.
+   * @brief This operation is used to extract the thread from the thread queue.
*
-   * Called by object routines to extract a thread from a thread queue.
+   * The extract operation is intended for timeouts, thread restarts, and
+   * thread cancellation.  In SMP configurations, the extract operation does
+   * not ensure FIFO fairness across schedulers for priority queues.  The
+   * surrender operation should be used to dequeue a thread from the thread
+   * queue under normal conditions (no timeout, no thread restart, and no
+   * thread cancellation).
*/
   Thread_queue_Extract_operation extract;
 
   /**
-   * @brief Thread queue surrender operation.
+   * @brief This operation is used to dequeue the thread from the thread queue
+   *   and optionally surrender the thread queue from a previous owner to the
+   *   thread.
+   *
+   * In addition to the optional surrender, there is a subtle difference
+   * between the extract and dequeue of a thread from a thread queue.  In SMP
+   * configurations, FIFO fairness across schedulers for priority queues is
+   * only ensured for the surrender operation and not for the extract 
operation.
*/
   Thread_queue_Surrender_operation surrender;
 
   /**
-   * @brief Thread queue first operation.
+   * @brief This operation returns the first thread on the thread queue.
+   *
+   * This operation may be called only when the thread queue contains at least
+   * one thread.  Use ::Thread_queue_Queue::heads to determine if a thread
+   * queue is empty.
*/
   Thread_queue_First_operation first;
 };
diff --git a/cpukit/include/rtems/score/threadqimpl.h 
b/cpukit/include/rtems/score/threadqimpl.h
index 33cdb3058d..22e0c7f069 100644
--- a/cpukit/include/rtems/score/threadqimpl.h
+++ b/cpukit/include/rtems/score/threadqimpl.h
@@ -1396,12 +1396,33 @@ typedef struct {
 Wait_queue.Queue \
   )
 
+/**
+ * @brief The default thread queue operations are used when a thread is not
+ *   enqueued on a thread queue.
+ *
+ * The default operations may be used by _Thread_Priority_apply() and
+ * _Thread_Continue() if the thread is not enqueued on a thread queue.  The
+ * default operations do nothing.
+ */
 extern const Thread_queue_Operations _Thread_queue_Operations_default;
 
+/**
+ * @brief The FIFO thread queue operations are used when a thread is enqueued
+ *   on a thread queue and provide FIFO ordering of enqueued threads.
+ */
 extern const Thread_queue_Operations _Thread_queue_Operations_FIFO;
 
+/**
+ * @brief The FIFO thread queue operations are used when a thread is enqueued
+ *   on a thread queue and provide priority ordering of enqueued threads.
+ */
 extern const Thread_queue_Operations _Thread_queue_Operations_priority;
 
+/**
+ * @brief The FIFO thread queue operations are used when a thread is enqueued
+ *   on a thread queue and provide priority ordering of enqueued threads with
+ *  

Re: [PATCH] score: Document thread queue operations

[Chris Johns]

On 2/9/21 5:09 pm, Sebastian Huber wrote:
> Hello Chris,
> 
> thanks for the review, I will send a v2 of the patch.
> 
> On 02/09/2021 08:38, Chris Johns wrote:
>>>  */
>>>     Thread_queue_Enqueue_operation enqueue;
>>>       /**
>>> -   * @brief Thread queue extract operation.
>>> +   * @brief This operation is used to extract the thread from the thread 
>>> queue.
>>>  *
>>> -   * Called by object routines to extract a thread from a thread queue.
>>> +   * The extract operation is intended for timeouts, thread restarts, and
>>> +   * thread cancellation.  The surrender operation should be used to 
>>> dequeue a
>>> +   * thread from the thread queue.
>>  From the start, end, or where it is in the queue?
>>
> 
> I think this is clear:
> 
> "This operation is used to extract the thread from the thread queue."
> 
> A thread is extracted. It doesn't matter at which position it is.

It may if you know the signature. I do not know the signature nor does it say.
As a result I question it being "clear".

Chris
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [PATCH v2] score: Document thread queue operations

[Chris Johns]

Looks good. Thanks.

Chris

On 2/9/21 5:13 pm, Sebastian Huber wrote:
> ---
>  cpukit/include/rtems/score/threadq.h | 48 +++-
>  cpukit/include/rtems/score/threadqimpl.h | 21 +++
>  cpukit/score/src/threadqops.c|  4 +-
>  3 files changed, 62 insertions(+), 11 deletions(-)
> 
> diff --git a/cpukit/include/rtems/score/threadq.h 
> b/cpukit/include/rtems/score/threadq.h
> index 10476888d4..9d1ad84e81 100644
> --- a/cpukit/include/rtems/score/threadq.h
> +++ b/cpukit/include/rtems/score/threadq.h
> @@ -406,6 +406,7 @@ typedef struct _Thread_queue_Heads {
>  } Thread_queue_Heads;
>  
>  struct Thread_queue_Queue {
> +#if defined(RTEMS_SMP)
>/**
> * @brief Lock to protect this thread queue.
> *
> @@ -418,7 +419,6 @@ struct Thread_queue_Queue {
> * @see _Thread_queue_Acquire(), _Thread_queue_Acquire_critical() and
> * _Thread_queue_Release().
> */
> -#if defined(RTEMS_SMP)
>SMP_ticket_lock_Control Lock;
>  #endif
>  
> @@ -517,37 +517,67 @@ typedef Thread_Control *( *Thread_queue_First_operation 
> )(
>  );
>  
>  /**
> - * @brief Thread queue operations.
> + * @brief The thread queue operations are used to manage the threads of a
> + *   thread queue.
> + *
> + * The standard thread queue operation variants are:
> + *
> + * * ::_Thread_queue_Operations_default
> + *
> + * * ::_Thread_queue_Operations_FIFO
> + *
> + * * ::_Thread_queue_Operations_priority
> + *
> + * * ::_Thread_queue_Operations_priority_inherit
>   *
>   * @see _Thread_wait_Set_operations().
>   */
>  struct Thread_queue_Operations {
>/**
> -   * @brief Thread queue priority actions operation.
> +   * @brief This operation performs the thread queue priority actions.
> +   *
> +   * Priority actions are produced and processed during enqueue, extract, and
> +   * surrender operations.
> */
>Thread_queue_Priority_actions_operation priority_actions;
>  
>/**
> -   * @brief Thread queue enqueue operation.
> +   * @brief This operation is used to enqueue the thread on the thread queue.
> *
> -   * Called by object routines to enqueue the thread.
> +   * The enqueue order is defined by the operations variant.
> */
>Thread_queue_Enqueue_operation enqueue;
>  
>/**
> -   * @brief Thread queue extract operation.
> +   * @brief This operation is used to extract the thread from the thread 
> queue.
> *
> -   * Called by object routines to extract a thread from a thread queue.
> +   * The extract operation is intended for timeouts, thread restarts, and
> +   * thread cancellation.  In SMP configurations, the extract operation does
> +   * not ensure FIFO fairness across schedulers for priority queues.  The
> +   * surrender operation should be used to dequeue a thread from the thread
> +   * queue under normal conditions (no timeout, no thread restart, and no
> +   * thread cancellation).
> */
>Thread_queue_Extract_operation extract;
>  
>/**
> -   * @brief Thread queue surrender operation.
> +   * @brief This operation is used to dequeue the thread from the thread 
> queue
> +   *   and optionally surrender the thread queue from a previous owner to the
> +   *   thread.
> +   *
> +   * In addition to the optional surrender, there is a subtle difference
> +   * between the extract and dequeue of a thread from a thread queue.  In SMP
> +   * configurations, FIFO fairness across schedulers for priority queues is
> +   * only ensured for the surrender operation and not for the extract 
> operation.
> */
>Thread_queue_Surrender_operation surrender;
>  
>/**
> -   * @brief Thread queue first operation.
> +   * @brief This operation returns the first thread on the thread queue.
> +   *
> +   * This operation may be called only when the thread queue contains at 
> least
> +   * one thread.  Use ::Thread_queue_Queue::heads to determine if a thread
> +   * queue is empty.
> */
>Thread_queue_First_operation first;
>  };
> diff --git a/cpukit/include/rtems/score/threadqimpl.h 
> b/cpukit/include/rtems/score/threadqimpl.h
> index 33cdb3058d..22e0c7f069 100644
> --- a/cpukit/include/rtems/score/threadqimpl.h
> +++ b/cpukit/include/rtems/score/threadqimpl.h
> @@ -1396,12 +1396,33 @@ typedef struct {
>  Wait_queue.Queue \
>)
>  
> +/**
> + * @brief The default thread queue operations are used when a thread is not
> + *   enqueued on a thread queue.
> + *
> + * The default operations may be used by _Thread_Priority_apply() and
> + * _Thread_Continue() if the thread is not enqueued on a thread queue.  The
> + * default operations do nothing.
> + */
>  extern const Thread_queue_Operations _Thread_queue_Operations_default;
>  
> +/**
> + * @brief The FIFO thread queue operations are used when a thread is enqueued
> + *   on a thread queue and provide FIFO ordering of enqueued threads.
> + */
>  extern const Thread_queue_Operations _Thread_queue_Operations_FIFO;
>  
> +/**
> + * @brief The FIFO thread queue operations are used when 

Re: [PATCH rtems-libbsd] imx: Remove ccm functions alredy defined in RTEMS

[Christian MAUDERER]

Am 02.09.21 um 08:44 schrieb Christian MAUDERER:

Am 02.09.21 um 08:40 schrieb Chris Johns:



On 2/9/21 4:35 pm, Christian MAUDERER wrote:

Am 02.09.21 um 04:57 schrieb Chris Johns:

On 1/9/21 4:24 pm, Christian MAUDERER wrote:

Hello Gedare and Chris,

Am 01.09.21 um 07:55 schrieb Chris Johns:

On 1/9/21 7:26 am, Gedare Bloom wrote:
Sorry, i think libbsd is still a bit slushy, wait for Chris to ok 
thx


I planned to wait for Chris work anyway.


I have pushed the changes so this change is OK to push. Thank you 
for your

patience and consideration.

Chris


Hello Chris,

that's great news. Thanks a lot for all your great improvements.

I noted that currently your patches are only on 6-freebsd-12. Do you 
plan to

have the same (or a similar) patch set on master too? Background of the
question: Normally I would push the patch to both branches. But if 
you have a
similar patch set pending for master too, I think it would be a bad 
timing if I

push my patch now.


Please push to master. It can be handled as part of that task.


OK. Thanks.

Best regards

Christian



And already a question regarding the new branch: You cleaned up the 
rtems-bsd-kernel-namespace.h (which is great).


My patch removes some duplicated functions that are in RTEMS and in 
libbsd. The ones in RTEMS are definitively the correct ones and the ones 
in libbsd return wrong values sometimes. With your patch, the functions 
are now in the namespace header (which is OK).


I tried to regenerate the header using the rtems-kern-symbols. But even 
with the --regenerate option like described in the CONTRIBUTING.md, the 
symbols are not removed. I can remove them manually and the 
rtems-kern-symbols script doesn't change the header if I re-run it. So 
I'm quite sure that this is an OK solution. But it somehow feels wrong 
to manually change a generated header. Is there something I used wrong?


What I did:

- Re-build libbsd and note that the tests don't link any more.

- run `./rtems-kern-symbols --regenerate --rtems-tools=`
  Nothing has been changed.

- manually remove the symbols

- re-build (now successfully)

- run `./rtems-kern-symbols --regenerate --rtems-tools=`
  Nothing has been changed. Symbols are still removed.

- add symbols manually again

- run `./rtems-kern-symbols --regenerate --rtems-tools=`
  Nothing has been changed. Symbols are still there.

So I think my core question is: Does the tool clean up old symbols or do 
I have to do that manually?


Best regards

Christian
--

embedded brains GmbH
Herr Christian MAUDERER
Dornierstr. 4
82178 Puchheim
Germany
email: christian.maude...@embedded-brains.de
phone: +49-89-18 94 741 - 18
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [PATCH rtems-libbsd] imx: Remove ccm functions alredy defined in RTEMS

[Chris Johns]

On 2/9/21 5:18 pm, Christian MAUDERER wrote:
> And already a question regarding the new branch: You cleaned up the
> rtems-bsd-kernel-namespace.h (which is great).

Thanks. It came as a surprise the script broke on FreeBSD.

> My patch removes some duplicated functions that are in RTEMS and in libbsd. 
> The
> ones in RTEMS are definitively the correct ones and the ones in libbsd return
> wrong values sometimes. With your patch, the functions are now in the 
> namespace
> header (which is OK).

OK

> I tried to regenerate the header using the rtems-kern-symbols. But even with 
> the
> --regenerate option like described in the CONTRIBUTING.md, the symbols are not
> removed. I can remove them manually and the rtems-kern-symbols script doesn't
> change the header if I re-run it. So I'm quite sure that this is an OK 
> solution.
> But it somehow feels wrong to manually change a generated header. Is there
> something I used wrong?

No nothing at all.

> What I did:
> 
> - Re-build libbsd and note that the tests don't link any more.
> 
> - run `./rtems-kern-symbols --regenerate --rtems-tools=`
>   Nothing has been changed.
> 
> - manually remove the symbols
> 
> - re-build (now successfully)
> 
> - run `./rtems-kern-symbols --regenerate --rtems-tools=`
>   Nothing has been changed. Symbols are still removed.
> 
> - add symbols manually again
> 
> - run `./rtems-kern-symbols --regenerate --rtems-tools=`
>   Nothing has been changed. Symbols are still there.
> 
> So I think my core question is: Does the tool clean up old symbols or do I 
> have
> to do that manually?

You need to remove the lines manually. I could not find a suitable solution that
worked effective and efficiently.

I spent sometime on this and considered a number of options for a while and came
to no workable solution to handle it automatically. An automatic tool that can
remove calls needs to build all combinations of arch/bsps multiplied by all the
possible options for RTEMS and the BSPs multiplied by all libbsd options to know
a symbol is not needed. That was all way too hard.

My compromise solution was adding a report option to help you see what is
happening with the symbols. The intention is to aid managing those symbols you
are interested in.

Chris
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [PATCH rtems-libbsd] imx: Remove ccm functions alredy defined in RTEMS

[Christian MAUDERER]

Hello Chris,

Am 02.09.21 um 09:33 schrieb Chris Johns:

On 2/9/21 5:18 pm, Christian MAUDERER wrote:

And already a question regarding the new branch: You cleaned up the
rtems-bsd-kernel-namespace.h (which is great).


Thanks. It came as a surprise the script broke on FreeBSD.


My patch removes some duplicated functions that are in RTEMS and in libbsd. The
ones in RTEMS are definitively the correct ones and the ones in libbsd return
wrong values sometimes. With your patch, the functions are now in the namespace
header (which is OK).


OK


I tried to regenerate the header using the rtems-kern-symbols. But even with the
--regenerate option like described in the CONTRIBUTING.md, the symbols are not
removed. I can remove them manually and the rtems-kern-symbols script doesn't
change the header if I re-run it. So I'm quite sure that this is an OK solution.
But it somehow feels wrong to manually change a generated header. Is there
something I used wrong?


No nothing at all.


What I did:

- Re-build libbsd and note that the tests don't link any more.

- run `./rtems-kern-symbols --regenerate --rtems-tools=`
   Nothing has been changed.

- manually remove the symbols

- re-build (now successfully)

- run `./rtems-kern-symbols --regenerate --rtems-tools=`
   Nothing has been changed. Symbols are still removed.

- add symbols manually again

- run `./rtems-kern-symbols --regenerate --rtems-tools=`
   Nothing has been changed. Symbols are still there.

So I think my core question is: Does the tool clean up old symbols or do I have
to do that manually?


You need to remove the lines manually. I could not find a suitable solution that
worked effective and efficiently.

I spent sometime on this and considered a number of options for a while and came
to no workable solution to handle it automatically. An automatic tool that can
remove calls needs to build all combinations of arch/bsps multiplied by all the
possible options for RTEMS and the BSPs multiplied by all libbsd options to know
a symbol is not needed. That was all way too hard.


OK. Good reason.



My compromise solution was adding a report option to help you see what is
happening with the symbols. The intention is to aid managing those symbols you
are interested in.



After you explained it here I have found the right paragraph in the new 
parts of the CONTRIBUTING.md where you already documented that. I should 
have read the new parts more thoroughly.


Thanks for the explanation. I'll update the patch accordingly and push 
it to 6-freebsd-12 too.


Best regards

Christian


Chris



--

embedded brains GmbH
Herr Christian MAUDERER
Dornierstr. 4
82178 Puchheim
Germany
email: christian.maude...@embedded-brains.de
phone: +49-89-18 94 741 - 18
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [PATCH rtems-libbsd] imx: Remove ccm functions alredy defined in RTEMS

[Chris Johns]

On 2/9/21 5:40 pm, Christian MAUDERER wrote:
> Am 02.09.21 um 09:33 schrieb Chris Johns:
>> On 2/9/21 5:18 pm, Christian MAUDERER wrote:
>> My compromise solution was adding a report option to help you see what is
>> happening with the symbols. The intention is to aid managing those symbols 
>> you
>> are interested in.
> 
> After you explained it here I have found the right paragraph in the new parts 
> of
> the CONTRIBUTING.md where you already documented that. I should have read the
> new parts more thoroughly.

No problem.

> Thanks for the explanation. I'll update the patch accordingly and push it to
> 6-freebsd-12 too.

Please do. I hope the new way of handling this lets us manage these symbols 
better.

Chris
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

[PATCH] c-user: Add LIFO to glossary

[Sebastian Huber]

---
 c-user/glossary.rst | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/c-user/glossary.rst b/c-user/glossary.rst
index 99f7c60..63876ac 100644
--- a/c-user/glossary.rst
+++ b/c-user/glossary.rst
@@ -334,7 +334,8 @@ Glossary
 This term is an acronym for :term:`First In First Out`.
 
 First In First Out
-A discipline for manipulating entries in a data structure.
+A discipline for manipulating entries in a data structure.  See also
+:term:`Last In First Out`.
 
 floating point coprocessor
 A component used in computer systems to enhance performance in
@@ -460,6 +461,13 @@ Glossary
 kernel
 In this document, this term is used as a synonym for executive.
 
+Last In First Out
+A discipline for manipulating entries in a data structure.  See also
+:term:`First In First Out`.
+
+LIFO
+This term is an acronym for :term:`Last In First Out`.
+
 list
 A data structure which allows for dynamic addition and removal of
 entries.  It is not statically limited to a particular size.
-- 
2.31.1

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

[PATCH] eng: Remove bogus coding rule

[Sebastian Huber]

Declaring functions used only through function pointers as inline makes
absolutely no sense at all.
---
 eng/coding-conventions.rst | 2 --
 1 file changed, 2 deletions(-)

diff --git a/eng/coding-conventions.rst b/eng/coding-conventions.rst
index 668a917..575dd44 100644
--- a/eng/coding-conventions.rst
+++ b/eng/coding-conventions.rst
@@ -219,8 +219,6 @@ Performance
 * Understand the constraints of `real-time programming 
`_..
   Limit execution times in interrupt contexts and critical sections,
   such as Interrupt and Timer Service Routines (TSRs).
-* Functions used only through function pointers should be declared
-  'static inline' (RTEMS_INLINE_ROUTINE)
 * Prefer to ++preincrement instead of postincrement++.
 * Avoid using floating point except where absolutely necessary.
 
-- 
2.31.1

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

RE: [PATCH v1 3/3] rld-dwarf.cpp: Initialize member variables

[Ryan Long]

I remember prototyped_ being one that the issue was complaining about, but I 
can't remember the others. I just went ahead and added the rest of the member 
variables because Coverity doesn't always identify all the subsequent issues.

-Original Message-
From: Chris Johns  
Sent: Wednesday, September 1, 2021 10:01 PM
To: Ryan Long ; devel@rtems.org
Subject: Re: [PATCH v1 3/3] rld-dwarf.cpp: Initialize member variables

On 25/8/21 3:12 am, Ryan Long wrote:
> Initialize member variables not listed.
> 
> CID 1503019: Uninitialized scalar field.
> 
> Closes #4500
> ---
>  rtemstoolkit/rld-dwarf.cpp | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/rtemstoolkit/rld-dwarf.cpp b/rtemstoolkit/rld-dwarf.cpp 
> index 1eae50c..2d6f306 100644
> --- a/rtemstoolkit/rld-dwarf.cpp
> +++ b/rtemstoolkit/rld-dwarf.cpp
> @@ -679,12 +679,18 @@ namespace rld
>  machine_code_ (false),
>  external_ (false),
>  declaration_ (false),
> +prototyped_ (false),
>  inline_ (DW_INL_not_inlined),
>  entry_pc_ (0),
>  has_entry_pc_ (false),
>  pc_low_ (0),
>  pc_high_ (0),
>  ranges_ (debug),
> +name_ (),
> +linkage_name_ (),

Did coverity ask for this to be added? Why do default constructors need to be 
added?

Chris

> +decl_file_ (),
> +decl_line_ (0),
> +call_file_ (),
>  call_line_ (0)
>  {
>dwarf_bool db;
> @@ -819,6 +825,7 @@ namespace rld
>  machine_code_ (orig.machine_code_),
>  external_ (orig.external_),
>  declaration_ (orig.declaration_),
> +prototyped_ (orig.prototyped_),
>  inline_ (orig.inline_),
>  entry_pc_ (orig.entry_pc_),
>  has_entry_pc_ (orig.has_entry_pc_), @@ -827,6 +834,8 @@ 
> namespace rld
>  ranges_ (orig.ranges_),
>  name_ (orig.name_),
>  linkage_name_ (orig.linkage_name_),
> +decl_file_ (orig.decl_file_),
> +decl_line_ (orig.decl_line_),
>  call_file_ (orig.call_file_),
>  call_line_ (orig.call_line_)
>  {
> @@ -986,7 +995,10 @@ namespace rld
>  ranges_ = rhs.ranges_;
>  name_ = rhs.name_;
>  linkage_name_ = rhs.linkage_name_;
> +decl_file_ = rhs.decl_file_;
> +decl_line_ = rhs.decl_line_;
>  call_file_ = rhs.call_file_;
> +call_line_ = rhs.call_line_;
>}
>return *this;
>  }
> 
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [PATCH] eng: Remove bogus coding rule

[Kinsey Moore]

This change looks good.

On 9/2/2021 04:21, Sebastian Huber wrote:

Declaring functions used only through function pointers as inline makes
absolutely no sense at all.
---
  eng/coding-conventions.rst | 2 --
  1 file changed, 2 deletions(-)

diff --git a/eng/coding-conventions.rst b/eng/coding-conventions.rst
index 668a917..575dd44 100644
--- a/eng/coding-conventions.rst
+++ b/eng/coding-conventions.rst
@@ -219,8 +219,6 @@ Performance
  * Understand the constraints of `real-time programming 
`_..
Limit execution times in interrupt contexts and critical sections,
such as Interrupt and Timer Service Routines (TSRs).
-* Functions used only through function pointers should be declared
-  'static inline' (RTEMS_INLINE_ROUTINE)
  * Prefer to ++preincrement instead of postincrement++.
  * Avoid using floating point except where absolutely necessary.
  

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

[PATCH v1 00/10] Convert and reformat rtems-tools pt. 1

[Ryan Long]

Hi,

For this and the following series of patches I followed this series of
steps.

1. Convert a file from the C way of doing things to C++
2. Go through all the files that had to do with the converted file and
make the formatting consistent.

Thanks,
Ryan

Ryan Long (10):
  CoverageMap.cc: Fix formatting
  CoverageReader: Convert to C++
  CoverageFactory.cc: Fix formatting
  CoverageMapBase.cc: Fix formatting
  CoverageFactory.h: Fix formatting
  CoverageMapBase.h: Fix formatting
  CoverageReaderQEMU.cc: Fix formatting
  CoverageReaderRTEMS.cc: Fix formatting
  CoverageReaderSkyeye.cc: Fix formatting
  CoverageReaderTSIM.cc: Fix formatting

 tester/covoar/CoverageFactory.cc  |  18 ++-
 tester/covoar/CoverageFactory.h   |  12 +-
 tester/covoar/CoverageMap.cc  |   2 +-
 tester/covoar/CoverageMapBase.cc  | 261 +-
 tester/covoar/CoverageMapBase.h   |  66 ++---
 tester/covoar/CoverageReaderBase.h|   2 +-
 tester/covoar/CoverageReaderQEMU.cc   |  57 +++-
 tester/covoar/CoverageReaderQEMU.h|   4 +-
 tester/covoar/CoverageReaderRTEMS.cc  |  44 +++---
 tester/covoar/CoverageReaderRTEMS.h   |   2 +-
 tester/covoar/CoverageReaderSkyeye.cc |  41 +++---
 tester/covoar/CoverageReaderSkyeye.h  |   2 +-
 tester/covoar/CoverageReaderTSIM.cc   |  31 ++--
 tester/covoar/CoverageReaderTSIM.h|   4 +-
 14 files changed, 308 insertions(+), 238 deletions(-)

-- 
1.8.3.1

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

[PATCH v1 03/10] CoverageFactory.cc: Fix formatting

[Ryan Long]

---
 tester/covoar/CoverageFactory.cc | 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/tester/covoar/CoverageFactory.cc b/tester/covoar/CoverageFactory.cc
index e6c3774..89f8376 100644
--- a/tester/covoar/CoverageFactory.cc
+++ b/tester/covoar/CoverageFactory.cc
@@ -25,17 +25,21 @@ Coverage::CoverageFormats_t Coverage::CoverageFormatToEnum(
   const std::string& format
 )
 {
-  if (format == "QEMU")
+  if ( format == "QEMU" ) {
 return COVERAGE_FORMAT_QEMU;
+  }
 
-  if (format == "RTEMS")
+  if ( format == "RTEMS" ) {
 return COVERAGE_FORMAT_RTEMS;
+  }
 
-  if (format == "Skyeye")
+  if (format == "Skyeye") {
 return COVERAGE_FORMAT_SKYEYE;
+  }
 
-  if (format == "TSIM")
+  if ( format == "TSIM" ) {
 return COVERAGE_FORMAT_TSIM;
+  }
 
   std::ostringstream what;
   what << format << " is an unknown coverage format "
@@ -47,7 +51,7 @@ Coverage::CoverageReaderBase* Coverage::CreateCoverageReader(
   CoverageFormats_t format
 )
 {
-  switch (format) {
+  switch ( format ) {
 case COVERAGE_FORMAT_QEMU:
   return new Coverage::CoverageReaderQEMU();
 case COVERAGE_FORMAT_RTEMS:
@@ -66,7 +70,7 @@ Coverage::CoverageWriterBase* Coverage::CreateCoverageWriter(
   CoverageFormats_t format
 )
 {
-  switch (format) {
+  switch ( format ) {
 case COVERAGE_FORMAT_RTEMS:
   return new Coverage::CoverageWriterRTEMS();
 case COVERAGE_FORMAT_SKYEYE:
-- 
1.8.3.1

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

[PATCH v1 01/10] CoverageMap.cc: Fix formatting

[Ryan Long]

---
 tester/covoar/CoverageMap.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tester/covoar/CoverageMap.cc b/tester/covoar/CoverageMap.cc
index b470fea..3e0b9ce 100644
--- a/tester/covoar/CoverageMap.cc
+++ b/tester/covoar/CoverageMap.cc
@@ -13,7 +13,7 @@ namespace Coverage {
 const std::string& exefileName,
 uint32_t   low,
 uint32_t   high
-  ) : CoverageMapBase(exefileName, low, high)
+  ) : CoverageMapBase( exefileName, low, high )
   {
   }
 
-- 
1.8.3.1

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

[PATCH v1 02/10] CoverageReader: Convert to C++

[Ryan Long]

---
 tester/covoar/CoverageFactory.cc  | 10 +-
 tester/covoar/CoverageFactory.h   |  2 +-
 tester/covoar/CoverageReaderBase.h|  2 +-
 tester/covoar/CoverageReaderQEMU.cc   | 35 ++-
 tester/covoar/CoverageReaderQEMU.h|  4 +++-
 tester/covoar/CoverageReaderRTEMS.cc  | 20 +---
 tester/covoar/CoverageReaderRTEMS.h   |  2 +-
 tester/covoar/CoverageReaderSkyeye.cc | 20 +---
 tester/covoar/CoverageReaderSkyeye.h  |  2 +-
 tester/covoar/CoverageReaderTSIM.cc   | 21 +++--
 tester/covoar/CoverageReaderTSIM.h|  4 +++-
 11 files changed, 54 insertions(+), 68 deletions(-)

diff --git a/tester/covoar/CoverageFactory.cc b/tester/covoar/CoverageFactory.cc
index 991ba8f..e6c3774 100644
--- a/tester/covoar/CoverageFactory.cc
+++ b/tester/covoar/CoverageFactory.cc
@@ -22,19 +22,19 @@
 #include "CoverageWriterTSIM.h"
 
 Coverage::CoverageFormats_t Coverage::CoverageFormatToEnum(
-  const char* const format
+  const std::string& format
 )
 {
-  if (!strcmp( format, "QEMU" ))
+  if (format == "QEMU")
 return COVERAGE_FORMAT_QEMU;
 
-  if (!strcmp( format, "RTEMS" ))
+  if (format == "RTEMS")
 return COVERAGE_FORMAT_RTEMS;
 
-  if (!strcmp( format, "Skyeye" ))
+  if (format == "Skyeye")
 return COVERAGE_FORMAT_SKYEYE;
 
-  if (!strcmp( format, "TSIM" ))
+  if (format == "TSIM")
 return COVERAGE_FORMAT_TSIM;
 
   std::ostringstream what;
diff --git a/tester/covoar/CoverageFactory.h b/tester/covoar/CoverageFactory.h
index 4770d78..f71860d 100644
--- a/tester/covoar/CoverageFactory.h
+++ b/tester/covoar/CoverageFactory.h
@@ -33,7 +33,7 @@ namespace Coverage {
*  @return Returns a coverage file format.
*/
   CoverageFormats_t CoverageFormatToEnum(
-const char* const format
+const std::string& format
   );
 
   /*!
diff --git a/tester/covoar/CoverageReaderBase.h 
b/tester/covoar/CoverageReaderBase.h
index 332fc4d..7064b8d 100644
--- a/tester/covoar/CoverageReaderBase.h
+++ b/tester/covoar/CoverageReaderBase.h
@@ -39,7 +39,7 @@ namespace Coverage {
  * associated executable
  */
 virtual void processFile(
-  const char* const file,
+  const std::string&file,
   ExecutableInfo* const executableInformation
 ) = 0;
 
diff --git a/tester/covoar/CoverageReaderQEMU.cc 
b/tester/covoar/CoverageReaderQEMU.cc
index a44097d..cef09b3 100644
--- a/tester/covoar/CoverageReaderQEMU.cc
+++ b/tester/covoar/CoverageReaderQEMU.cc
@@ -20,12 +20,6 @@
 
 #include "qemu-traces.h"
 
-#if HAVE_OPEN64
-#define OPEN fopen64
-#else
-#define OPEN fopen
-#endif
-
 namespace Coverage {
 
   CoverageReaderQEMU::CoverageReaderQEMU()
@@ -38,14 +32,13 @@ namespace Coverage {
   }
 
   void CoverageReaderQEMU::processFile(
-const char* const file,
+const std::string&file,
 ExecutableInfo* const executableInformation
   )
   {
 struct trace_header header;
 uintptr_t   i;
-int status;
-FILE*   traceFile;
+std::ifstream   traceFile;
 uint8_t taken;
 uint8_t notTaken;
 uint8_t branchInfo;
@@ -57,16 +50,15 @@ namespace Coverage {
 //
 // Open the coverage file and read the header.
 //
-traceFile = ::OPEN( file, "r" );
-if (!traceFile) {
+traceFile.open( file );
+if ( !traceFile.is_open() ) {
   std::ostringstream what;
   what << "Unable to open " << file;
   throw rld::error( what, "CoverageReaderQEMU::processFile" );
 }
 
-status = ::fread( &header, sizeof(trace_header), 1, traceFile );
-if (status != 1) {
-  ::fclose( traceFile );
+traceFile.read( (char *) &header, sizeof( trace_header ) );
+if ( traceFile.fail() || traceFile.gcount() != sizeof( trace_header ) ) {
   std::ostringstream what;
   what << "Unable to read header from " << file;
   throw rld::error( what, "CoverageReaderQEMU::processFile" );
@@ -80,22 +72,16 @@ namespace Coverage {
   CoverageMapBase *aCoverageMap = NULL;
   struct trace_entry  entries[ENTRIES];
   struct trace_entry  *entry;
-  int num_entries;
-
 
   // Read and process each line of the coverage file.
-  num_entries = ::fread(
-entries,
-sizeof(struct trace_entry),
-ENTRIES,
-traceFile
-  );
-  if (num_entries == 0)
+  traceFile.read( (char *) entries, sizeof( struct trace_entry ) );
+  if ( traceFile.gcount() == 0 ) {
 break;
+  }
 
   // Get the coverage map for each entry.  Note that the map is
   // the same for each entry in the coverage map
-  for (int count=0; count
+
 #include "CoverageReaderBase.h"
 #include "ExecutableInfo.h"
 
@@ -37,7 +39,7 @@ TBD
 
 /* Inherit documentation from base class. */
 void processFile(
-  const char* const file,
+  const std::string&file,
   ExecutableInfo* const executableIn

[PATCH v1 04/10] CoverageMapBase.cc: Fix formatting

[Ryan Long]

---
 tester/covoar/CoverageMapBase.cc | 261 ---
 1 file changed, 159 insertions(+), 102 deletions(-)

diff --git a/tester/covoar/CoverageMapBase.cc b/tester/covoar/CoverageMapBase.cc
index 334426b..7073260 100644
--- a/tester/covoar/CoverageMapBase.cc
+++ b/tester/covoar/CoverageMapBase.cc
@@ -18,98 +18,109 @@
 
 namespace Coverage {
 
-  AddressInfo::AddressInfo ()
-: isStartOfInstruction (false),
-  wasExecuted (false),
-  isBranch (false),
-  isNop (false),
-  wasTaken (false),
-  wasNotTaken (false)
+  AddressInfo::AddressInfo()
+: isStartOfInstruction( false ),
+  wasExecuted( false ),
+  isBranch( false ),
+  isNop( false ),
+  wasTaken( false ),
+  wasNotTaken( false )
   {
   }
 
-  AddressRange::AddressRange ()
-: lowAddress(0),
-  highAddress(0)
+  AddressRange::AddressRange()
+: lowAddress( 0 ),
+  highAddress( 0 )
   {
   }
 
-  AddressRange::AddressRange (const std::string& name,
-  uint32_t   lowAddress,
-  uint32_t   highAddress)
-: fileName (name),
-  lowAddress (lowAddress),
-  highAddress (highAddress)
+  AddressRange::AddressRange(
+const std::string& name,
+uint32_t   lowAddress,
+uint32_t   highAddress)
+: fileName( name ),
+  lowAddress( lowAddress ),
+  highAddress( highAddress )
   {
-info.resize( size( ) );
+info.resize( size() );
   }
 
-  size_t AddressRange::size () const
+  size_t AddressRange::size() const
   {
 return highAddress - lowAddress + 1;
   }
 
-  bool AddressRange::inside (uint32_t address) const
+  bool AddressRange::inside( uint32_t address ) const
   {
 return address >= lowAddress && address <= highAddress;
   }
 
-  AddressInfo& AddressRange::get (uint32_t address)
+  AddressInfo& AddressRange::get( uint32_t address )
   {
-if ( !inside( address ) )
+if ( !inside( address ) ) {
   throw rld::error( "address outside range", "AddressRange::get" );
+}
+
 size_t slot = address - lowAddress;
-if (slot >= info.size ())
+
+if ( slot >= info.size () ) {
   throw rld::error( "address slot not found", "AddressRange::get" );
+}
+
 return info[slot];
   }
 
-  const AddressInfo& AddressRange::get (uint32_t address) const
+  const AddressInfo& AddressRange::get( uint32_t address ) const
   {
-if ( !inside( address ) )
+if ( !inside( address ) ) {
   throw rld::error( "address outside range", "AddressRange::get" );
+}
+
 size_t slot = address - lowAddress;
-if (slot >= info.size ())
+
+if ( slot >= info.size() ) {
   throw rld::error( "address slot not found", "AddressRange::get" );
+}
+
 return info[slot];
   }
 
-  void AddressRange::dump (std::ostream& out, bool show_slots) const
+  void AddressRange::dump( std::ostream& out, bool show_slots ) const
   {
 rtems::utils::ostream_guard old_state( out );
 
-out << std::hex << std::setfill('0')
-<< "Address range: low = " << std::setw(8) << lowAddress
-<< " high = " << std::setw(8) << highAddress
+out << std::hex << std::setfill( '0' )
+<< "Address range: low = " << std::setw( 8 ) << lowAddress
+<< " high = " << std::setw( 8 ) << highAddress
 << std::endl;
+
 if (show_slots) {
   size_t slot = 0;
-  for (auto& i : info) {
-out << std::hex << std::setfill('0')
-<< "0x" << std::setw(8) << slot++ + lowAddress
+
+  for ( auto& i : info ) {
+out << std::hex << std::setfill( '0' )
+<< "0x" << std::setw( 8 ) << slot++ + lowAddress
 << "- isStartOfInstruction:"
-<< (char*) (i.isStartOfInstruction ? "yes" : "no")
+<< (char*) ( i.isStartOfInstruction ? "yes" : "no" )
 << " wasExecuted:"
-<< (char*) (i.wasExecuted ? "yes" : "no")
-<< std::endl
-<< "   isBranch:"
-<< (char*) (i.isBranch ? "yes" : "no")
+<< (char*) ( i.wasExecuted ? "yes" : "no" )
+<< "\n   isBranch:"
+<< (char*) ( i.isBranch ? "yes" : "no" )
 << " wasTaken:"
-<< (char*) (i.wasTaken ? "yes" : "no")
+<< (char*) ( i.wasTaken ? "yes" : "no" )
 << " wasNotTaken:"
-<< (char*) (i.wasNotTaken ? "yes" : "no")
-<< std::dec << std::setfill(' ')
+<< (char*) ( i.wasNotTaken ? "yes" : "no" )
+<< std::dec << std::setfill( ' ' )
 << std::endl;
   }
 }
-
   }
 
   CoverageMapBase::CoverageMapBase(
 const std::string& exefileName,
 uint32_t   low,
 uint32_t   high
-) : exefileName (exefileName)
+  ) : exefileName( exefileName )
   {
 Ranges.push_back( AddressRange( exefileName, low, high ) );
   }
@@ -125,30 +136,38 @@ namespace Coverage {
 
   bool CoverageMapBase::validAdd

[PATCH v1 05/10] CoverageFactory.h: Fix formatting

[Ryan Long]

---
 tester/covoar/CoverageFactory.h | 12 +++-
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/tester/covoar/CoverageFactory.h b/tester/covoar/CoverageFactory.h
index f71860d..52443e8 100644
--- a/tester/covoar/CoverageFactory.h
+++ b/tester/covoar/CoverageFactory.h
@@ -32,9 +32,7 @@ namespace Coverage {
*
*  @return Returns a coverage file format.
*/
-  CoverageFormats_t CoverageFormatToEnum(
-const std::string& format
-  );
+  CoverageFormats_t CoverageFormatToEnum( const std::string& format );
 
   /*!
*  This method returns an instance of a Coverage File Reader class
@@ -44,9 +42,7 @@ namespace Coverage {
*
*  @return Returns a Coverage File Reader class instance.
*/
-  CoverageReaderBase* CreateCoverageReader(
-CoverageFormats_t format
-  );
+  CoverageReaderBase* CreateCoverageReader( CoverageFormats_t format );
 
   /*!
*  This method returns an instance of a Coverage File Writer class
@@ -56,9 +52,7 @@ namespace Coverage {
*
*  @return Returns a Coverage File Writer class instance.
*/
-  CoverageWriterBase* CreateCoverageWriter(
-CoverageFormats_t format
-  );
+  CoverageWriterBase* CreateCoverageWriter( CoverageFormats_t format );
 }
 
 #endif
-- 
1.8.3.1

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

[PATCH v1 06/10] CoverageMapBase.h: Fix formatting

[Ryan Long]

---
 tester/covoar/CoverageMapBase.h | 66 -
 1 file changed, 45 insertions(+), 21 deletions(-)

diff --git a/tester/covoar/CoverageMapBase.h b/tester/covoar/CoverageMapBase.h
index a58c696..0da5f1c 100644
--- a/tester/covoar/CoverageMapBase.h
+++ b/tester/covoar/CoverageMapBase.h
@@ -27,23 +27,28 @@ namespace Coverage {
  *  an instruction.
  */
 bool isStartOfInstruction;
+
 /*!
  *  This member indicates how many times the address was executed.
  */
 uint32_t wasExecuted;
+
 /*!
  *  This member indicates that the address is a branch instruction.
  */
 bool isBranch;
+
 /*!
  *  This member indicates that the address is a NOP instruction.
  */
 bool isNop;
+
 /*!
  *  When isBranch is TRUE, this member indicates that the branch
  *  instruction at the address was taken.
  */
 uint32_t wasTaken;
+
 /*!
  *  When isBranch is TRUE, this member indicates that the branch
  *  instruction at the address was NOT taken.
@@ -52,7 +57,7 @@ namespace Coverage {
 
   };
 
-  typedef std::vector < AddressInfo > AddressInfos;
+  typedef std::vector AddressInfos;
 
   /*!
*  This structure identifies the low and high addresses
@@ -61,19 +66,22 @@ namespace Coverage {
*/
   struct AddressRange {
 
-AddressRange ();
-AddressRange (const std::string& name,
-  uint32_t   lowAddress,
-  uint32_t   highAddress);
+AddressRange();
+AddressRange(
+  const std::string& name,
+  uint32_t   lowAddress,
+  uint32_t   highAddress
+);
+
+size_t size() const;
 
-size_t size () const;
+bool inside( uint32_t address ) const;
 
-bool inside (uint32_t address) const;
+AddressInfo& get( uint32_t address );
 
-AddressInfo& get (uint32_t address);
-const AddressInfo& get (uint32_t address) const;
+const AddressInfo& get( uint32_t address ) const;
 
-void dump (std::ostream& out, bool show_slots = false) const;
+void dump( std::ostream& out, bool show_slots = false ) const;
 
 /*!
  *  This is the file from which this originated.
@@ -100,7 +108,7 @@ namespace Coverage {
   /*
*  This type identifies a list of ranges.
*/
-  typedef std::vector< AddressRange >  AddressRanges;
+  typedef std::vector AddressRanges;
 
   /*! @class CoverageMapBase
*
@@ -140,10 +148,14 @@ namespace Coverage {
 /*!
  *  This method prints the contents of the coverage map to stdout.
  */
-void dump( void ) const;
+void dump() const;
 
 /*!
- *  Address valid?
+*  This method checks whether an address is valid.
+*
+*  @param[in] address specifies the address to check
+*
+*  @return Returns true if @p address is valid and false if not.
  */
 bool validAddress( const uint32_t address ) const;
 
@@ -156,6 +168,14 @@ namespace Coverage {
  */
 int32_t getFirstLowAddress() const;
 
+/*!
+ *  This method gets the low address of the range at @p index.
+ *
+ * @param[in] index the index of the range to get the size of
+ *
+ *  @return Returns the low address of the range at @p index in the
+ *  RangeList.
+ */
 uint32_t getLowAddressOfRange( size_t index ) const;
 
 /*!
@@ -179,6 +199,13 @@ namespace Coverage {
  */
 uint32_t getSize() const;
 
+/*!
+ *  This method gets the size of the address range at @p index.
+ *
+ *  @param[in] index the index of the range to get the size of
+ *
+ *  @return Returns the size of the address range.
+ */
 uint32_t getSizeOfRange( size_t index ) const;
 
 /*!
@@ -201,14 +228,14 @@ namespace Coverage {
  *  This method returns the high address of the coverage map.
  *
  *  @return Returns the high address of the coverage map.
-uint32_t getHighAddress( void ) const;
+uint32_t getHighAddress() const;
  */
 
 /*!
  *  This method returns the low address of the coverage map.
  *
  *  @return Returns the low address of the coverage map.
-uint32_t getLowAddress( void ) const;
+uint32_t getLowAddress() const;
  */
 
 /*!
@@ -217,9 +244,7 @@ namespace Coverage {
  *
  *  @param[in] address specifies the address of the start of an instruction
  */
-void setIsStartOfInstruction(
-  uint32_t address
-);
+void setIsStartOfInstruction( uint32_t address );
 
 /*!
  *  This method returns a boolean which indicates if this
@@ -368,7 +393,6 @@ namespace Coverage {
  */
 uint32_t getWasNotTaken( uint32_t address ) const;
 
-
 /*!
  *  This method returns a boolean which indicates if the branch
  *  instruction at the specified address is ALWAYS taken.
@@ -429,12 +453,12 @@ namespace Coverage {
 /*!
  * Range checked access to the info.
  */
-AddressInfo& getInfo(uint32_t offset);
+AddressInfo& g

[PATCH v1 07/10] CoverageReaderQEMU.cc: Fix formatting

[Ryan Long]

---
 tester/covoar/CoverageReaderQEMU.cc | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/tester/covoar/CoverageReaderQEMU.cc 
b/tester/covoar/CoverageReaderQEMU.cc
index cef09b3..53d8041 100644
--- a/tester/covoar/CoverageReaderQEMU.cc
+++ b/tester/covoar/CoverageReaderQEMU.cc
@@ -68,10 +68,10 @@ namespace Coverage {
 // Read ENTRIES number of trace entries.
 //
 #define ENTRIES 1024
-while (true) {
-  CoverageMapBase *aCoverageMap = NULL;
+while ( true ) {
+  CoverageMapBase*aCoverageMap = NULL;
   struct trace_entry  entries[ENTRIES];
-  struct trace_entry  *entry;
+  struct trace_entry* entry;
 
   // Read and process each line of the coverage file.
   traceFile.read( (char *) entries, sizeof( struct trace_entry ) );
@@ -90,31 +90,31 @@ namespace Coverage {
 aCoverageMap = executableInformation->getCoverageMap( entry->pc );
 
 // Ensure that coverage map exists.
-if (!aCoverageMap)
+if ( !aCoverageMap )
   continue;
 
 // Set was executed for each TRACE_OP_BLOCK
-if (entry->op & TRACE_OP_BLOCK) {
- for (i=0; isize; i++) {
+if ( entry->op & TRACE_OP_BLOCK ) {
+ for ( i = 0; i < entry->size; i++ ) {
 aCoverageMap->setWasExecuted( entry->pc + i );
   }
 }
 
 // Determine if additional branch information is available.
-if ( (entry->op & branchInfo) != 0 ) {
+if ( ( entry->op & branchInfo ) != 0 ) {
   uint32_t  a = entry->pc + entry->size - 1;
-while (a > entry->pc && !aCoverageMap->isStartOfInstruction(a))
+while ( a > entry->pc && !aCoverageMap->isStartOfInstruction( a ) )
   a--;
-if (a == entry->pc && !aCoverageMap->isStartOfInstruction(a)) {
+if ( a == entry->pc && !aCoverageMap->isStartOfInstruction( a ) ) {
   // Something went wrong parsing the objdump.
   std::ostringstream what;
   what << "Reached beginning of range in " << file
 << " at " << entry->pc << " with no start of instruction.";
   throw rld::error( what, "CoverageReaderQEMU::processFile" );
 }
-if (entry->op & taken) {
+if ( entry->op & taken ) {
   aCoverageMap->setWasTaken( a );
-} else if (entry->op & notTaken) {
+} else if ( entry->op & notTaken ) {
   aCoverageMap->setWasNotTaken( a );
 }
 }
-- 
1.8.3.1

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

[PATCH v1 08/10] CoverageReaderRTEMS.cc: Fix formatting

[Ryan Long]

---
 tester/covoar/CoverageReaderRTEMS.cc | 26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/tester/covoar/CoverageReaderRTEMS.cc 
b/tester/covoar/CoverageReaderRTEMS.cc
index e869fc2..6331a81 100644
--- a/tester/covoar/CoverageReaderRTEMS.cc
+++ b/tester/covoar/CoverageReaderRTEMS.cc
@@ -35,13 +35,13 @@ namespace Coverage {
 ExecutableInfo* const executableInformation
   )
   {
-CoverageMapBase* aCoverageMap = NULL;
-uintptr_tbaseAddress;
-uint8_t  cover;
-std::ifstreamcoverageFile;
-rtems_coverage_map_header_t  header;
-uintptr_ti;
-uintptr_tlength;
+CoverageMapBase*aCoverageMap = NULL;
+uintptr_t   baseAddress;
+uint8_t cover;
+std::ifstream   coverageFile;
+rtems_coverage_map_header_t header;
+uintptr_t   i;
+uintptr_t   length;
 
 //
 // Open the coverage file and read the header.
@@ -66,13 +66,13 @@ namespace Coverage {
 //
 // Read and process each line of the coverage file.
 //
-for (i = 0; i < length; i++) {
+for ( i = 0; i < length; i++ ) {
   coverageFile.read( (char *) &cover, sizeof( uint8_t ) );
   if ( coverageFile.fail() ) {
 std::cerr << "breaking after 0x"
-  << std::hex << std::setfill('0')
-  << std::setw(8) << i
-  << std::setfill(' ') << std::dec
+  << std::hex << std::setfill( '0' )
+  << std::setw( 8 ) << i
+  << std::setfill( ' ' ) << std::dec
   << " in " << file
   << std::endl;
 break;
@@ -82,9 +82,9 @@ namespace Coverage {
   // Obtain the coverage map containing the address and
   // mark the address as executed.
   //
-  if (cover) {
+  if ( cover ) {
 aCoverageMap = executableInformation->getCoverageMap( baseAddress + i 
);
-if (aCoverageMap)
+if ( aCoverageMap )
   aCoverageMap->setWasExecuted( baseAddress + i );
   }
 }
-- 
1.8.3.1

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

[PATCH v1 09/10] CoverageReaderSkyeye.cc: Fix formatting

[Ryan Long]

---
 tester/covoar/CoverageReaderSkyeye.cc | 21 -
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/tester/covoar/CoverageReaderSkyeye.cc 
b/tester/covoar/CoverageReaderSkyeye.cc
index de0d4ad..abb3878 100644
--- a/tester/covoar/CoverageReaderSkyeye.cc
+++ b/tester/covoar/CoverageReaderSkyeye.cc
@@ -64,13 +64,13 @@ namespace Coverage {
 //
 // Read and process each line of the coverage file.
 //
-for (i = 0; i < length; i += 8) {
+for ( i = 0; i < length; i += 8 ) {
   coverageFile.read( (char *) &cover, sizeof( uint8_t ) );
   if ( coverageFile.gcount() != sizeof( uint8_t ) ) {
 std::cerr << "CoverageReaderSkyeye::ProcessFile - breaking after 0x"
-  << std::hex << std::setfill('0')
-  << std::setw(8) << i
-  << std::setfill(' ') << std::dec
+  << std::hex << std::setfill( '0' )
+  << std::setw( 8 ) << i
+  << std::setfill( ' ' ) << std::dec
   << " in " << file
   << std::endl;
 break;
@@ -82,9 +82,12 @@ namespace Coverage {
   //
   // NOTE: This method ONLY works for Skyeye in 32-bit mode.
   //
-  if (cover & 0x01) {
-aCoverageMap = executableInformation->getCoverageMap( baseAddress + i 
);
-if (aCoverageMap) {
+  if ( cover & 0x01 ) {
+aCoverageMap = executableInformation->getCoverageMap(
+  baseAddress + i
+);
+
+if ( aCoverageMap ) {
   aCoverageMap->setWasExecuted( baseAddress + i );
   aCoverageMap->setWasExecuted( baseAddress + i + 1 );
   aCoverageMap->setWasExecuted( baseAddress + i + 2 );
@@ -92,11 +95,11 @@ namespace Coverage {
 }
   }
 
-  if (cover & 0x10) {
+  if ( cover & 0x10 ) {
 aCoverageMap = executableInformation->getCoverageMap(
   baseAddress + i + 4
 );
-if (aCoverageMap) {
+if ( aCoverageMap ) {
   aCoverageMap->setWasExecuted( baseAddress + i + 4 );
   aCoverageMap->setWasExecuted( baseAddress + i + 5 );
   aCoverageMap->setWasExecuted( baseAddress + i + 6 );
-- 
1.8.3.1

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

[PATCH v1 10/10] CoverageReaderTSIM.cc: Fix formatting

[Ryan Long]

---
 tester/covoar/CoverageReaderTSIM.cc | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tester/covoar/CoverageReaderTSIM.cc 
b/tester/covoar/CoverageReaderTSIM.cc
index 9c6ff7d..11f0be6 100644
--- a/tester/covoar/CoverageReaderTSIM.cc
+++ b/tester/covoar/CoverageReaderTSIM.cc
@@ -61,7 +61,7 @@ namespace Coverage {
 break;
   }
 
-  for (i = 0; i < 0x80; i += 4) {
+  for ( i = 0; i < 0x80; i += 4 ) {
 unsigned int a;
 
 coverageFile >> std::hex >> cover >> std::dec;
@@ -69,9 +69,9 @@ namespace Coverage {
   std::cerr << "CoverageReaderTSIM: WARNING! Short line in "
 << file
 << " at address 0x"
-<< std::hex << std::setfill('0')
+<< std::hex << std::setfill( '0' )
 << baseAddress
-<< std::setfill(' ') << std::dec
+<< std::setfill( ' ' ) << std::dec
 << std::endl;
   break;
 }
@@ -82,8 +82,10 @@ namespace Coverage {
 //
 a = baseAddress + i;
 aCoverageMap = executableInformation->getCoverageMap( a );
-if ( !aCoverageMap )
+if ( !aCoverageMap ) {
   continue;
+}
+
 if ( cover & 0x01 ) {
   aCoverageMap->setWasExecuted( a );
   aCoverageMap->setWasExecuted( a + 1 );
-- 
1.8.3.1

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

[PATCH RSB] Update newlib to 9069cb9 hash

[Joel Sherrill]

This update is to include a fix for building rtems-libbsd using the
previous newlib hash.  was using a macro specific to
the newlib implementation of . When building rtems-libbsd,
-ffreestanding is used and that switches to the gcc  and
there was a compilation error.
---
 rtems/config/tools/rtems-gcc-10-newlib-head.cfg   | 4 ++--
 rtems/config/tools/rtems-gcc-head-newlib-head.cfg | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/rtems/config/tools/rtems-gcc-10-newlib-head.cfg 
b/rtems/config/tools/rtems-gcc-10-newlib-head.cfg
index 69c4fb5..399710c 100644
--- a/rtems/config/tools/rtems-gcc-10-newlib-head.cfg
+++ b/rtems/config/tools/rtems-gcc-10-newlib-head.cfg
@@ -13,11 +13,11 @@
 %patch add gcc -p1 
https://devel.rtems.org/raw-attachment/ticket/4215/0001-nios2-Remove-custom-instruction-warnings.patch
 %hash sha512 0001-nios2-Remove-custom-instruction-warnings.patch 
afd8a5e6bdcc5b75d5fbbf558bdf56ccac400521a6eec9d88cc95f6be67c481f2dbf8faa0f6ddc1e4ac7c56a84938714d80e46e9cf80ec4b8fcd739986449881
 
-%define newlib_version 4f81149
+%define newlib_version 9069cb9
 %define newlib_external 1
 %define newlib_expand_name sourceware-mirror-newlib-cygwin-%{newlib_version}
 %source set newlib --rsb-file=newlib-%{newlib_version}.tar.gz 
https://codeload.github.com/RTEMS/sourceware-mirror-newlib-cygwin/tar.gz/%{newlib_version}
-%hash sha512 newlib-%{newlib_version}.tar.gz 
D3solpDJpJR0P8MciqOTI9BXKLzd7YQmZbzIFXClXN37Q+PKKkkVlc3q9v4lfZbVGXEMEIiu4zTLPf4bKUfG2w==
+%hash sha512 newlib-%{newlib_version}.tar.gz 
r/FkML8n7cTUGaWik8RqdCXuoxfT9N26aWXw404TkQOFiHnCXBT8P1kxGhCEptOpNqaIRsgamvWx5RPowpXqqg==
 
 %define with_threads 1
 %define with_plugin 0
diff --git a/rtems/config/tools/rtems-gcc-head-newlib-head.cfg 
b/rtems/config/tools/rtems-gcc-head-newlib-head.cfg
index 4939ca5..111d6c5 100644
--- a/rtems/config/tools/rtems-gcc-head-newlib-head.cfg
+++ b/rtems/config/tools/rtems-gcc-head-newlib-head.cfg
@@ -7,11 +7,11 @@
 %source set gcc --rsb-file=%{gcc_expand_name}.tar.gz 
https://codeload.github.com/RTEMS/gnu-mirror-gcc/tar.gz/%{gcc_version}
 %hash sha512 %{gcc_expand_name}.tar.gz 
565db44c7427c3157f7a8c5541e1e4f849412c9967c8385738dc0fea366998be7a8f2b0b1f31d2197745b541c9d9f85cc4dfeb68632dab96c7ae7efbb500ae67
 
-%define newlib_version 0c0f3df
+%define newlib_version 9069cb9
 %define newlib_external 1
 %define newlib_expand_name sourceware-mirror-newlib-cygwin-%{newlib_version}
 %source set newlib --rsb-file=newlib-%{newlib_version}.tar.gz 
https://codeload.github.com/RTEMS/sourceware-mirror-newlib-cygwin/tar.gz/%{newlib_version}
-%hash sha512 newlib-%{newlib_version}.tar.gz 
9ded46b3077508ef05bbb4bf424777a0baa5aab9c7c0c902fb5529bb73b5b5034c35282e2dbf270cbcd44d84940a20ee270e329db4e4b501046978c18f78a11c
+%hash sha512 newlib-%{newlib_version}.tar.gz 
r/FkML8n7cTUGaWik8RqdCXuoxfT9N26aWXw404TkQOFiHnCXBT8P1kxGhCEptOpNqaIRsgamvWx5RPowpXqqg==
 
 %define with_threads 1
 %define with_plugin 0
-- 
2.24.4

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Ticket #4203 clarification

[zack leung]

Hello!

I'd like to begin working on 4203. It involves improving the exception
output  I'd like to know how I spot exceptions that could be reported
better by the system?   How do I also know how do I test it and what BSPs i
can test with only my computer Also joel told me The list of exception
number/names is in the manual for the architecture. For an i386 or later,
that should be in many places. Where do i find the documentation of the
bsps?

Thanks

Zack
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: Ticket #4203 clarification

[Joel Sherrill]

On Thu, Sep 2, 2021 at 8:01 PM zack leung  wrote:
>
> Hello!
>
> I'd like to begin working on 4203. It involves improving the exception output 
>  I'd like to know how I spot exceptions that could be reported better by the 
> system?   How do I also know how do I test it and what BSPs i can test with 
> only my computer Also joel told me The list of exception number/names is in 
> the manual for the architecture. For an i386 or later, that should be in many 
> places. Where do i find the documentation of the bsps?
>

Bringing this over from chat.

The methods are architecture specific and may be in score/cpu or
bsps/shared or a specific BSP (hope now there). For an example,
score/cpu/i386/cpu.c has an exception printer. It seems to be the one
cited as an example that could be improved. The thread id is printed
in decimal, not hex. The exception type is only printed as a number.
This could be turned into a number and exception name. The list of
exception number/names is in the manual for the architecture. For an
i386 or later, that should be in many places. There is only so much
information in the exception frame passed in, so just make it more
useful. If you look on other architectures for similar exception
printers, you may find some print RTEMS state. Perhaps that's common
info that could be printed by a shared subroutine.

For a manual, this is the grandparent for i386 info:

https://css.csail.mit.edu/6.858/2014/readings/i386.pdf

And yes, that is from 1986 and I think I have a paper copy that old.
But all x86 info should be essentially the same as what's presented in
there. There may be additions but that's the original root.

I wouldn't look actively at other architecture exception handlers. You
could easily get lost in the details of each architecture and
confused. Focus on the i386 first and how it could be better. For
example, taking the exception number of printing a string name so you
don't have to look it up. You find the list in the manual.

FWIW I think the PowerPC prints numbers also.

The name of the handler may be architecture specific. Hopefully
someone can point to some for other architectures. But a good solution
on x86 would be a good start.

--joel

--joel
> Thanks
>
> Zack
> ___
> devel mailing list
> devel@rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [PATCH v1 3/3] rld-dwarf.cpp: Initialize member variables

[Chris Johns]

On 3/9/21 12:08 am, Ryan Long wrote:
> I remember prototyped_ being one that the issue was complaining about, but I 
> can't remember the others. I just went ahead and added the rest of the member 
> variables because Coverity doesn't always identify all the subsequent issues.

May I suggest you only fix the specific issues coverity raises. I have removed
those extra lines as I need to fix another issues when building on FreeBSD and
MacOS.

Chris
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [PATCH v1 00/10] Convert and reformat rtems-tools pt. 1

[Chris Johns]

On 3/9/21 5:24 am, Ryan Long wrote:
> For this and the following series of patches I followed this series of
> steps.
> 
> 1. Convert a file from the C way of doing things to C++
> 2. Go through all the files that had to do with the converted file and
> make the formatting consistent.

Looks good and thanks for taking this on.

Have you built this with clang on FreeBSD? I only ask because the minor fix I
pushed where clang complained about the `std::ostream::tellp()` return value
being compared to `long int` 

https://lists.rtems.org/pipermail/build/2021-September/028853.html

I suspect if this change build on FreeBSD it will also build on MacOS. Do you
have access to a MacOS machine?

There is a new piece of code around `gcount()` and this is the reason for my
question.

Chris



___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel