Re: [PATCH 4/5] flashdisk.c: Fix Resource leak (CID #1439298)

[Joel Sherrill]

On Mon, Mar 15, 2021, 8:00 PM Chris Johns  wrote:

> On 16/3/21 11:49 am, Joel Sherrill wrote:
> > On Mon, Mar 15, 2021, 6:10 PM Chris Johns  > > wrote:
> >
> > On 15/3/21 2:21 pm, Joel Sherrill wrote:
> > > On Sun, Mar 14, 2021, 9:27 PM Chris Johns  > 
> > > >> wrote:
> > > On 13/3/21 2:18 am, Ryan Long wrote:
> > > > CID 1439298: Resource leak in rtems_fdisk_initialize().
> > > >
> > > > Closes #4299
> > > > ---
> > > >  cpukit/libblock/src/flashdisk.c | 42
> > > ++---
> > > >  1 file changed, 31 insertions(+), 11 deletions(-)
> > > >
> > > > diff --git a/cpukit/libblock/src/flashdisk.c
> > b/cpukit/libblock/src/flashdisk.c
> > > > index 91f99e0..c4bac82 100644
> > > > --- a/cpukit/libblock/src/flashdisk.c
> > > > +++ b/cpukit/libblock/src/flashdisk.c
> > > > @@ -2463,6 +2463,7 @@ rtems_fdisk_initialize
> > (rtems_device_major_number major,
> > > >{
> > > >  char name[] = RTEMS_FLASHDISK_DEVICE_BASE_NAME "a";
> > > >  uint32_t device;
> > > > +uint32_t device_to_free;
> > > >  uint32_t blocks = 0;
> > > >  int  ret;
> > > >
> > > > @@ -2485,18 +2486,27 @@ rtems_fdisk_initialize
> > (rtems_device_major_number
> > > major,
> > > >   * One copy buffer of a page size.
> > > >   */
> > > >  fd->copy_buffer = malloc (c->block_size);
> > > > -if (!fd->copy_buffer)
> > > > +if (!fd->copy_buffer) {
> > > > +  free(fd);
> > > >return RTEMS_NO_MEMORY;
> > > > +}
> > > >
> > > >  fd->blocks = calloc (blocks, sizeof
> (rtems_fdisk_block_ctl));
> > > > -if (!fd->blocks)
> > > > +if (!fd->blocks) {
> > > > +  free(fd->copy_buffer);
> > > > +  free(fd);
> > > >return RTEMS_NO_MEMORY;
> > > > +}
> > > >
> > > >  fd->block_count = blocks;
> > > >
> > > >  fd->devices = calloc (c->device_count, sizeof
> > (rtems_fdisk_device_ctl));
> > > > -if (!fd->devices)
> > > > +if (!fd->devices) {
> > > > +  free (fd->blocks);
> > > > +  free (fd->copy_buffer);
> > > > +  free (fd);
> > > >return RTEMS_NO_MEMORY;
> > > > +}
> > > >
> > > >  rtems_mutex_init (>lock, "Flash Disk");
> > > >
> > > > @@ -2505,9 +2515,10 @@ rtems_fdisk_initialize
> (rtems_device_major_number
> > > major,
> > > >  if (sc != RTEMS_SUCCESSFUL)
> > > >  {
> > > >rtems_mutex_destroy (>lock);
> > > > -  free (fd->copy_buffer);
> > > > -  free (fd->blocks);
> > > >free (fd->devices);
> > > > +  free (fd->blocks);
> > > > +  free (fd->copy_buffer);
> > >
> > > Why the order change?
> > >
> > > Does the change make it exactly the opposite order of creation or
> do you
> > see it
> > > not being in inverse order?
> >
> > If there is no reason to change the order the blocks are freed then
> I suggest
> > not changing the order. It avoids adding noise to the change.
> >
> > > This was a hard one. It was missing a LOT of cleanup.
> > >
> > >
> > > > +  free (fd);
> > >
> > > What happens to the created blkdev the fd is passed into? Does
> that
> > need to be
> > > destroyed before this is released?
> >
> > I do not know. I have not looked.
> >
> > > I didn't recognise that as an allocation. What's the destroy call
> for that?
> >
> > If the block dev holds the pointer and you have freed it bad things
> will happen.
> >
> > I have a funny feeling there was no block dev destroy when the code
> was written
> > and why there are no free's for this allocation.
> >
> >
> > Is this one we should leave a version of the patch on the ticket, link
> to this
> > threadz and walk away from?
>
> The patch is wrong so I do not think it is a good idea holding it on the
> ticket.
>
> Walking away ... I have walked around this one for a while now so that is
> what
> we have been doing but it does not resolve the coverity side of things.
>
> > Or add block dev destroy?
>
> This is one option and would be a reasonable path but I am not sure how
> deep the
> pit it opens is.
>
> > What concerns me about
> > that is that it is another layer in the onion we (Ryan and I) don't fully
> > understand the interactions.
>
> Yeah, this is where it gets hard. What does it mean if the nvdisk create
> fails?
>
> > Is blocked destroy going to be just resources 

Re: code reformatting was Re: Regarding gsoc project 3860

[Ayushman Mishra]

I am extremely sorry for my behaviour and promise that this won't happen
again . Actually I am pretty new to open source community ( I have
contributed before but not interacted a lot with other people) and I am
really grateful to you for pointing out my mistake , I will definitely make
sure about this point while contributing and interacting with others in
future.

AYUSHMAN

On Tue, Mar 16, 2021, 3:35 AM Gedare Bloom  wrote:

> On Mon, Mar 15, 2021 at 6:44 AM Joel Sherrill  wrote:
> >
> >
> >
> > On Mon, Mar 15, 2021 at 4:20 AM Hesham Almatary <
> hesham.almat...@cl.cam.ac.uk> wrote:
> >>
> >> Hello Ayushman and Ida,
> >>
> >> Usually, if multiple students really want to work on a particular
> >> project (and can't/don't want to choose another), there can be
> >> multiple proposals for the same project and we choose the best one.
> >> Sometimes a project can be split up between two students to work on to
> >> minimise conflicts.
> >
> >
> > There are multiple things that need to be addressed here.
> >
> > First, there have been discussions on devel@ about code formatting
> tools.
> > Sebastian has posted a configuration for the indent program but offhand
> > I don't know where that is. It may be in the documentation.
> >
> I posted about this to Ida. I think it was uncrustify? I think several
> tools have been looked into. No specific tool is required, but we
> should pick the one that best allows us to meet the needs of the
> project.
>
> > For indent to move forward from here, its impact on the code in a
> directory
> > that is thought to follow the RTEMS style well would need to be
> evaluated.
> > Do the rules need to be tweaked to avoid changes? Is the source code
> actually
> > just not in conformance with published rules? The process here is to
> evaluate
> > the difference between tool output and existing code and work to close
> the
> > delta by tweaking rules and fixing code. The end is expected to be that
> there
> > are a few places where the tool can't produce RTEMS style and we have to
> > discuss adopting something the tool can produce.
> >
> > I don't recall if Sebastian evaluated the llvm formatter and created a
> configuration
> > for it. In this case, creating a configuration for this tool before
> evaluating the
> > difference in output would be the path forward. If this formatter is
> better, then
> > I would like to see an RTEMS style added to their options.
> >
> > With either tool, a factor is integrating it into the development
> process. I'm
> > not sure what a GSoC project would do about this.
> >
>
> I think the tool integration is the main piece of GSoC-relevant work,
> as this would involve some level of scripting and automation.
>
> > So there are two potential projects here. My question is not conflict on
> > project choice, it is whether either is an appropriate GSoC project. With
> > the shorter time frame, I think the scope of the project is in the right
> ballpark.
> > Is there enough coding? I don't know.
> >
>
> I'm not currently convinced there is enough coding work for two
> projects in this area. I don't think there would have been enough
> coding work for one project under the old GSoC scope.
>
> Running the code formatter and submitting patches won't really count
> as "code contributions"
>
> > --joel
> >
> >>
> >>
> >> On Mon, 15 Mar 2021 at 09:45, Ida Delphine  wrote:
> >> >
> >> > Umm...did you bring up a discussion regarding this project earlier?
> >> >
> I do not have a record of Ayushman "claiming" this project, and anyway
> we don't allow students to "claim" a project.
>
> >> > On Mon, 15 Mar 2021, 8:10 am Ayushman Mishra, <
> ayushvidush...@gmail.com> wrote:
> >> >>
> >> >> AYUSHMAN MISHRA
> >> >>
> >> >> Hello Ida delphini AYUSHMAN here , Can you please select any other
> project for gsoc as I am also currently working on proposal for the same
> project
> >> >> https://devel.rtems.org/ticket/3860 for gsoc 2021
> >> >
> Ayushman, this is not a polite request for you to make, in addition it
> would best have been made by direct reply to her email in the same
> thread, not by starting a new e-mail thread. In an open-source
> community, you should not impose yourself on another person. It goes
> against the fundamental ideas of "freedom" that open-source is based
> on. Part of GSoC is exactly about learning this kind of lesson, so
> don't feel too bad about it, but do pay attention to how you interact
> with others and make sure you are respecting their autonomy and
> perspectives.
>
> >> > ___
> >> > 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
> >
> > ___
> > devel mailing list
> > devel@rtems.org
> > http://lists.rtems.org/mailman/listinfo/devel
>

Re: [PATCH 4/5] flashdisk.c: Fix Resource leak (CID #1439298)

[Chris Johns]

On 16/3/21 11:49 am, Joel Sherrill wrote:
> On Mon, Mar 15, 2021, 6:10 PM Chris Johns  > wrote:
> 
> On 15/3/21 2:21 pm, Joel Sherrill wrote:
> > On Sun, Mar 14, 2021, 9:27 PM Chris Johns  
> > >> wrote:
> >     On 13/3/21 2:18 am, Ryan Long wrote:
> >     > CID 1439298: Resource leak in rtems_fdisk_initialize().
> >     >
> >     > Closes #4299
> >     > ---
> >     >  cpukit/libblock/src/flashdisk.c | 42
> >     ++---
> >     >  1 file changed, 31 insertions(+), 11 deletions(-)
> >     >
> >     > diff --git a/cpukit/libblock/src/flashdisk.c
> b/cpukit/libblock/src/flashdisk.c
> >     > index 91f99e0..c4bac82 100644
> >     > --- a/cpukit/libblock/src/flashdisk.c
> >     > +++ b/cpukit/libblock/src/flashdisk.c
> >     > @@ -2463,6 +2463,7 @@ rtems_fdisk_initialize
> (rtems_device_major_number major,
> >     >    {
> >     >      char     name[] = RTEMS_FLASHDISK_DEVICE_BASE_NAME "a";
> >     >      uint32_t device;
> >     > +    uint32_t device_to_free;
> >     >      uint32_t blocks = 0;
> >     >      int      ret;
> >     > 
> >     > @@ -2485,18 +2486,27 @@ rtems_fdisk_initialize
> (rtems_device_major_number
> >     major,
> >     >       * One copy buffer of a page size.
> >     >       */
> >     >      fd->copy_buffer = malloc (c->block_size);
> >     > -    if (!fd->copy_buffer)
> >     > +    if (!fd->copy_buffer) {
> >     > +      free(fd);
> >     >        return RTEMS_NO_MEMORY;
> >     > +    }
> >     > 
> >     >      fd->blocks = calloc (blocks, sizeof (rtems_fdisk_block_ctl));
> >     > -    if (!fd->blocks)
> >     > +    if (!fd->blocks) {
> >     > +      free(fd->copy_buffer);
> >     > +      free(fd);
> >     >        return RTEMS_NO_MEMORY;
> >     > +    }
> >     > 
> >     >      fd->block_count = blocks;
> >     > 
> >     >      fd->devices = calloc (c->device_count, sizeof
> (rtems_fdisk_device_ctl));
> >     > -    if (!fd->devices)
> >     > +    if (!fd->devices) {
> >     > +      free (fd->blocks);
> >     > +      free (fd->copy_buffer);
> >     > +      free (fd);
> >     >        return RTEMS_NO_MEMORY;
> >     > +    }
> >     > 
> >     >      rtems_mutex_init (>lock, "Flash Disk");
> >     > 
> >     > @@ -2505,9 +2515,10 @@ rtems_fdisk_initialize 
> (rtems_device_major_number
> >     major,
> >     >      if (sc != RTEMS_SUCCESSFUL)
> >     >      {
> >     >        rtems_mutex_destroy (>lock);
> >     > -      free (fd->copy_buffer);
> >     > -      free (fd->blocks);
> >     >        free (fd->devices);
> >     > +      free (fd->blocks);
> >     > +      free (fd->copy_buffer);
> >
> >     Why the order change?
> >
> > Does the change make it exactly the opposite order of creation or do you
> see it
> > not being in inverse order?
> 
> If there is no reason to change the order the blocks are freed then I 
> suggest
> not changing the order. It avoids adding noise to the change.
> 
> > This was a hard one. It was missing a LOT of cleanup.
> >
> >
> >     > +      free (fd);
> >
> >     What happens to the created blkdev the fd is passed into? Does that
> need to be
> >     destroyed before this is released?
> 
> I do not know. I have not looked.
> 
> > I didn't recognise that as an allocation. What's the destroy call for 
> that?
> 
> If the block dev holds the pointer and you have freed it bad things will 
> happen.
> 
> I have a funny feeling there was no block dev destroy when the code was 
> written
> and why there are no free's for this allocation.
> 
> 
> Is this one we should leave a version of the patch on the ticket, link to this
> threadz and walk away from? 

The patch is wrong so I do not think it is a good idea holding it on the ticket.

Walking away ... I have walked around this one for a while now so that is what
we have been doing but it does not resolve the coverity side of things.

> Or add block dev destroy? 

This is one option and would be a reasonable path but I am not sure how deep the
pit it opens is.

> What concerns me about
> that is that it is another layer in the onion we (Ryan and I) don't fully
> understand the interactions.

Yeah, this is where it gets hard. What does it mean if the nvdisk create fails?

> Is blocked destroy going to be just resources or will it have to interrupt
> outstanding work, etc?

I do not know.

Can this code be arrange so everything but the block dev create is done and if
that fails you can undo everything else?

Chris
___
devel mailing list
devel@rtems.org

Re: [PATCH 4/5] flashdisk.c: Fix Resource leak (CID #1439298)

[Joel Sherrill]

On Mon, Mar 15, 2021, 6:10 PM Chris Johns  wrote:

> On 15/3/21 2:21 pm, Joel Sherrill wrote:
> > On Sun, Mar 14, 2021, 9:27 PM Chris Johns  > > wrote:
> > On 13/3/21 2:18 am, Ryan Long wrote:
> > > CID 1439298: Resource leak in rtems_fdisk_initialize().
> > >
> > > Closes #4299
> > > ---
> > >  cpukit/libblock/src/flashdisk.c | 42
> > ++---
> > >  1 file changed, 31 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/cpukit/libblock/src/flashdisk.c
> b/cpukit/libblock/src/flashdisk.c
> > > index 91f99e0..c4bac82 100644
> > > --- a/cpukit/libblock/src/flashdisk.c
> > > +++ b/cpukit/libblock/src/flashdisk.c
> > > @@ -2463,6 +2463,7 @@ rtems_fdisk_initialize
> (rtems_device_major_number major,
> > >{
> > >  char name[] = RTEMS_FLASHDISK_DEVICE_BASE_NAME "a";
> > >  uint32_t device;
> > > +uint32_t device_to_free;
> > >  uint32_t blocks = 0;
> > >  int  ret;
> > >
> > > @@ -2485,18 +2486,27 @@ rtems_fdisk_initialize
> (rtems_device_major_number
> > major,
> > >   * One copy buffer of a page size.
> > >   */
> > >  fd->copy_buffer = malloc (c->block_size);
> > > -if (!fd->copy_buffer)
> > > +if (!fd->copy_buffer) {
> > > +  free(fd);
> > >return RTEMS_NO_MEMORY;
> > > +}
> > >
> > >  fd->blocks = calloc (blocks, sizeof (rtems_fdisk_block_ctl));
> > > -if (!fd->blocks)
> > > +if (!fd->blocks) {
> > > +  free(fd->copy_buffer);
> > > +  free(fd);
> > >return RTEMS_NO_MEMORY;
> > > +}
> > >
> > >  fd->block_count = blocks;
> > >
> > >  fd->devices = calloc (c->device_count, sizeof
> (rtems_fdisk_device_ctl));
> > > -if (!fd->devices)
> > > +if (!fd->devices) {
> > > +  free (fd->blocks);
> > > +  free (fd->copy_buffer);
> > > +  free (fd);
> > >return RTEMS_NO_MEMORY;
> > > +}
> > >
> > >  rtems_mutex_init (>lock, "Flash Disk");
> > >
> > > @@ -2505,9 +2515,10 @@ rtems_fdisk_initialize
> (rtems_device_major_number
> > major,
> > >  if (sc != RTEMS_SUCCESSFUL)
> > >  {
> > >rtems_mutex_destroy (>lock);
> > > -  free (fd->copy_buffer);
> > > -  free (fd->blocks);
> > >free (fd->devices);
> > > +  free (fd->blocks);
> > > +  free (fd->copy_buffer);
> >
> > Why the order change?
> >
> > Does the change make it exactly the opposite order of creation or do you
> see it
> > not being in inverse order?
>
> If there is no reason to change the order the blocks are freed then I
> suggest
> not changing the order. It avoids adding noise to the change.
>
> > This was a hard one. It was missing a LOT of cleanup.
> >
> >
> > > +  free (fd);
> >
> > What happens to the created blkdev the fd is passed into? Does that
> need to be
> > destroyed before this is released?
>
> I do not know. I have not looked.
>
> > I didn't recognise that as an allocation. What's the destroy call for
> that?
>
> If the block dev holds the pointer and you have freed it bad things will
> happen.
>
> I have a funny feeling there was no block dev destroy when the code was
> written
> and why there are no free's for this allocation.
>

Is this one we should leave a version of the patch on the ticket, link to
this threadz and walk away from? Or add block dev destroy? What concerns me
about that is that it is another layer in the onion we (Ryan and I) don't
fully understand the interactions.

Is blocked destroy going to be just resources or will it have to interrupt
outstanding work, etc?

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

Re: [PATCH] covoar: Handle periods in symbols from objdump

[Chris Johns]

On 16/3/21 8:26 am, Alex White wrote:
> The exceptions are being used as legitimate error cases. The "what" messages 
> that I use here are not helpful. I think that's why it looks like they're 
> being used as labels. 

Understood and thanks.

> As part of a revision of my "covoar: Fix NOP execution marking" patch that 
> I'll be sending out soon, this has been greatly simplified and a specific 
> exception class has been defined.
> 
> I'll get v2 of this patch out, and that should clear the exception handling 
> up.

Excellent.

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

Re: [PATCH v2] covoar: Handle periods in symbols from objdump

[Chris Johns]

On 16/3/21 9:04 am, Alex White wrote:
> Occasionally the compiler will generate symbols that look similar to
> symbols defined in RTEMS code except that they contain some suffix.
> This looks to be related to compiler optimizations. Such symbols were
> being treated as unique. For our purposes, they should be mapped to
> the equivalent symbols in the DWARF info. This has been fixed.

Is this specific to an architecture or common to all?
What does the suffix look like that is being stripped?
Are the symbols in the ELF symbols table or just in the DWARF data?

I am not objecting to the change but I think this commit messages would be a
better long term help if it carried some more detail. The questions I have
provided are an example of might be useful to add.

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

Re: [PATCH v2 0/4] coverage/reports: Improve coverage reports

[Chris Johns]

OK. Nice change and thank you.

Chris

On 16/3/21 9:09 am, Alex White wrote:
> v2:
> - Replace tab expansion function in ReportsBase with std::string version
> 
> This patch set includes a few improvements to the coverage reports.
> 
> Alex White (4):
>   covoar/reports: Add new statistics to summary
>   coverage/reports: Improve formatting and clarity
>   coverage/reports: Share common JS and CSS in reports
>   coverage: Give coverage bars red background
> 
>  tester/covoar/DesiredSymbols.cc   |  10 ++
>  tester/covoar/DesiredSymbols.h|  24 -
>  tester/covoar/ObjdumpProcessor.cc |   5 +
>  tester/covoar/ReportsBase.cc  |  67 ++--
>  tester/covoar/ReportsHtml.cc  | 163 +-
>  tester/covoar/ReportsText.cc  |  76 --
>  tester/rt/coverage.py |  66 
>  7 files changed, 275 insertions(+), 136 deletions(-)
> 
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [PATCH 1/2] covoar/CoverageReaderQEMU: Fix infinite loop

[Chris Johns]

On 16/3/21 10:00 am, Joel Sherrill wrote:
 On Mon, Mar 15, 2021 at 5:58 PM Chris Johns  > wrote:
> On 16/3/21 9:11 am, Gedare Bloom wrote:
> > On Mon, Mar 15, 2021 at 3:34 PM Alex White  
> > >> wrote:
> >
> >     I honestly can't remember why I changed 1024 to 20,000.
> >
> >     I've looked back at that code and changed it back to 1024 without 
> any
> >     issues. I think I might have missed that this is all happening in a 
> loop,
> >     and at one point during a (long) debugging session I convinced 
> myself that
> >     it wasn't reading all of the entries.
> >
> >     At least that's the most rational explanation I can think up for 
> that
> >     particular change. 
> >
> >     If I revert ENTRIES from 2 back to 1024, are we satisfied to 
> leave the
> >     "entries" array as-is?
> >
> > I think that Chris' main points here are that, as you get covoar 
> working again
> > and cleaning it up, it should be made more C++ (and less C).
> 
> Thanks Gedare, yes I am asking if this could be considered. A total 
> conversion
> is not realistic and would be asking too much but my hope is making 
> changes in
> small pieces can be done. Some changes will requiring new C++ skills but 
> that
> should be thought of as a fun challenge.
> 
> In this case I think changing to a vector would be a good thing for 1024 
> entries
> but we had 1024 in the code previously and it was fine so I am also OK if 
> it
> left that way.
> 
> This is a different case than many of the others, it is reading a block of 
> fixed
> size
> binary entries from the Qemu trace log. It is just avoiding reading the 
> 32-byte
> entries one at a time. It is read, process, and discard. How would
> std::ANYTHING help here?

Nothing or lots, it depends on the code. I did not check this specific case and
reacted to the rather large jump in size that did at one point in the testing
seem important. Once the data is in a container you have a range things std:*
gives you when you use containers, eg find_if with lambda functions. If I was
writing the code in C++ I would have used a container from the beginning and not
a fixed size array with the maintance overhead of sizeof or #defined limits,
this is the the point I am making.

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

Re: [PATCH 1/5] hexdump-parse.c: Fix Resource leak (CID #26032)

[Chris Johns]

On 16/3/21 10:07 am, Joel Sherrill wrote:
> On Mon, Mar 15, 2021 at 6:01 PM Chris Johns  > wrote:
> On 16/3/21 6:55 am, Joel Sherrill wrote:
> >
> >
> > On Mon, Mar 15, 2021 at 2:46 PM Gedare Bloom  
> > >> wrote:
> >
> >     On Sun, Mar 14, 2021 at 8:27 PM Chris Johns  
> >     >> wrote:
> >     >
> >     > On 13/3/21 2:18 am, Ryan Long wrote:
> >     > > CID 26032: Resource leak in rtems_shell_hexdump_rewrite().
> >     > >
> >     > > Closes #4296
> >     > > ---
> >     > >  cpukit/libmisc/shell/hexdump-parse.c | 3 +++
> >     > >  1 file changed, 3 insertions(+)
> >     > >
> >     > > diff --git a/cpukit/libmisc/shell/hexdump-parse.c
> >     b/cpukit/libmisc/shell/hexdump-parse.c
> >     > > index 88b9d56..5b56bbf 100644
> >     > > --- a/cpukit/libmisc/shell/hexdump-parse.c
> >     > > +++ b/cpukit/libmisc/shell/hexdump-parse.c
> >     > > @@ -462,6 +462,9 @@ isint2:                                 
> >      switch(fu->bcnt) {
> >     > >               (void)printf("\n");
> >     > >       }
> >     > >  #endif
> >     > > +#ifdef __rtems__
> >     > > +     free(nextpr);
> >     > > +#endif
> >     >
> >     > I know this has not been done in imported code in rtems.git 
> before but
> >     should we
> >     > adopt the libbsd standard of adding /* __rtems__ */ to the #else 
> and
> #endif?
> >
> >     Probably, but we also clone-and-own this shell/ code, and we should
> >     not bother with these #ifdefs in there. I think I have said this 3
> >     times this past week about shell/. The upstream does not want our
> >     changes, and we don't import from them anymore.
> >
> > Some of these files have massive changes and some don't. Ryan and 
> > I looked at main_cp.c and it had at least 40 revisions since the version
> > we have. The same Coverity issue appeared to be present but the variable
> > names were changed and much clearer.
> 
> Yes.
> 
> > Chris imported this code initially. I'll trust his ruling on this since 
> I
> assume
> > he is likely to either be the one to update it eventually or have to 
> help
> someone
> > a lot. :)
> 
> If the code was updated I would use the libbsd way of handing it. It has 
> been
> proven to work.
> 
> > And some of it is close enough that the ifdef's are worth it. Some 
> isn't. 
> > Hard call on the overall value.
> 
> For the existing code it is hard to call. If I was starting again I would 
> say we
> had to support a clean separation.
> 
> 
> I assume you would checkout the original version and diff it. But would the
> ifdef rtems ease the burden any?

Yes and a good question, I do not know. I suppose it would depend on the file
and changes.

> I tend to like going with them otherwise, we have some files we do it
> on and others we don't.

This makes sense so if we feel it would help even now lets add them and if we
add then I feel it is only small amount of extra effort to add them in the
libbsd style.

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

Re: [PATCH 4/5] flashdisk.c: Fix Resource leak (CID #1439298)

[Chris Johns]

On 15/3/21 2:21 pm, Joel Sherrill wrote:
> On Sun, Mar 14, 2021, 9:27 PM Chris Johns  > wrote:
> On 13/3/21 2:18 am, Ryan Long wrote:
> > CID 1439298: Resource leak in rtems_fdisk_initialize().
> >
> > Closes #4299
> > ---
> >  cpukit/libblock/src/flashdisk.c | 42
> ++---
> >  1 file changed, 31 insertions(+), 11 deletions(-)
> >
> > diff --git a/cpukit/libblock/src/flashdisk.c 
> b/cpukit/libblock/src/flashdisk.c
> > index 91f99e0..c4bac82 100644
> > --- a/cpukit/libblock/src/flashdisk.c
> > +++ b/cpukit/libblock/src/flashdisk.c
> > @@ -2463,6 +2463,7 @@ rtems_fdisk_initialize (rtems_device_major_number 
> major,
> >    {
> >      char     name[] = RTEMS_FLASHDISK_DEVICE_BASE_NAME "a";
> >      uint32_t device;
> > +    uint32_t device_to_free;
> >      uint32_t blocks = 0;
> >      int      ret;
> > 
> > @@ -2485,18 +2486,27 @@ rtems_fdisk_initialize 
> (rtems_device_major_number
> major,
> >       * One copy buffer of a page size.
> >       */
> >      fd->copy_buffer = malloc (c->block_size);
> > -    if (!fd->copy_buffer)
> > +    if (!fd->copy_buffer) {
> > +      free(fd);
> >        return RTEMS_NO_MEMORY;
> > +    }
> > 
> >      fd->blocks = calloc (blocks, sizeof (rtems_fdisk_block_ctl));
> > -    if (!fd->blocks)
> > +    if (!fd->blocks) {
> > +      free(fd->copy_buffer);
> > +      free(fd);
> >        return RTEMS_NO_MEMORY;
> > +    }
> > 
> >      fd->block_count = blocks;
> > 
> >      fd->devices = calloc (c->device_count, sizeof 
> (rtems_fdisk_device_ctl));
> > -    if (!fd->devices)
> > +    if (!fd->devices) {
> > +      free (fd->blocks);
> > +      free (fd->copy_buffer);
> > +      free (fd);
> >        return RTEMS_NO_MEMORY;
> > +    }
> > 
> >      rtems_mutex_init (>lock, "Flash Disk");
> > 
> > @@ -2505,9 +2515,10 @@ rtems_fdisk_initialize (rtems_device_major_number
> major,
> >      if (sc != RTEMS_SUCCESSFUL)
> >      {
> >        rtems_mutex_destroy (>lock);
> > -      free (fd->copy_buffer);
> > -      free (fd->blocks);
> >        free (fd->devices);
> > +      free (fd->blocks);
> > +      free (fd->copy_buffer);
> 
> Why the order change?
> 
> Does the change make it exactly the opposite order of creation or do you see 
> it
> not being in inverse order?

If there is no reason to change the order the blocks are freed then I suggest
not changing the order. It avoids adding noise to the change.

> This was a hard one. It was missing a LOT of cleanup.
> 
> 
> > +      free (fd);
> 
> What happens to the created blkdev the fd is passed into? Does that need 
> to be
> destroyed before this is released?

I do not know. I have not looked.

> I didn't recognise that as an allocation. What's the destroy call for that?

If the block dev holds the pointer and you have freed it bad things will happen.

I have a funny feeling there was no block dev destroy when the code was written
and why there are no free's for this allocation.

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

Re: [PATCH 1/5] hexdump-parse.c: Fix Resource leak (CID #26032)

[Joel Sherrill]

On Mon, Mar 15, 2021 at 6:01 PM Chris Johns  wrote:

>
>
> On 16/3/21 6:55 am, Joel Sherrill wrote:
> >
> >
> > On Mon, Mar 15, 2021 at 2:46 PM Gedare Bloom  > > wrote:
> >
> > On Sun, Mar 14, 2021 at 8:27 PM Chris Johns  > > wrote:
> > >
> > > On 13/3/21 2:18 am, Ryan Long wrote:
> > > > CID 26032: Resource leak in rtems_shell_hexdump_rewrite().
> > > >
> > > > Closes #4296
> > > > ---
> > > >  cpukit/libmisc/shell/hexdump-parse.c | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/cpukit/libmisc/shell/hexdump-parse.c
> > b/cpukit/libmisc/shell/hexdump-parse.c
> > > > index 88b9d56..5b56bbf 100644
> > > > --- a/cpukit/libmisc/shell/hexdump-parse.c
> > > > +++ b/cpukit/libmisc/shell/hexdump-parse.c
> > > > @@ -462,6 +462,9 @@ isint2:
> >  switch(fu->bcnt) {
> > > >   (void)printf("\n");
> > > >   }
> > > >  #endif
> > > > +#ifdef __rtems__
> > > > + free(nextpr);
> > > > +#endif
> > >
> > > I know this has not been done in imported code in rtems.git before
> but
> > should we
> > > adopt the libbsd standard of adding /* __rtems__ */ to the #else
> and #endif?
> >
> > Probably, but we also clone-and-own this shell/ code, and we should
> > not bother with these #ifdefs in there. I think I have said this 3
> > times this past week about shell/. The upstream does not want our
> > changes, and we don't import from them anymore.
> >
> > Some of these files have massive changes and some don't. Ryan and
> > I looked at main_cp.c and it had at least 40 revisions since the version
> > we have. The same Coverity issue appeared to be present but the variable
> > names were changed and much clearer.
>
> Yes.
>
> > Chris imported this code initially. I'll trust his ruling on this since
> I assume
> > he is likely to either be the one to update it eventually or have to
> help someone
> > a lot. :)
>
> If the code was updated I would use the libbsd way of handing it. It has
> been
> proven to work.
>
> > And some of it is close enough that the ifdef's are worth it. Some
> isn't.
> > Hard call on the overall value.
>
> For the existing code it is hard to call. If I was starting again I would
> say we
> had to support a clean separation.
>

I assume you would checkout the original version and diff it. But would the
ifdef rtems ease the burden any?

I tend to like going with them otherwise, we have some files we do it
on and others we don't.

--joel

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

Re: [PATCH 1/5] hexdump-parse.c: Fix Resource leak (CID #26032)

[Chris Johns]

On 16/3/21 6:55 am, Joel Sherrill wrote:
> 
> 
> On Mon, Mar 15, 2021 at 2:46 PM Gedare Bloom  > wrote:
> 
> On Sun, Mar 14, 2021 at 8:27 PM Chris Johns  > wrote:
> >
> > On 13/3/21 2:18 am, Ryan Long wrote:
> > > CID 26032: Resource leak in rtems_shell_hexdump_rewrite().
> > >
> > > Closes #4296
> > > ---
> > >  cpukit/libmisc/shell/hexdump-parse.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/cpukit/libmisc/shell/hexdump-parse.c
> b/cpukit/libmisc/shell/hexdump-parse.c
> > > index 88b9d56..5b56bbf 100644
> > > --- a/cpukit/libmisc/shell/hexdump-parse.c
> > > +++ b/cpukit/libmisc/shell/hexdump-parse.c
> > > @@ -462,6 +462,9 @@ isint2:                                 
>  switch(fu->bcnt) {
> > >               (void)printf("\n");
> > >       }
> > >  #endif
> > > +#ifdef __rtems__
> > > +     free(nextpr);
> > > +#endif
> >
> > I know this has not been done in imported code in rtems.git before but
> should we
> > adopt the libbsd standard of adding /* __rtems__ */ to the #else and 
> #endif?
> 
> Probably, but we also clone-and-own this shell/ code, and we should
> not bother with these #ifdefs in there. I think I have said this 3
> times this past week about shell/. The upstream does not want our
> changes, and we don't import from them anymore.
> 
> Some of these files have massive changes and some don't. Ryan and 
> I looked at main_cp.c and it had at least 40 revisions since the version
> we have. The same Coverity issue appeared to be present but the variable
> names were changed and much clearer.

Yes.

> Chris imported this code initially. I'll trust his ruling on this since I 
> assume
> he is likely to either be the one to update it eventually or have to help 
> someone
> a lot. :)

If the code was updated I would use the libbsd way of handing it. It has been
proven to work.

> And some of it is close enough that the ifdef's are worth it. Some isn't. 
> Hard call on the overall value.

For the existing code it is hard to call. If I was starting again I would say we
had to support a clean separation.

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

Re: [PATCH 1/2] covoar/CoverageReaderQEMU: Fix infinite loop

[Joel Sherrill]

On Mon, Mar 15, 2021 at 5:58 PM Chris Johns  wrote:

> On 16/3/21 9:11 am, Gedare Bloom wrote:
> > On Mon, Mar 15, 2021 at 3:34 PM Alex White  > > wrote:
> >
> > I honestly can't remember why I changed 1024 to 20,000.
> >
> > I've looked back at that code and changed it back to 1024 without any
> > issues. I think I might have missed that this is all happening in a
> loop,
> > and at one point during a (long) debugging session I convinced
> myself that
> > it wasn't reading all of the entries.
> >
> > At least that's the most rational explanation I can think up for that
> > particular change. 
> >
> > If I revert ENTRIES from 2 back to 1024, are we satisfied to
> leave the
> > "entries" array as-is?
> >
> > I think that Chris' main points here are that, as you get covoar working
> again
> > and cleaning it up, it should be made more C++ (and less C).
>
> Thanks Gedare, yes I am asking if this could be considered. A total
> conversion
> is not realistic and would be asking too much but my hope is making
> changes in
> small pieces can be done. Some changes will requiring new C++ skills but
> that
> should be thought of as a fun challenge.
>
> In this case I think changing to a vector would be a good thing for 1024
> entries
> but we had 1024 in the code previously and it was fine so I am also OK if
> it
> left that way.
>


This is a different case than many of the others, it is reading a block of
fixed size
binary entries from the Qemu trace log. It is just avoiding reading the
32-byte
entries one at a time. It is read, process, and discard. How would
std::ANYTHING help here?

--joel


>
> Chris
> ___
> 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 1/2] covoar/CoverageReaderQEMU: Fix infinite loop

[Chris Johns]

On 16/3/21 9:11 am, Gedare Bloom wrote:
> On Mon, Mar 15, 2021 at 3:34 PM Alex White  > wrote:
> 
> I honestly can't remember why I changed 1024 to 20,000.
> 
> I've looked back at that code and changed it back to 1024 without any
> issues. I think I might have missed that this is all happening in a loop,
> and at one point during a (long) debugging session I convinced myself that
> it wasn't reading all of the entries.
> 
> At least that's the most rational explanation I can think up for that
> particular change. 
> 
> If I revert ENTRIES from 2 back to 1024, are we satisfied to leave the
> "entries" array as-is?
> 
> I think that Chris' main points here are that, as you get covoar working again
> and cleaning it up, it should be made more C++ (and less C).

Thanks Gedare, yes I am asking if this could be considered. A total conversion
is not realistic and would be asking too much but my hope is making changes in
small pieces can be done. Some changes will requiring new C++ skills but that
should be thought of as a fun challenge.

In this case I think changing to a vector would be a good thing for 1024 entries
but we had 1024 in the code previously and it was fine so I am also OK if it
left that way.

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

Re: [PATCH 1/2] covoar/CoverageReaderQEMU: Fix infinite loop

[Chris Johns]

On 16/3/21 8:34 am, Alex White wrote:
> I honestly can't remember why I changed 1024 to 20,000.
> 
> I've looked back at that code and changed it back to 1024 without any issues. 
> I think I might have missed that this is all happening in a loop, and at one 
> point during a (long) debugging session I convinced myself that it wasn't 
> reading all of the entries.
> 
> At least that's the most rational explanation I can think up for that 
> particular change. 

Great :)

> If I revert ENTRIES from 2 back to 1024, are we satisfied to leave the 
> "entries" array as-is?

Yes. The need for a change was not my focus rather it was how it was being
handled so I am fine with the old value.

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

Re: [PATCH 1/2] covoar/CoverageReaderQEMU: Fix infinite loop

[Gedare Bloom]

On Mon, Mar 15, 2021 at 3:34 PM Alex White  wrote:

> I honestly can't remember why I changed 1024 to 20,000.
>
> I've looked back at that code and changed it back to 1024 without any
> issues. I think I might have missed that this is all happening in a loop,
> and at one point during a (long) debugging session I convinced myself that
> it wasn't reading all of the entries.
>
> At least that's the most rational explanation I can think up for that
> particular change. 
>
> If I revert ENTRIES from 2 back to 1024, are we satisfied to leave the
> "entries" array as-is?
>
>
I think that Chris' main points here are that, as you get covoar working
again and cleaning it up, it should be made more C++ (and less C).


> Alex
>
> -Original Message-
> From: Chris Johns 
> Sent: Sunday, March 14, 2021 7:27 PM
> To: Alex White ; devel@rtems.org
> Subject: Re: [PATCH 1/2] covoar/CoverageReaderQEMU: Fix infinite loop
>
> On 12/3/21 5:30 am, Alex White wrote:
> > There was a potential that the branch info loop never terminated.
> > This has been fixed by adding a more reliable termination condition
> > and logging an error if it cannot find the branch target.
> > ---
> >  tester/covoar/CoverageReaderQEMU.cc | 11 +--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/tester/covoar/CoverageReaderQEMU.cc
> > b/tester/covoar/CoverageReaderQEMU.cc
> > index 7c344e4..fb1709d 100644
> > --- a/tester/covoar/CoverageReaderQEMU.cc
> > +++ b/tester/covoar/CoverageReaderQEMU.cc
> > @@ -76,7 +76,7 @@ namespace Coverage {
> >  //
> >  // Read ENTRIES number of trace entries.
> >  //
> > -#define ENTRIES 1024
> > +#define ENTRIES 2
>
> 1024 sure, 20,000 ... hmmm ... I am not so sure. If you need more would is
> the change 200,000? Maybe a better solution exists.
>
> >  while (true) {
> >CoverageMapBase *aCoverageMap = NULL;
> >struct trace_entry  entries[ENTRIES];
>
> Can an array or resized vector be used here?
>
> > @@ -118,8 +118,15 @@ namespace Coverage {
> >  // Determine if additional branch information is available.
> >  if ( (entry->op & branchInfo) != 0 ) {
> >uint32_t  a = entry->pc + entry->size - 1;
>
> An aside ... more pointers being used.
>
> Chris
>
> > -while (!aCoverageMap->isStartOfInstruction(a))
> > +while (a > entry->pc &&
> > + !aCoverageMap->isStartOfInstruction(a))
> >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) {
> >aCoverageMap->setWasTaken( a );
> >  } else if (entry->op & notTaken) {
> >
> ___
> 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 3/5] nvdisk.c: Fix Resource leak (CID #1439297)

[Gedare Bloom]

On Mon, Mar 15, 2021 at 2:28 PM Joel Sherrill  wrote:
>
>
>
> On Mon, Mar 15, 2021, 3:10 PM Gedare Bloom  wrote:
>>
>> On Fri, Mar 12, 2021 at 8:18 AM Ryan Long  wrote:
>> >
>> > CID 1439297: Resource leak in rtems_nvdisk_initialize().
>> >
>> > Closes #4298
>> > ---
>> >  cpukit/libblock/src/nvdisk.c | 8 +++-
>> >  1 file changed, 7 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/cpukit/libblock/src/nvdisk.c b/cpukit/libblock/src/nvdisk.c
>> > index a7f4167..d742baf 100644
>> > --- a/cpukit/libblock/src/nvdisk.c
>> > +++ b/cpukit/libblock/src/nvdisk.c
>> > @@ -766,8 +766,10 @@ rtems_nvdisk_initialize (rtems_device_major_number 
>> > major RTEMS_UNUSED,
>> >  nvd->info_level   = c->info_level;
>> >
>> >  nvd->devices = calloc (c->device_count, sizeof 
>> > (rtems_nvdisk_device_ctl));
>> > -if (!nvd->devices)
>> > +if (!nvd->devices) {
>> > +  free(nvd);
>> >return RTEMS_NO_MEMORY;
>> > +}
>> >
>> >  for (device = 0; device < c->device_count; device++)
>> >  {
>> > @@ -790,6 +792,8 @@ rtems_nvdisk_initialize (rtems_device_major_number 
>> > major RTEMS_UNUSED,
>> >   rtems_nvdisk_ioctl, nvd);
>> >  if (sc != RTEMS_SUCCESSFUL)
>> >  {
>> > +  free(nvd->devices);
>> > +  free(nvd);
>> >rtems_nvdisk_error ("disk create phy failed");
>> >return sc;
>> >  }
>> > @@ -797,5 +801,7 @@ rtems_nvdisk_initialize (rtems_device_major_number 
>> > major RTEMS_UNUSED,
>> >  rtems_mutex_init (>lock, "NV Disk");
>> >}
>> >
>> > +  free(nvd->devices);
>> > +  free(nvd);
>>
>> Does the rtems_blkdev_create() link the nvd? (I think the answer is
>> yes.) Then, this is not correct.
>
>
> I think this is my suggestion so what do you think should happen on this case?

Does the block device driver register a callback handler for the
device_data destruction?

>>
>>
>> >return RTEMS_SUCCESSFUL;
>> >  }
>> > --
>> > 1.8.3.1
>> >
>> > ___
>> > 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
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

[PATCH v2 2/4] coverage/reports: Improve formatting and clarity

[Alex White]

The coverage reports contain places where they display incorrect or
vague information particularly when some statistic is unavailable. This
has been fixed. The formatting and wording of various things has been
improved as well.
---
 tester/covoar/ObjdumpProcessor.cc |   5 +
 tester/covoar/ReportsBase.cc  |  21 +++-
 tester/covoar/ReportsHtml.cc  | 159 +-
 tester/covoar/ReportsText.cc  |  76 --
 tester/rt/coverage.py |  19 +++-
 5 files changed, 173 insertions(+), 107 deletions(-)

diff --git a/tester/covoar/ObjdumpProcessor.cc 
b/tester/covoar/ObjdumpProcessor.cc
index 544bfa1..f90138c 100644
--- a/tester/covoar/ObjdumpProcessor.cc
+++ b/tester/covoar/ObjdumpProcessor.cc
@@ -374,6 +374,11 @@ namespace Coverage {
 break;
   }
 
+  // Remove any extra line break
+  if (line.back() == '\n') {
+line.erase(line.end() - 1);
+  }
+
   lineInfo.line  = line;
   lineInfo.address   = 0x;
   lineInfo.isInstruction = false;
diff --git a/tester/covoar/ReportsBase.cc b/tester/covoar/ReportsBase.cc
index 0be5567..1615c0b 100644
--- a/tester/covoar/ReportsBase.cc
+++ b/tester/covoar/ReportsBase.cc
@@ -161,6 +161,24 @@ void  ReportsBase::CloseSymbolSummaryFile(
   CloseFile( aFile );
 }
 
+std::string expand_tabs(const std::string& in) {
+  std::string expanded = "";
+  int i = 0;
+
+  for (char c : in) {
+if (c == '\t') {
+  int num_tabs = 4 - (i % 4);
+  expanded.append(num_tabs, ' ');
+  i += num_tabs;
+} else {
+  expanded += c;
+  i++;
+}
+  }
+
+  return expanded;
+}
+
 /*
  *  Write annotated report
  */
@@ -237,7 +255,8 @@ void ReportsBase::WriteAnnotatedReport(
 }
   }
 
-  snprintf( textLine, LINE_LENGTH, "%-70s", itr->line.c_str() );
+  std::string textLineWithoutTabs = expand_tabs(itr->line);
+  snprintf( textLine, LINE_LENGTH, "%-90s", textLineWithoutTabs.c_str() );
   line = textLine + annotation;
 
   PutAnnotatedLine( aFile, state, line, id);
diff --git a/tester/covoar/ReportsHtml.cc b/tester/covoar/ReportsHtml.cc
index ebc6ee0..cce0a4f 100644
--- a/tester/covoar/ReportsHtml.cc
+++ b/tester/covoar/ReportsHtml.cc
@@ -89,7 +89,7 @@ namespace Coverage {
 PRINT_ITEM( "Branch Report","branch" );
 PRINT_ITEM( "Annotated Assembly",   "annotated" );
 PRINT_ITEM( "Symbol Summary",   "symbolSummary" );
-PRINT_ITEM( "Size Report",  "sizes" );
+PRINT_ITEM( "Uncovered Range Size Report",  "sizes" );
 
 PRINT_TEXT_ITEM( "Explanations Not Found", "ExplanationsNotFound.txt" );
 
@@ -176,7 +176,7 @@ namespace Coverage {
   // Put header information into the file
   fprintf(
 aFile,
-"Branch ReportBranch Report\n"
 ""
   );
 
@@ -321,7 +321,7 @@ namespace Coverage {
 // Put header information into the file
 fprintf(
   aFile,
-  "Size Report\n"
+  "Uncovered Range Size Report\n"
 ""
 );
 
@@ -334,7 +334,7 @@ namespace Coverage {
 
 fprintf(
   aFile,
-  "Size Report\n"
+  "Uncovered Range Size Report\n"
"%s\n"
   "\n"
   "getNumberBranchesFound() != 0)
   fprintf( report, "All branch paths taken.\n" );
 else
   fprintf( report, "No branch information found.\n" );
@@ -861,90 +861,107 @@ namespace Coverage {
   symbol->first.c_str()
 );
 
-// Total Size in Bytes
-fprintf(
-  report,
-  "%d\n",
-  symbol->second.stats.sizeInBytes
-);
-
-// Total Size in Instructions
-fprintf(
-  report,
-  "%d\n",
-  symbol->second.stats.sizeInInstructions
-);
-
-// Total Uncovered Ranges
-fprintf(
-  report,
-  "%d\n",
-  symbol->second.stats.uncoveredRanges
-);
-
-// Uncovered Size in Bytes
-fprintf(
-  report,
-  "%d\n",
-  symbol->second.stats.uncoveredBytes
-);
-
-// Uncovered Size in Instructions
-fprintf(
-  report,
-  "%d\n",
-   symbol->second.stats.uncoveredInstructions
-);
+if (symbol->second.stats.sizeInBytes == 0) {
+  // The symbol has never been seen. Write "unknown" for all columns.
+  fprintf(
+report,
+"unknown\n"
+"unknown\n"
+"unknown\n"
+"unknown\n"
+"unknown\n"
+"unknown\n"
+"unknown\n"
+"unknown\n"
+"unknown\n"
+"unknown\n"
+  );
+} else {
+  // Total Size in Bytes
+  fprintf(
+report,
+"%d\n",
+symbol->second.stats.sizeInBytes
+  );
 
-// Total number of branches
-fprintf(
-  report,
-  "%d\n",
-  symbol->second.stats.branchesNotExecuted +  
symbol->second.stats.branchesExecuted
-);
+  // Total Size in Instructions
+  fprintf(
+report,
+"%d\n",
+symbol->second.stats.sizeInInstructions
+  );
 
-// Total Always Taken
-fprintf(
-  

[PATCH v2 3/4] coverage/reports: Share common JS and CSS in reports

[Alex White]

This moves all of the javascript and CSS files that are shared by the
symbol set HTML reports to the shared parent directory. It also includes
the javascript and CSS in the top-level index file.
---
 tester/covoar/ReportsHtml.cc |  4 ++--
 tester/rt/coverage.py| 11 ++-
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/tester/covoar/ReportsHtml.cc b/tester/covoar/ReportsHtml.cc
index cce0a4f..3d20aec 100644
--- a/tester/covoar/ReportsHtml.cc
+++ b/tester/covoar/ReportsHtml.cc
@@ -121,8 +121,8 @@ namespace Coverage {
   "\n"
   "\n"
   "\n"
-  "\n"
-  "\n"
+  "\n"
+  "\n"
 );
 
 return aFile;
diff --git a/tester/rt/coverage.py b/tester/rt/coverage.py
index d62c853..ec3957c 100644
--- a/tester/rt/coverage.py
+++ b/tester/rt/coverage.py
@@ -137,6 +137,8 @@ class report_gen_html:
 head_section += 'text-align:center;' + os.linesep
 head_section += '  }' + os.linesep
 head_section += ' ' + os.linesep
+head_section += ' ' + os.linesep
+head_section += ' ' + os.linesep
 head_section += '' + os.linesep
 return head_section
 
@@ -221,11 +223,10 @@ class report_gen_html:
 def add_covoar_css(self):
 table_js_path = path.join(self.covoar_src_path, 'table.js')
 covoar_css_path = path.join(self.covoar_src_path, 'covoar.css')
-for symbol_set in self.symbol_sets:
-symbol_set_dir = path.join(self.build_dir,
-   self.bsp + '-coverage', symbol_set)
-path.copy_tree(covoar_css_path, symbol_set_dir)
-path.copy_tree(table_js_path, symbol_set_dir)
+coverage_directory = path.join(self.build_dir,
+self.bsp + '-coverage')
+path.copy_tree(covoar_css_path, coverage_directory)
+path.copy_tree(table_js_path, coverage_directory)
 
 def add_dir_name(self):
 for symbol_set in self.symbol_sets:
-- 
2.27.0

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

[PATCH v2 4/4] coverage: Give coverage bars red background

[Alex White]

---
 tester/rt/coverage.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tester/rt/coverage.py b/tester/rt/coverage.py
index ec3957c..a561d26 100644
--- a/tester/rt/coverage.py
+++ b/tester/rt/coverage.py
@@ -129,6 +129,9 @@ class report_gen_html:
 head_section += 'width: 150px;' + os.linesep
 head_section += 'height: 15px;' + os.linesep
 head_section += '  }' + os.linesep
+head_section += '  progress::-webkit-progress-bar {' + os.linesep
+head_section += 'background: red;' + os.linesep
+head_section += '  }' + os.linesep
 head_section += '  table, th, td {' + os.linesep
 head_section += 'border: 1px solid black;' + os.linesep
 head_section += 'border-collapse: collapse;' + os.linesep
-- 
2.27.0

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

[PATCH v2 0/4] coverage/reports: Improve coverage reports

[Alex White]

v2:
- Replace tab expansion function in ReportsBase with std::string version

This patch set includes a few improvements to the coverage reports.

Alex White (4):
  covoar/reports: Add new statistics to summary
  coverage/reports: Improve formatting and clarity
  coverage/reports: Share common JS and CSS in reports
  coverage: Give coverage bars red background

 tester/covoar/DesiredSymbols.cc   |  10 ++
 tester/covoar/DesiredSymbols.h|  24 -
 tester/covoar/ObjdumpProcessor.cc |   5 +
 tester/covoar/ReportsBase.cc  |  67 ++--
 tester/covoar/ReportsHtml.cc  | 163 +-
 tester/covoar/ReportsText.cc  |  76 --
 tester/rt/coverage.py |  66 
 7 files changed, 275 insertions(+), 136 deletions(-)

-- 
2.27.0

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

[PATCH v2 1/4] covoar/reports: Add new statistics to summary

[Alex White]

The following new statistics have been added to the summary report:
number of unreferenced symbols, total branch paths found, number of
branch paths not executed, and percentage of branch paths covered.
---
 tester/covoar/DesiredSymbols.cc | 10 +++
 tester/covoar/DesiredSymbols.h  | 24 -
 tester/covoar/ReportsBase.cc| 46 +++--
 tester/rt/coverage.py   | 33 +--
 4 files changed, 91 insertions(+), 22 deletions(-)

diff --git a/tester/covoar/DesiredSymbols.cc b/tester/covoar/DesiredSymbols.cc
index c97b25c..ffc4f86 100644
--- a/tester/covoar/DesiredSymbols.cc
+++ b/tester/covoar/DesiredSymbols.cc
@@ -175,6 +175,8 @@ namespace Coverage {
 s.second.stats.uncoveredBytes++;
   }
 }
+  } else {
+stats.unreferencedSymbols++;
   }
 }
   }
@@ -470,10 +472,18 @@ namespace Coverage {
 return stats.branchesNeverTaken;
   };
 
+  uint32_t DesiredSymbols::getNumberBranchesNotExecuted( void ) const {
+return stats.branchesNotExecuted;
+  };
+
   uint32_t DesiredSymbols::getNumberUncoveredRanges( void ) const {
 return stats.uncoveredRanges;
   };
 
+  uint32_t DesiredSymbols::getNumberUnreferencedSymbols( void ) const {
+return stats.unreferencedSymbols;
+  };
+
   bool DesiredSymbols::isDesired (
 const std::string& symbolName
   ) const
diff --git a/tester/covoar/DesiredSymbols.h b/tester/covoar/DesiredSymbols.h
index 367ca95..5cf96e9 100644
--- a/tester/covoar/DesiredSymbols.h
+++ b/tester/covoar/DesiredSymbols.h
@@ -82,6 +82,11 @@ namespace Coverage {
  */
 int uncoveredRanges;
 
+/*!
+ *  This member variable contains the total number of unreferenced symbols.
+ */
+int unreferencedSymbols;
+
 /*!
  *  This method returns the percentage of uncovered instructions.
  *
@@ -109,7 +114,8 @@ namespace Coverage {
sizeInInstructions(0),
uncoveredBytes(0),
uncoveredInstructions(0),
-   uncoveredRanges(0)
+   uncoveredRanges(0),
+   unreferencedSymbols(0)
  {
  }
 
@@ -278,6 +284,14 @@ namespace Coverage {
  */
 uint32_t getNumberBranchesNeverTaken( void ) const;
 
+/*!
+ *  This method returns the total number of branches not executed
+ *  for all analyzed symbols.
+ *
+ *  @return Returns the total number of branches not executed
+ */
+uint32_t getNumberBranchesNotExecuted( void ) const;
+
 /*!
  *  This method returns the total number of uncovered ranges
  *  for all analyzed symbols.
@@ -286,6 +300,14 @@ namespace Coverage {
  */
 uint32_t getNumberUncoveredRanges( void ) const;
 
+/*!
+ *  This method returns the total number of unreferenced symbols
+ *  for all analyzed symbols.
+ *
+ *  @return Returns the total number of unreferenced symbols
+ */
+uint32_t getNumberUnreferencedSymbols( void ) const;
+
 /*!
  *  This method returns an indication of whether or not the specified
  *  symbol is a symbol to analyze.
diff --git a/tester/covoar/ReportsBase.cc b/tester/covoar/ReportsBase.cc
index 0244601..0be5567 100644
--- a/tester/covoar/ReportsBase.cc
+++ b/tester/covoar/ReportsBase.cc
@@ -441,6 +441,7 @@ void  ReportsBase::WriteSummaryReport(
   Coverage::DesiredSymbols::symbolSet_t::iterator itr;
   uint32_tnotExecuted = 0;
   double  percentage;
+  double  percentageBranches;
   Coverage::CoverageMapBase*  theCoverageMap;
   uint32_ttotalBytes = 0;
   FILE*   report;
@@ -475,13 +476,26 @@ void  ReportsBase::WriteSummaryReport(
   percentage /= (double) totalBytes;
   percentage *= 100.0;
 
-  fprintf( report, "Bytes Analyzed   : %d\n", totalBytes );
-  fprintf( report, "Bytes Not Executed   : %d\n", notExecuted );
-  fprintf( report, "Percentage Executed  : %5.4g\n", 100.0 - percentage  );
-  fprintf( report, "Percentage Not Executed  : %5.4g\n", percentage  );
+  percentageBranches = (double) (
+SymbolsToAnalyze->getNumberBranchesAlwaysTaken() +
+  SymbolsToAnalyze->getNumberBranchesNeverTaken() +
+  (SymbolsToAnalyze->getNumberBranchesNotExecuted() * 2)
+  );
+  percentageBranches /= (double) SymbolsToAnalyze->getNumberBranchesFound() * 
2;
+  percentageBranches *= 100.0;
+
+  fprintf( report, "Bytes Analyzed   : %d\n", totalBytes );
+  fprintf( report, "Bytes Not Executed   : %d\n", notExecuted );
+  fprintf( report, "Percentage Executed  : %5.4g\n", 100.0 - 
percentage  );
+  fprintf( report, "Percentage Not Executed  : %5.4g\n", percentage  );
   fprintf(
 report,
-"Uncovered ranges found   : %d\n",
+"Unreferenced Symbols : %d\n",
+SymbolsToAnalyze->getNumberUnreferencedSymbols()

Re: [PATCH 1/2] objectextendinformation.c: Fix Dereference after null check (CID #26033)

[Gedare Bloom]

On Mon, Mar 15, 2021 at 2:27 PM Joel Sherrill  wrote:
>
>
>
> On Mon, Mar 15, 2021, 3:21 PM Gedare Bloom  wrote:
>>
>> On Fri, Mar 12, 2021 at 7:55 AM Ryan Long  wrote:
>> >
>> > CID 26033: Dereference after null check in _Objects_Extend_information().
>> >
>> > Closes #4326
>> > ---
>> >  cpukit/score/src/objectextendinformation.c | 11 +++
>> >  1 file changed, 11 insertions(+)
>> >
>> > diff --git a/cpukit/score/src/objectextendinformation.c 
>> > b/cpukit/score/src/objectextendinformation.c
>> > index 9796eb9..c669301 100644
>> > --- a/cpukit/score/src/objectextendinformation.c
>> > +++ b/cpukit/score/src/objectextendinformation.c
>> > @@ -171,6 +171,17 @@ Objects_Maximum _Objects_Extend_information(
>> >
>> >  if ( old_maximum > extend_count ) {
>> >/*
>> > +   * Coverity thinks there is a way for this to be NULL (CID #26033).
>> > +   * After much time spent analyzing this, no one has identified the
>> > +   * conditions where this can actually occur. Adding this _Assert 
>> > ensures
>> > +   * that it is never NULL. If this assert is triggered, condition
>> > +   * generating this case will have been identified and it can be 
>> > revisted.
>> > +   * This is being done out of an abundance of caution since we could 
>> > have
>> > +   * easily flagged this as a false positive and ignored it 
>> > completely.
>> > +   */
>> > +  _Assert(information->object_blocks != NULL);
>> > +
>> That's interesting. It would help if you could share your analysis.
>
>
> This is the oldest Coverity issue that is open. It is over five years old. 
> Chris and I have tried multiple times to figure out if it is valid. We never 
> get any confidence that it cannot occur.
>
>>
>> How does
>> 70  if ( information->object_blocks == NULL ) {
>> be true, and if it is true, how does the exectuion flow proceed in
>> such a way that it will not reach this assert?
>
>
> No idea but it apparently doesn't based on our tests.
>
> Adding the assert is an attempt to finally find the case that trips this. It 
> is either something I can never occur or something we don't know how to make 
> happen. Either way the asserting like a good idea.
>
> if you have a test case in mind that can reproduce this coverity path, let's 
> try it and push this to failure. But we have no evidence that it's ever 
> occurred in the field.

Can information->object_blocks be NULL at line 70?

>>
>>
>>
>> > +  /*
>> > *  Copy each section of the table over. This has to be performed as
>> > *  separate parts as size of each block has changed.
>> > */
>> > --
>> > 1.8.3.1
>> >
>> > ___
>> > 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
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [PATCH v2] user/bsps: Mention fixed console baud rate for zynq

[Gedare Bloom]

Yeah go ahead, thanks Jan.

On Mon, Mar 15, 2021 at 6:58 AM Joel Sherrill  wrote:
>
> If this matches the state of the got master, then ok.
>
> On Mon, Mar 15, 2021, 7:55 AM  wrote:
>>
>> Could someone please have a look at this patch?
>>
>> > -Original Message-
>> > From: Sommer, Jan
>> > Sent: Friday, March 5, 2021 7:04 PM
>> > To: devel@rtems.org
>> > Cc: Sommer, Jan 
>> > Subject: [PATCH v2] user/bsps: Mention fixed console baud rate for zynq
>> >
>> > ---
>> >  user/bsps/arm/xilinx-zynq.rst | 12 
>> >  1 file changed, 12 insertions(+)
>> >
>> > diff --git a/user/bsps/arm/xilinx-zynq.rst b/user/bsps/arm/xilinx-zynq.rst
>> > index 365c336..29f9cb0 100644
>> > --- a/user/bsps/arm/xilinx-zynq.rst
>> > +++ b/user/bsps/arm/xilinx-zynq.rst
>> > @@ -37,6 +37,18 @@ to return the peripheral clock. Normally this is half 
>> > the
>> > CPU  clock. This function is declared ``weak`` so you can override the  
>> > default
>> > behaviour by providing it in your application.
>> >
>> > +Console
>> > +---
>> > +
>> > +The console driver for the UARTs will always be initialized to a baud
>> > +rate of 115200 with 8 bit characters, 1 stop bit and no parity bits
>> > +during start up.
>> > +Previous configurations programmed into the hardware by the Xilinx
>> > +tools or a bootloader will be overwritten.
>> > +
>> > +The settings for the console driver can be changed by the user
>> > +application through the termios API afterwards.
>> > +
>> >  Debugging with xilinx_zynq_a9_qemu
>> >  --
>> >
>> > --
>> > 2.17.1
>>
>> ___
>> 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
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: code reformatting was Re: Regarding gsoc project 3860

[Gedare Bloom]

On Mon, Mar 15, 2021 at 6:44 AM Joel Sherrill  wrote:
>
>
>
> On Mon, Mar 15, 2021 at 4:20 AM Hesham Almatary 
>  wrote:
>>
>> Hello Ayushman and Ida,
>>
>> Usually, if multiple students really want to work on a particular
>> project (and can't/don't want to choose another), there can be
>> multiple proposals for the same project and we choose the best one.
>> Sometimes a project can be split up between two students to work on to
>> minimise conflicts.
>
>
> There are multiple things that need to be addressed here.
>
> First, there have been discussions on devel@ about code formatting tools.
> Sebastian has posted a configuration for the indent program but offhand
> I don't know where that is. It may be in the documentation.
>
I posted about this to Ida. I think it was uncrustify? I think several
tools have been looked into. No specific tool is required, but we
should pick the one that best allows us to meet the needs of the
project.

> For indent to move forward from here, its impact on the code in a directory
> that is thought to follow the RTEMS style well would need to be evaluated.
> Do the rules need to be tweaked to avoid changes? Is the source code actually
> just not in conformance with published rules? The process here is to evaluate
> the difference between tool output and existing code and work to close the
> delta by tweaking rules and fixing code. The end is expected to be that there
> are a few places where the tool can't produce RTEMS style and we have to
> discuss adopting something the tool can produce.
>
> I don't recall if Sebastian evaluated the llvm formatter and created a 
> configuration
> for it. In this case, creating a configuration for this tool before 
> evaluating the
> difference in output would be the path forward. If this formatter is better, 
> then
> I would like to see an RTEMS style added to their options.
>
> With either tool, a factor is integrating it into the development process. I'm
> not sure what a GSoC project would do about this.
>

I think the tool integration is the main piece of GSoC-relevant work,
as this would involve some level of scripting and automation.

> So there are two potential projects here. My question is not conflict on
> project choice, it is whether either is an appropriate GSoC project. With
> the shorter time frame, I think the scope of the project is in the right 
> ballpark.
> Is there enough coding? I don't know.
>

I'm not currently convinced there is enough coding work for two
projects in this area. I don't think there would have been enough
coding work for one project under the old GSoC scope.

Running the code formatter and submitting patches won't really count
as "code contributions"

> --joel
>
>>
>>
>> On Mon, 15 Mar 2021 at 09:45, Ida Delphine  wrote:
>> >
>> > Umm...did you bring up a discussion regarding this project earlier?
>> >
I do not have a record of Ayushman "claiming" this project, and anyway
we don't allow students to "claim" a project.

>> > On Mon, 15 Mar 2021, 8:10 am Ayushman Mishra,  
>> > wrote:
>> >>
>> >> AYUSHMAN MISHRA
>> >>
>> >> Hello Ida delphini AYUSHMAN here , Can you please select any other 
>> >> project for gsoc as I am also currently working on proposal for the same 
>> >> project
>> >> https://devel.rtems.org/ticket/3860 for gsoc 2021
>> >
Ayushman, this is not a polite request for you to make, in addition it
would best have been made by direct reply to her email in the same
thread, not by starting a new e-mail thread. In an open-source
community, you should not impose yourself on another person. It goes
against the fundamental ideas of "freedom" that open-source is based
on. Part of GSoC is exactly about learning this kind of lesson, so
don't feel too bad about it, but do pay attention to how you interact
with others and make sure you are respecting their autonomy and
perspectives.

>> > ___
>> > 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
>
> ___
> 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

[PATCH v2] covoar: Handle periods in symbols from objdump

[Alex White]

Occasionally the compiler will generate symbols that look similar to
symbols defined in RTEMS code except that they contain some suffix.
This looks to be related to compiler optimizations. Such symbols were
being treated as unique. For our purposes, they should be mapped to
the equivalent symbols in the DWARF info. This has been fixed.
---
 tester/covoar/ExecutableInfo.cc   | 22 +++---
 tester/covoar/ObjdumpProcessor.cc |  6 ++
 tester/covoar/SymbolTable.cc  | 12 +---
 3 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/tester/covoar/ExecutableInfo.cc b/tester/covoar/ExecutableInfo.cc
index c996d75..9384973 100644
--- a/tester/covoar/ExecutableInfo.cc
+++ b/tester/covoar/ExecutableInfo.cc
@@ -118,8 +118,7 @@ namespace Coverage {
 // Obtain the coverage map containing the specified address.
 itsSymbol = theSymbolTable.getSymbol( address );
 if (itsSymbol != "") {
-  it = coverageMaps.find( itsSymbol );
-  aCoverageMap = (*it).second;
+  aCoverageMap = (itsSymbol);
 }
 
 return aCoverageMap;
@@ -150,8 +149,25 @@ namespace Coverage {
   )
   {
 CoverageMaps::iterator cmi = coverageMaps.find( symbolName );
-if ( cmi == coverageMaps.end() )
+if ( cmi != coverageMaps.end() ) {
+  return *(cmi->second);
+}
+
+size_t periodIndex = symbolName.find(".");
+
+if (periodIndex == std::string::npos) {
+  // Symbol name has no '.', can't do another lookup.
   throw CoverageMapNotFoundError(symbolName);
+}
+
+cmi = coverageMaps.find(
+  symbolName.substr(0, periodIndex)
+);
+
+if ( cmi == coverageMaps.end() ) {
+  throw CoverageMapNotFoundError(symbolName);
+}
+
 return *(cmi->second);
   }
 
diff --git a/tester/covoar/ObjdumpProcessor.cc 
b/tester/covoar/ObjdumpProcessor.cc
index fa9894d..544bfa1 100644
--- a/tester/covoar/ObjdumpProcessor.cc
+++ b/tester/covoar/ObjdumpProcessor.cc
@@ -417,6 +417,12 @@ namespace Coverage {
 processSymbol = false;
 theInstructions.clear();
 
+// Look for a '.' character and strip everything after it.
+char *periodIndex = strstr(symbol, ".");
+if (periodIndex != NULL) {
+  *periodIndex = 0;
+}
+
 // See if the new symbol is one that we care about.
 if (SymbolsToAnalyze->isDesired( symbol )) {
   currentSymbol = symbol;
diff --git a/tester/covoar/SymbolTable.cc b/tester/covoar/SymbolTable.cc
index 53bc8af..00062cc 100644
--- a/tester/covoar/SymbolTable.cc
+++ b/tester/covoar/SymbolTable.cc
@@ -46,12 +46,18 @@ namespace Coverage {
 symbolData.startingAddress = start;
 symbolData.length = length;
 
-if ( info[ symbol ].empty() == false ) {
-  if ( info[ symbol ].front().length != length ) {
+for (auto& symData : info[ symbol ]) {
+  // The starting address could differ since we strip any suffixes 
beginning
+  // with a '.'
+  if (symData.startingAddress != start) {
+continue;
+  }
+
+  if (symData.length != length) {
 std::ostringstream what;
 what << "Different lengths for the symbol "
  << symbol
- << " (" << info[ symbol ].front().length
+ << " (" << symData.length
  << " and " << length
  << ")";
 throw rld::error( what, "SymbolTable::addSymbol" );
-- 
2.27.0

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

Re: [PATCH 1/1] Edited hello world message in init.c as prerequisite for GSOC application.

[Gedare Bloom]

Hi Matt,


On Mon, Mar 15, 2021 at 2:50 PM Matt Joyce  wrote:
>
> ---
>  testsuites/samples/hello/init.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/testsuites/samples/hello/init.c b/testsuites/samples/hello/init.c
> index 34ded37c55..48498fd52b 100644
> --- a/testsuites/samples/hello/init.c
> +++ b/testsuites/samples/hello/init.c
> @@ -22,7 +22,7 @@ static rtems_task Init(
>  {
>rtems_print_printer_fprintf_putc(_test_printer);
>TEST_BEGIN();
> -  printf( "Hello World\n" );
> +  printf("Hello RTEMS World! From Matt Joyce, GSOC 2021 Applicant.\n");
>TEST_END();

This is suitable for the submission.

See https://devel.rtems.org/wiki/Developer/Git#GitCommits for more
about preparing "real commits" in the future including the commit message
formatting.

(We need to merge that text from
https://devel.rtems.org/wiki/Developer/Git#GitCommits into a suitable
subsection in 
https://docs.rtems.org/branches/master/eng/vc-users.html#creating-a-patch
someday.)

I will send you separate instructions regarding your screenshot.

Gedare

>rtems_test_exit( 0 );
>  }
> --
> 2.30.2
>
> ___
> 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 1/2] covoar/CoverageReaderQEMU: Fix infinite loop

[Alex White]

I honestly can't remember why I changed 1024 to 20,000.

I've looked back at that code and changed it back to 1024 without any issues. I 
think I might have missed that this is all happening in a loop, and at one 
point during a (long) debugging session I convinced myself that it wasn't 
reading all of the entries.

At least that's the most rational explanation I can think up for that 
particular change. 

If I revert ENTRIES from 2 back to 1024, are we satisfied to leave the 
"entries" array as-is?

Alex

-Original Message-
From: Chris Johns  
Sent: Sunday, March 14, 2021 7:27 PM
To: Alex White ; devel@rtems.org
Subject: Re: [PATCH 1/2] covoar/CoverageReaderQEMU: Fix infinite loop

On 12/3/21 5:30 am, Alex White wrote:
> There was a potential that the branch info loop never terminated.
> This has been fixed by adding a more reliable termination condition 
> and logging an error if it cannot find the branch target.
> ---
>  tester/covoar/CoverageReaderQEMU.cc | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/tester/covoar/CoverageReaderQEMU.cc 
> b/tester/covoar/CoverageReaderQEMU.cc
> index 7c344e4..fb1709d 100644
> --- a/tester/covoar/CoverageReaderQEMU.cc
> +++ b/tester/covoar/CoverageReaderQEMU.cc
> @@ -76,7 +76,7 @@ namespace Coverage {
>  //
>  // Read ENTRIES number of trace entries.
>  //
> -#define ENTRIES 1024
> +#define ENTRIES 2

1024 sure, 20,000 ... hmmm ... I am not so sure. If you need more would is the 
change 200,000? Maybe a better solution exists.

>  while (true) {
>CoverageMapBase *aCoverageMap = NULL;
>struct trace_entry  entries[ENTRIES];

Can an array or resized vector be used here?

> @@ -118,8 +118,15 @@ namespace Coverage {
>  // Determine if additional branch information is available.
>  if ( (entry->op & branchInfo) != 0 ) {
>uint32_t  a = entry->pc + entry->size - 1;

An aside ... more pointers being used.

Chris

> -while (!aCoverageMap->isStartOfInstruction(a))
> +while (a > entry->pc && 
> + !aCoverageMap->isStartOfInstruction(a))
>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) {
>aCoverageMap->setWasTaken( a );
>  } else if (entry->op & notTaken) {
> 
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

RE: [PATCH] covoar: Handle periods in symbols from objdump

[Alex White]

The exceptions are being used as legitimate error cases. The "what" messages 
that I use here are not helpful. I think that's why it looks like they're being 
used as labels. 

As part of a revision of my "covoar: Fix NOP execution marking" patch that I'll 
be sending out soon, this has been greatly simplified and a specific exception 
class has been defined.

I'll get v2 of this patch out, and that should clear the exception handling up.

Alex

-Original Message-
From: Chris Johns  
Sent: Sunday, March 14, 2021 8:01 PM
To: Alex White ; devel@rtems.org
Subject: Re: [PATCH] covoar: Handle periods in symbols from objdump



On 13/3/21 3:37 am, Alex White wrote:
> Occasionally the compiler will generate symbols that look similar to 
> symbols defined in RTEMS code except that they contain some suffix.
> This looks to be related to compiler optimizations. Such symbols were 
> being treated as unique. For our purposes, they should be mapped to 
> the equivalent symbols in the DWARF info. This has been fixed.
> ---
>  tester/covoar/ExecutableInfo.cc   | 35 ++-
>  tester/covoar/ObjdumpProcessor.cc |  6 ++
>  tester/covoar/SymbolTable.cc  | 12 ---
>  3 files changed, 49 insertions(+), 4 deletions(-)
> 
> diff --git a/tester/covoar/ExecutableInfo.cc 
> b/tester/covoar/ExecutableInfo.cc index c4257f0..1396519 100644
> --- a/tester/covoar/ExecutableInfo.cc
> +++ b/tester/covoar/ExecutableInfo.cc
> @@ -119,6 +119,22 @@ namespace Coverage {
>  itsSymbol = theSymbolTable.getSymbol( address );
>  if (itsSymbol != "") {
>it = coverageMaps.find( itsSymbol );
> +  if (it == coverageMaps.end()) {
> +size_t periodIndex = itsSymbol.find(".");
> +
> +if (periodIndex == std::string::npos) {
> +  // Symbol name has no '.', can't do another lookup.
> +  throw rld::error (itsSymbol, "ExecutableInfo::getCoverageMap");
> +}
> +
> +it = coverageMaps.find(
> +  itsSymbol.substr(0, periodIndex)
> +);
> +
> +if (it == coverageMaps.end()) {
> +  throw rld::error (itsSymbol, "ExecutableInfo::getCoverageMap");
> +}
> +  }
>aCoverageMap = (*it).second;
>  }
>  
> @@ -150,8 +166,25 @@ namespace Coverage {
>)
>{
>  CoverageMaps::iterator cmi = coverageMaps.find( symbolName );
> -if ( cmi == coverageMaps.end() )
> +if ( cmi != coverageMaps.end() ) {
> +  return *(cmi->second);
> +}
> +
> +size_t periodIndex = symbolName.find(".");
> +
> +if (periodIndex == std::string::npos) {
> +  // Symbol name has no '.', can't do another lookup.
>throw rld::error (symbolName, 
> "ExecutableInfo::findCoverageMap");
> +}
> +
> +cmi = coverageMaps.find(
> +  symbolName.substr(0, periodIndex)
> +);
> +
> +if ( cmi == coverageMaps.end() ) {
> +  throw rld::error (symbolName, 
> + "ExecutableInfo::findCoverageMap");

The exception takes a what message and this looks like a label?

Is the exception being treated as signal to be caught in another place? I hope 
this is _not_ the case. Herb Sutter's write up is good place to start ...

https://www.drdobbs.com/when-and-how-to-use-exceptions/184401836

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

[Screenshot] Matt Joyce GSOC Hello World Patch

[Matthew Joyce]

Hello RTEMS Community,

Please find my attached screenshot for the GSOC Hello World introduction.
Thank you again for your time!

Sincerely,

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

[PATCH 1/1] Edited hello world message in init.c as prerequisite for GSOC application.

[Matt Joyce]

---
 testsuites/samples/hello/init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/testsuites/samples/hello/init.c b/testsuites/samples/hello/init.c
index 34ded37c55..48498fd52b 100644
--- a/testsuites/samples/hello/init.c
+++ b/testsuites/samples/hello/init.c
@@ -22,7 +22,7 @@ static rtems_task Init(
 {
   rtems_print_printer_fprintf_putc(_test_printer);
   TEST_BEGIN();
-  printf( "Hello World\n" );
+  printf("Hello RTEMS World! From Matt Joyce, GSOC 2021 Applicant.\n"); 
   TEST_END();
   rtems_test_exit( 0 );
 }
-- 
2.30.2

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

[PATCH 0/1] Matt Joyce GSOC Hello World Patch

[Matt Joyce]

Hello RTEMS Community! Please see my Hello World patch as per the
GSOC instructions. Thank you very much for your time and I'm 
excited to join you!

Sincerely,

Matt

Matt Joyce (1):
  Edited hello world message in init.c as prerequisite for GSOC
application.

 testsuites/samples/hello/init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.30.2

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

Re: #3860 - GSoC enquiries

[Gedare Bloom]

See the related thread, and we'll have to discuss how to move forward.
The existing approach provides an uncrustify script:
https://lists.rtems.org/pipermail/devel/2020-October/062769.html


On Sun, Mar 14, 2021 at 9:47 PM Ida Delphine  wrote:
>
> Hello everyone,
> This ticket(https://devel.rtems.org/ticket/3860) was proposed to me and I'm 
> interested in it for GSoC.
> The first task there is to find a code checker or formater that can produce 
> results that match the RTEMS coding conventions. It also made mention some 
> tools have been discussed in the past. Please I will love suggestions on 
> possible tools I could use to achieve this.
>
>
> Cheers,
> Ida.
> ___
> 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] rtems-examples: Add CoreMark Benchmark

[Gedare Bloom]

On Mon, Mar 15, 2021 at 6:02 AM Joel Sherrill  wrote:
>
>
>
> On Mon, Mar 15, 2021, 5:53 AM Hesham Almatary  
> wrote:
>>
>> Hello Gedare,
>>
>> Yeah adding Make support should be straightforward. I just assumed
>> Make will be deprecated soon based on [1] and haven't bothered
>> supporting it.
>>
>> [1] 
>> https://github.com/RTEMS/rtems-examples/blob/983926a7e519be85f630c620430e7e1ac3e0f4ea/README.Makefile#L32
>
>
> There are projects that use it. Although it is old and it would be nice to go 
> to something else, that something else isn't there. It's there.foenthe 
> foreseeable future.
>

Makefile will always be an option for users (application level) to
compile, and we should continue to provide/test that in our examples
repo. It would be best to show a variety of build systems in examples,
but limited bandwidth means we should at least show make and waf
approaches.

> --joel
>
>>
>>
>>
>>
>> On Sun, 14 Mar 2021 at 18:47, Gedare Bloom  wrote:
>> >
>> > Hi Hesham,
>> >
>> > Nice work getting this integrated in the upstream. I guess that the
>> > git submodule instructions for building with waf will work for this,
>> > but not Make. How hard would it be for you to integrate the submodule
>> > with Make?
>> >
>> > This is fine with me as-is, I just want to know if we can keep it
>> > supporting both build systems.
>> >
>> > On Sat, Mar 13, 2021 at 12:50 AM Hesham Almatary
>> >  wrote:
>> > >
>> > > CoreMark's primary goals are simplicity and providing a method for
>> > > testing only a processor's core features. It is used primarily here as
>> > > a performance benchmark.
>> > >
>> > > Built and tested for RISC-V rv64imafdc_medany on QEMU and HW
>> > > ---
>> > >  .gitmodules  |  3 +++
>> > >  benchmarks/coremark/coremark |  1 +
>> > >  benchmarks/coremark/wscript  | 50 
>> > >  benchmarks/wscript   |  2 +-
>> > >  4 files changed, 55 insertions(+), 1 deletion(-)
>> > >  create mode 16 benchmarks/coremark/coremark
>> > >  create mode 100644 benchmarks/coremark/wscript
>> > >
>> > > diff --git a/.gitmodules b/.gitmodules
>> > > index ae86e49..d7e52b9 100644
>> > > --- a/.gitmodules
>> > > +++ b/.gitmodules
>> > > @@ -1,3 +1,6 @@
>> > >  [submodule "rtems_waf"]
>> > > path = rtems_waf
>> > > url = git://git.rtems.org/rtems_waf.git
>> > > +[submodule "benchmarks/coremark/coremark"]
>> > > +   path = benchmarks/coremark/coremark
>> > > +   url = g...@github.com:eembc/coremark.git
>> > > diff --git a/benchmarks/coremark/coremark b/benchmarks/coremark/coremark
>> > > new file mode 16
>> > > index 000..1541482
>> > > --- /dev/null
>> > > +++ b/benchmarks/coremark/coremark
>> > > @@ -0,0 +1 @@
>> > > +Subproject commit 1541482bf3e6ef7f5c69f5be76b14537b60833d0
>> > > diff --git a/benchmarks/coremark/wscript b/benchmarks/coremark/wscript
>> > > new file mode 100644
>> > > index 000..2ec5f1e
>> > > --- /dev/null
>> > > +++ b/benchmarks/coremark/wscript
>> > > @@ -0,0 +1,50 @@
>> > > +#-
>> > > +# SPDX-License-Identifier: BSD-2-Clause
>> > > +#
>> > > +# Copyright (c) 2021 Hesham Almatary
>> > > +#
>> > > +# This software was developed by SRI International and the University of
>> > > +# Cambridge Computer Laboratory (Department of Computer Science and
>> > > +# Technology) under DARPA contract HR0011-18-C-0016 ("ECATS"), as part 
>> > > of the
>> > > +# DARPA SSITH research programme.
>> > > +#
>> > > +# Redistribution and use in source and binary forms, with or without
>> > > +# modification, are permitted provided that the following conditions
>> > > +# are met:
>> > > +# 1. Redistributions of source code must retain the above copyright
>> > > +#notice, this list of conditions and the following disclaimer.
>> > > +# 2. Redistributions in binary form must reproduce the above copyright
>> > > +#notice, this list of conditions and the following disclaimer in the
>> > > +#documentation and/or other materials provided with the 
>> > > distribution.
>> > > +#
>> > > +# THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
>> > > +# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
>> > > +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR 
>> > > PURPOSE
>> > > +# ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE 
>> > > LIABLE
>> > > +# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR 
>> > > CONSEQUENTIAL
>> > > +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE 
>> > > GOODS
>> > > +# OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
>> > > +# HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, 
>> > > STRICT
>> > > +# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY 
>> > > WAY
>> > > +# OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
>> > > +# SUCH DAMAGE.
>> > > +#
>> > > +
>> > > +import rtems_waf.rtems as rtems
>> > > +
>> 

Re: Linking error with powerpc/beatnik

[Gedare Bloom]

Hi Vijay,

This seems like it could be a problem in the linkcmds, possibly in
some missing KEEP() sections plus LTO? Check/compare the linkcmds
between 6, 5, and 4.10 you might get some ideas.

On Sun, Mar 14, 2021 at 9:27 PM Vijay Kumar Banerjee  wrote:
>
> Hello,
>
> I'm trying to build EPICS7 with RTEMS 6 and legacy-net stack but I'm getting 
> link errors (posted at the end of the email) from librtemscpu and 
> librtemsbsp. I'm not sure what's wrong but I think this might be related to 
> this ticket: https://devel.rtems.org/ticket/3698
>
> There's also an error with multiple definitions of __getreent and that looks 
> similar to a discussion that happened in 2015: 
> https://lists.rtems.org/pipermail/devel/2015-June/011587.html
>
> I would appreciate any help or suggestions on how to fix these issues.
>
>
> Best regards,
> Vijay
>
> ```
> /home/vijay/development/rtems/6/lib/gcc/powerpc-rtems6/10.2.1/../../../../powerpc-rtems6/bin/ld:
>  
> /home/vijay/development/kernel/rtems/build/powerpc/beatnik/../../../cpukit/libdl/rtl-mdreloc-powerpc.c:78:
>  undefined reference to `bsp_section_sdata_libdl_begin'
> /home/vijay/development/rtems/6/lib/gcc/powerpc-rtems6/10.2.1/../../../../powerpc-rtems6/bin/ld:
>  
> /home/vijay/development/kernel/rtems/build/powerpc/beatnik/../../../cpukit/libdl/rtl-mdreloc-powerpc.c:79:
>  undefined reference to `bsp_section_sdata_libdl_end'
> /home/vijay/development/rtems/6/lib/gcc/powerpc-rtems6/10.2.1/../../../../powerpc-rtems6/bin/ld:
>  
> /home/vijay/development/kernel/rtems/build/powerpc/beatnik/../../../cpukit/libdl/rtl-mdreloc-powerpc.c:79:
>  undefined reference to `bsp_section_sdata_libdl_end'
> /home/vijay/development/rtems/6/lib/gcc/powerpc-rtems6/10.2.1/../../../../powerpc-rtems6/bin/ld:
>  
> /home/vijay/development/rtems/6/powerpc-rtems6/beatnik/lib/librtemscpu.a(rtl-mdreloc-powerpc.c.63.o):
>  in function `get_sdata_sbss_size':
> /home/vijay/development/kernel/rtems/build/powerpc/beatnik/../../../cpukit/libdl/rtl-mdreloc-powerpc.c:68:
>  undefined reference to `bsp_section_sdata_begin'
> /home/vijay/development/rtems/6/lib/gcc/powerpc-rtems6/10.2.1/../../../../powerpc-rtems6/bin/ld:
>  
> /home/vijay/development/kernel/rtems/build/powerpc/beatnik/../../../cpukit/libdl/rtl-mdreloc-powerpc.c:68:
>  undefined reference to `bsp_section_sdata_begin'
> /home/vijay/development/rtems/6/lib/gcc/powerpc-rtems6/10.2.1/../../../../powerpc-rtems6/bin/ld:
>  
> /home/vijay/development/kernel/rtems/build/powerpc/beatnik/../../../cpukit/libdl/rtl-mdreloc-powerpc.c:69:
>  undefined reference to `bsp_section_sbss_end'
> /home/vijay/development/rtems/6/lib/gcc/powerpc-rtems6/10.2.1/../../../../powerpc-rtems6/bin/ld:
>  
> /home/vijay/development/kernel/rtems/build/powerpc/beatnik/../../../cpukit/libdl/rtl-mdreloc-powerpc.c:69:
>  undefined reference to `bsp_section_sbss_end'
> /home/vijay/development/rtems/6/lib/gcc/powerpc-rtems6/10.2.1/../../../../powerpc-rtems6/bin/ld:
>  
> /home/vijay/development/rtems/6/powerpc-rtems6/beatnik/lib/librtemscpu.a(rtems_putc.c.70.o):
>  in function `rtems_putc':
> /home/vijay/development/kernel/rtems/build/powerpc/beatnik/../../../cpukit/libcsupport/src/rtems_putc.c:29:
>  undefined reference to `BSP_output_char'
> /home/vijay/development/rtems/6/lib/gcc/powerpc-rtems6/10.2.1/../../../../powerpc-rtems6/bin/ld:
>  
> /home/vijay/development/kernel/rtems/build/powerpc/beatnik/../../../cpukit/libcsupport/src/rtems_putc.c:31:
>  undefined reference to `BSP_output_char'
> /home/vijay/development/rtems/6/lib/gcc/powerpc-rtems6/10.2.1/../../../../powerpc-rtems6/bin/ld:
>  
> /home/vijay/development/kernel/rtems/build/powerpc/beatnik/../../../cpukit/libcsupport/src/rtems_putc.c:34:
>  undefined reference to `BSP_output_char'
> /home/vijay/development/rtems/6/lib/gcc/powerpc-rtems6/10.2.1/../../../../powerpc-rtems6/bin/ld:
>  
> /home/vijay/development/rtems/6/powerpc-rtems6/beatnik/lib/librtemscpu.a(cachealignedalloc.c.70.o):
>  in function `rtems_cache_aligned_malloc':
> /home/vijay/development/kernel/rtems/build/powerpc/beatnik/../../../cpukit/libcsupport/src/cachealignedalloc.c:18:
>  undefined reference to `rtems_cache_get_maximal_line_size'
> /home/vijay/development/rtems/6/lib/gcc/powerpc-rtems6/10.2.1/../../../../powerpc-rtems6/bin/ld:
>  libComTestHarness: hidden symbol `__dso_handle' isn't defined
> /home/vijay/development/rtems/6/lib/gcc/powerpc-rtems6/10.2.1/../../../../powerpc-rtems6/bin/ld:
>  final link failed: bad value
>
> ```
> ___
> 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 3/5] nvdisk.c: Fix Resource leak (CID #1439297)

[Joel Sherrill]

On Mon, Mar 15, 2021, 3:10 PM Gedare Bloom  wrote:

> On Fri, Mar 12, 2021 at 8:18 AM Ryan Long  wrote:
> >
> > CID 1439297: Resource leak in rtems_nvdisk_initialize().
> >
> > Closes #4298
> > ---
> >  cpukit/libblock/src/nvdisk.c | 8 +++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/cpukit/libblock/src/nvdisk.c b/cpukit/libblock/src/nvdisk.c
> > index a7f4167..d742baf 100644
> > --- a/cpukit/libblock/src/nvdisk.c
> > +++ b/cpukit/libblock/src/nvdisk.c
> > @@ -766,8 +766,10 @@ rtems_nvdisk_initialize (rtems_device_major_number
> major RTEMS_UNUSED,
> >  nvd->info_level   = c->info_level;
> >
> >  nvd->devices = calloc (c->device_count, sizeof
> (rtems_nvdisk_device_ctl));
> > -if (!nvd->devices)
> > +if (!nvd->devices) {
> > +  free(nvd);
> >return RTEMS_NO_MEMORY;
> > +}
> >
> >  for (device = 0; device < c->device_count; device++)
> >  {
> > @@ -790,6 +792,8 @@ rtems_nvdisk_initialize (rtems_device_major_number
> major RTEMS_UNUSED,
> >   rtems_nvdisk_ioctl, nvd);
> >  if (sc != RTEMS_SUCCESSFUL)
> >  {
> > +  free(nvd->devices);
> > +  free(nvd);
> >rtems_nvdisk_error ("disk create phy failed");
> >return sc;
> >  }
> > @@ -797,5 +801,7 @@ rtems_nvdisk_initialize (rtems_device_major_number
> major RTEMS_UNUSED,
> >  rtems_mutex_init (>lock, "NV Disk");
> >}
> >
> > +  free(nvd->devices);
> > +  free(nvd);
>
> Does the rtems_blkdev_create() link the nvd? (I think the answer is
> yes.) Then, this is not correct.
>

I think this is my suggestion so what do you think should happen on this
case?

>
> >return RTEMS_SUCCESSFUL;
> >  }
> > --
> > 1.8.3.1
> >
> > ___
> > 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
>
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [PATCH 1/2] objectextendinformation.c: Fix Dereference after null check (CID #26033)

[Joel Sherrill]

On Mon, Mar 15, 2021, 3:21 PM Gedare Bloom  wrote:

> On Fri, Mar 12, 2021 at 7:55 AM Ryan Long  wrote:
> >
> > CID 26033: Dereference after null check in _Objects_Extend_information().
> >
> > Closes #4326
> > ---
> >  cpukit/score/src/objectextendinformation.c | 11 +++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/cpukit/score/src/objectextendinformation.c
> b/cpukit/score/src/objectextendinformation.c
> > index 9796eb9..c669301 100644
> > --- a/cpukit/score/src/objectextendinformation.c
> > +++ b/cpukit/score/src/objectextendinformation.c
> > @@ -171,6 +171,17 @@ Objects_Maximum _Objects_Extend_information(
> >
> >  if ( old_maximum > extend_count ) {
> >/*
> > +   * Coverity thinks there is a way for this to be NULL (CID
> #26033).
> > +   * After much time spent analyzing this, no one has identified the
> > +   * conditions where this can actually occur. Adding this _Assert
> ensures
> > +   * that it is never NULL. If this assert is triggered, condition
> > +   * generating this case will have been identified and it can be
> revisted.
> > +   * This is being done out of an abundance of caution since we
> could have
> > +   * easily flagged this as a false positive and ignored it
> completely.
> > +   */
> > +  _Assert(information->object_blocks != NULL);
> > +
> That's interesting. It would help if you could share your analysis.
>

This is the oldest Coverity issue that is open. It is over five years old.
Chris and I have tried multiple times to figure out if it is valid. We
never get any confidence that it cannot occur.


> How does
> 70  if ( information->object_blocks == NULL ) {
> be true, and if it is true, how does the exectuion flow proceed in
> such a way that it will not reach this assert?
>

No idea but it apparently doesn't based on our tests.

Adding the assert is an attempt to finally find the case that trips this.
It is either something I can never occur or something we don't know how to
make happen. Either way the asserting like a good idea.

if you have a test case in mind that can reproduce this coverity path,
let's try it and push this to failure. But we have no evidence that it's
ever occurred in the field.

>
>
> > +  /*
> > *  Copy each section of the table over. This has to be performed
> as
> > *  separate parts as size of each block has changed.
> > */
> > --
> > 1.8.3.1
> >
> > ___
> > 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
>
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [PATCH] covoar: Fix null pointer dereference

[Gedare Bloom]

ok -- imagine that, cleaning up resources in a destructor ;)

On Fri, Mar 12, 2021 at 10:06 AM Alex White  wrote:
>
> A null pointer dereference happens later in the program execution if
> the files are cleaned up at the end of the ExecutableInfo constructor.
> This change fixes the null pointer dereference.
> ---
>  tester/covoar/ExecutableInfo.cc | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/tester/covoar/ExecutableInfo.cc b/tester/covoar/ExecutableInfo.cc
> index 1396519..187bb77 100644
> --- a/tester/covoar/ExecutableInfo.cc
> +++ b/tester/covoar/ExecutableInfo.cc
> @@ -82,13 +82,16 @@ namespace Coverage {
>throw;
>  }
>
> -debug.end();
> -executable.end();
> -executable.close();
> +// Can't cleanup handles until the destructor because the information is
> +// referenced elsewhere. NOTE: This could cause problems from too many 
> open
> +// file descriptors.
>}
>
>ExecutableInfo::~ExecutableInfo()
>{
> +debug.end();
> +executable.end();
> +executable.close();
>}
>
>void ExecutableInfo::dumpCoverageMaps( void ) {
> --
> 2.27.0
>
> ___
> 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] score: Add Thread_Configuration::cpu_time_budget

[Gedare Bloom]

looks good

On Fri, Mar 12, 2021 at 12:34 AM Sebastian Huber
 wrote:
>
> Move the CPU time budget to the thread configuration.  This simplifies
> _Thread_Initialize().
> ---
>  cpukit/include/rtems/posix/pthreadimpl.h | 26 +++---
>  cpukit/include/rtems/score/threadimpl.h  |  5 +++
>  cpukit/posix/src/psxtransschedparam.c| 23 +++--
>  cpukit/posix/src/pthreadcreate.c |  3 +-
>  cpukit/posix/src/pthreadsetschedparam.c  | 44 +---
>  cpukit/score/src/threadinitialize.c  | 18 +-
>  6 files changed, 49 insertions(+), 70 deletions(-)
>
> diff --git a/cpukit/include/rtems/posix/pthreadimpl.h 
> b/cpukit/include/rtems/posix/pthreadimpl.h
> index 52d462ab6f..723b20e8d2 100644
> --- a/cpukit/include/rtems/posix/pthreadimpl.h
> +++ b/cpukit/include/rtems/posix/pthreadimpl.h
> @@ -77,24 +77,24 @@ int _POSIX_Thread_Translate_to_sched_policy(
>  );
>
>  /**
> - * @brief Translate sched_param into SuperCore terms.
> + * @brief Translates the POSIX scheduling policy and parameters to parts of 
> the
> + *   thread configuration.
>   *
> - * This method translates the POSIX API sched_param into the corresponding
> - * SuperCore settings.
> + * @param policy is the POSIX scheduling policy.
>   *
> - * @param[in] policy is the POSIX scheduling policy
> - * @param[in] param points to the scheduling parameter structure
> - * @param[in] budget_algorithm points to the output CPU Budget algorithm
> - * @param[in] budget_callout points to the output CPU Callout
> + * @param param is the pointer to the POSIX scheduling parameters.
>   *
> - * @retval 0 Indicates success.
> - * @retval error_code POSIX error code indicating failure.
> + * @param[out] config is the pointer to a thread configuration to set the
> + *   budget algorithm, callout, and CPU time budget.
> + *
> + * @retval 0 The operation was successful.
> + *
> + * @retval EINVAL The POSIX scheduling policy or parameters were invalid.
>   */
>  int _POSIX_Thread_Translate_sched_param(
> -  int  policy,
> -  const struct sched_param*param,
> -  Thread_CPU_budget_algorithms*budget_algorithm,
> -  Thread_CPU_budget_algorithm_callout *budget_callout
> +  int   policy,
> +  const struct sched_param *param,
> +  Thread_Configuration *config
>  );
>
>  RTEMS_INLINE_ROUTINE Thread_Control *_POSIX_Threads_Allocate(void)
> diff --git a/cpukit/include/rtems/score/threadimpl.h 
> b/cpukit/include/rtems/score/threadimpl.h
> index 1e7d58609f..d9c0779b08 100644
> --- a/cpukit/include/rtems/score/threadimpl.h
> +++ b/cpukit/include/rtems/score/threadimpl.h
> @@ -163,6 +163,11 @@ typedef struct {
> */
>Thread_CPU_budget_algorithm_callout budget_callout;
>
> +  /**
> +   * @brief The thread's initial CPU time budget.
> +   */
> +  uint32_t cpu_time_budget;
> +
>/**
> * @brief 32-bit unsigned integer name of the object for the thread.
> */
> diff --git a/cpukit/posix/src/psxtransschedparam.c 
> b/cpukit/posix/src/psxtransschedparam.c
> index 6fa7a43886..eba26d4932 100644
> --- a/cpukit/posix/src/psxtransschedparam.c
> +++ b/cpukit/posix/src/psxtransschedparam.c
> @@ -42,27 +42,28 @@ int _POSIX_Thread_Translate_to_sched_policy(
>  }
>
>  int _POSIX_Thread_Translate_sched_param(
> -  int  policy,
> -  const struct sched_param*param,
> -  Thread_CPU_budget_algorithms*budget_algorithm,
> -  Thread_CPU_budget_algorithm_callout *budget_callout
> +  int   policy,
> +  const struct sched_param *param,
> +  Thread_Configuration *config
>  )
>  {
> -  *budget_algorithm = THREAD_CPU_BUDGET_ALGORITHM_NONE;
> -  *budget_callout = NULL;
> +  config->budget_algorithm = THREAD_CPU_BUDGET_ALGORITHM_NONE;
> +  config->budget_callout = NULL;
> +  config->cpu_time_budget = 0;
>
>if ( policy == SCHED_OTHER ) {
> -*budget_algorithm = THREAD_CPU_BUDGET_ALGORITHM_RESET_TIMESLICE;
> +config->budget_algorithm = THREAD_CPU_BUDGET_ALGORITHM_RESET_TIMESLICE;
>  return 0;
>}
>
>if ( policy == SCHED_FIFO ) {
> -*budget_algorithm = THREAD_CPU_BUDGET_ALGORITHM_NONE;
> +config->budget_algorithm = THREAD_CPU_BUDGET_ALGORITHM_NONE;
>  return 0;
>}
>
>if ( policy == SCHED_RR ) {
> -*budget_algorithm = THREAD_CPU_BUDGET_ALGORITHM_EXHAUST_TIMESLICE;
> +config->budget_algorithm = THREAD_CPU_BUDGET_ALGORITHM_EXHAUST_TIMESLICE;
> +config->cpu_time_budget = rtems_configuration_get_ticks_per_timeslice();
>  return 0;
>}
>
> @@ -80,8 +81,8 @@ int _POSIX_Thread_Translate_sched_param(
>  _Timespec_To_ticks( >sched_ss_init_budget ) )
>return EINVAL;
>
> -*budget_algorithm  = THREAD_CPU_BUDGET_ALGORITHM_CALLOUT;
> -*budget_callout = _POSIX_Threads_Sporadic_budget_callout;
> +config->budget_algorithm  = THREAD_CPU_BUDGET_ALGORITHM_CALLOUT;
> +config->budget_callout = _POSIX_Threads_Sporadic_budget_callout;
>  

Re: [PATCH 1/2] objectextendinformation.c: Fix Dereference after null check (CID #26033)

[Gedare Bloom]

On Fri, Mar 12, 2021 at 7:55 AM Ryan Long  wrote:
>
> CID 26033: Dereference after null check in _Objects_Extend_information().
>
> Closes #4326
> ---
>  cpukit/score/src/objectextendinformation.c | 11 +++
>  1 file changed, 11 insertions(+)
>
> diff --git a/cpukit/score/src/objectextendinformation.c 
> b/cpukit/score/src/objectextendinformation.c
> index 9796eb9..c669301 100644
> --- a/cpukit/score/src/objectextendinformation.c
> +++ b/cpukit/score/src/objectextendinformation.c
> @@ -171,6 +171,17 @@ Objects_Maximum _Objects_Extend_information(
>
>  if ( old_maximum > extend_count ) {
>/*
> +   * Coverity thinks there is a way for this to be NULL (CID #26033).
> +   * After much time spent analyzing this, no one has identified the
> +   * conditions where this can actually occur. Adding this _Assert 
> ensures
> +   * that it is never NULL. If this assert is triggered, condition
> +   * generating this case will have been identified and it can be 
> revisted.
> +   * This is being done out of an abundance of caution since we could 
> have
> +   * easily flagged this as a false positive and ignored it completely.
> +   */
> +  _Assert(information->object_blocks != NULL);
> +
That's interesting. It would help if you could share your analysis.

How does
70  if ( information->object_blocks == NULL ) {
be true, and if it is true, how does the exectuion flow proceed in
such a way that it will not reach this assert?


> +  /*
> *  Copy each section of the table over. This has to be performed as
> *  separate parts as size of each block has changed.
> */
> --
> 1.8.3.1
>
> ___
> 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 3/5] nvdisk.c: Fix Resource leak (CID #1439297)

[Gedare Bloom]

On Fri, Mar 12, 2021 at 8:18 AM Ryan Long  wrote:
>
> CID 1439297: Resource leak in rtems_nvdisk_initialize().
>
> Closes #4298
> ---
>  cpukit/libblock/src/nvdisk.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/cpukit/libblock/src/nvdisk.c b/cpukit/libblock/src/nvdisk.c
> index a7f4167..d742baf 100644
> --- a/cpukit/libblock/src/nvdisk.c
> +++ b/cpukit/libblock/src/nvdisk.c
> @@ -766,8 +766,10 @@ rtems_nvdisk_initialize (rtems_device_major_number major 
> RTEMS_UNUSED,
>  nvd->info_level   = c->info_level;
>
>  nvd->devices = calloc (c->device_count, sizeof 
> (rtems_nvdisk_device_ctl));
> -if (!nvd->devices)
> +if (!nvd->devices) {
> +  free(nvd);
>return RTEMS_NO_MEMORY;
> +}
>
>  for (device = 0; device < c->device_count; device++)
>  {
> @@ -790,6 +792,8 @@ rtems_nvdisk_initialize (rtems_device_major_number major 
> RTEMS_UNUSED,
>   rtems_nvdisk_ioctl, nvd);
>  if (sc != RTEMS_SUCCESSFUL)
>  {
> +  free(nvd->devices);
> +  free(nvd);
>rtems_nvdisk_error ("disk create phy failed");
>return sc;
>  }
> @@ -797,5 +801,7 @@ rtems_nvdisk_initialize (rtems_device_major_number major 
> RTEMS_UNUSED,
>  rtems_mutex_init (>lock, "NV Disk");
>}
>
> +  free(nvd->devices);
> +  free(nvd);

Does the rtems_blkdev_create() link the nvd? (I think the answer is
yes.) Then, this is not correct.

>return RTEMS_SUCCESSFUL;
>  }
> --
> 1.8.3.1
>
> ___
> 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 2/5] rtems-fdt.c: Fix Resource leak (CID #1437645)

[Gedare Bloom]

This one looks ok to me, Niteesh?

On Fri, Mar 12, 2021 at 8:19 AM Ryan Long  wrote:
>
> CID 1437645: Resource leak in rtems_fdt_load().
>
> Closes #4297
> ---
>  cpukit/libmisc/rtems-fdt/rtems-fdt.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/cpukit/libmisc/rtems-fdt/rtems-fdt.c 
> b/cpukit/libmisc/rtems-fdt/rtems-fdt.c
> index 0ea3653..5bb7ce0 100644
> --- a/cpukit/libmisc/rtems-fdt/rtems-fdt.c
> +++ b/cpukit/libmisc/rtems-fdt/rtems-fdt.c
> @@ -611,6 +611,7 @@ rtems_fdt_load (const char* filename, rtems_fdt_handle* 
> handle)
>  return fe;
>}
>
> +  close (bf);
>return 0;
>  }
>
> --
> 1.8.3.1
>
> ___
> 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 0/4] Import and Port Beagle pinmux driver

[Gedare Bloom]

Christian,

Can you please provide comments on this patch series and the TI pinmux
removal from libbsd?

thanks

On Sat, Mar 13, 2021 at 4:02 AM G S Niteesh Babu  wrote:
>
> The following series of patches import and port the beagle
> pinmux driver from FreeBSD to RTEMS.
>
> Porting this driver will avoid double initialization of
> pin multiplexers once during RTEMS initialization and second
> time during libBSD initialization.
>
> UPDATE #3782
>
> G S Niteesh Babu (4):
>   bsps/shared/ofw: Add rtems_ofw_is_node_compatible
>   bsp/beagle: Import Beagle pinmux from FreeBSD
>   bsps/beagle: Added SOC detection using FDT
>   bsp/beagle: Ported Beagle pinmux driver to RTEMS
>
>  bsps/arm/beagle/start/bsp-soc-detect.c|  55 ++
>  bsps/arm/beagle/start/bsp-soc-detect.h|  38 ++
>  bsps/arm/beagle/start/bspstart.c  |  54 +-
>  .../arm/ti/am335x/am335x_scm_padconf.h|  47 ++
>  bsps/include/arm/ti/ti_cpuid.h|  48 ++
>  bsps/include/arm/ti/ti_pinmux.h   |  87 +++
>  bsps/include/ofw/ofw.h|  17 +
>  .../sys/arm/ti/am335x/am335x_scm_padconf.c| 307 ++
>  bsps/shared/freebsd/sys/arm/ti/ti_pinmux.c| 574 ++
>  bsps/shared/ofw/ofw.c |  12 +
>  spec/build/bsps/arm/beagle/obj.yml|   1 +
>  spec/build/bsps/obj.yml   |   6 +
>  12 files changed, 1240 insertions(+), 6 deletions(-)
>  create mode 100644 bsps/arm/beagle/start/bsp-soc-detect.c
>  create mode 100644 bsps/arm/beagle/start/bsp-soc-detect.h
>  create mode 100644 bsps/include/arm/ti/am335x/am335x_scm_padconf.h
>  create mode 100644 bsps/include/arm/ti/ti_cpuid.h
>  create mode 100644 bsps/include/arm/ti/ti_pinmux.h
>  create mode 100644 bsps/shared/freebsd/sys/arm/ti/am335x/am335x_scm_padconf.c
>  create mode 100644 bsps/shared/freebsd/sys/arm/ti/ti_pinmux.c
>
> --
> 2.17.1
>
> ___
> 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 v2] shell.c: Fix Dereference after null check (CID #26083)

[Gedare Bloom]

On Sun, Mar 14, 2021 at 8:27 PM Chris Johns  wrote:
>
> Sorry, this patch is blocked by the v1 thread.
>
+1

And why I asked about the "use cases" for this shell login. I guess
those could be documented somewhere and some examples shown.

> Chris
>
> On 12/3/21 9:17 am, Ryan Long wrote:
> > CID 26083: Dereference after null check in rtems_shell_login().
> >
> > Closes #4327
> > ---
> >  cpukit/libmisc/shell/shell.c | 5 +
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/cpukit/libmisc/shell/shell.c b/cpukit/libmisc/shell/shell.c
> > index 1e5962b..ad47460 100644
> > --- a/cpukit/libmisc/shell/shell.c
> > +++ b/cpukit/libmisc/shell/shell.c
> > @@ -789,6 +789,8 @@ static bool rtems_shell_login(rtems_shell_env_t *env, 
> > FILE * in,FILE * out)
> >  }
> >}
> >
> > +  _Assert(out != NULL);
> > +
> >return rtems_shell_login_prompt(in, out, env->devname, env->login_check);
> >  }
> >
> > @@ -966,6 +968,9 @@ bool rtems_shell_main_loop(
> >  "shell: cannot allocate prompt memory\n");
> >}
> >
> > +  _Assert(stdin != NULL);
> > +  _Assert(stdout != NULL);
> > +
> >shell_std_debug("child out: %d (%p)\n", fileno(stdout), stdout);
> >shell_std_debug("child  in: %d (%p)\n", fileno(stdin), stdin);
> >
> >
> ___
> 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 6/6] rtems-docs: Edit commands to build first app

[Gedare Bloom]

Hi Ida,

On Mon, Mar 15, 2021 at 1:50 PM Gedare Bloom  wrote:
>
> Hi Ida,
>
> did you send the same set of patches twice, or are there changes
> between the two sets?
>

Since I'm not quite sure what to look at here, please do me a favor
and send a new patchset with a -v2 indicator [1], and also set the
patch message to specify rtems-docs. git format patch supports to set
the prefix with:
 git format-patch HEAD^ --subject-prefix="PATCH rtems-docs"
You can also create an alias to make this simpler [2]. In fact, if you
can send a patch to add instructions how to set up patch message with
the repo name to the text at [1] that would be another nice
contribution :)

[1] https://docs.rtems.org/branches/master/eng/vc-users.html#creating-a-patch
[2] https://lists.rtems.org/pipermail/devel/2020-April/059308.html

Thank you,
Gedare

> On Sun, Mar 14, 2021 at 8:26 PM Ida Delphine  wrote:
> >
> > ---
> >  user/start/app.rst | 104 ++---
> >  1 file changed, 52 insertions(+), 52 deletions(-)
> >
> > diff --git a/user/start/app.rst b/user/start/app.rst
> > index 8900f78..c343551 100644
> > --- a/user/start/app.rst
> > +++ b/user/start/app.rst
> > @@ -8,7 +8,7 @@ Build Your Application
> >  ==
> >
> >  You tested a BSP in the previous section.  We built the ``erc32`` BSP
> > -and it is installed under :file:`$HOME/quick-start/rtems/5`.
> > +and it is installed under :file:`$HOME/quick-start/rtems/6`.
> >
> >  We will now create a simple Hello World application with a Git
> >  repository and using the `Waf `_ build system.
> > @@ -107,7 +107,7 @@ and copy the Waf script:
> >  #
> >  from __future__ import print_function
> >
> > -rtems_version = "5"
> > +rtems_version = "6"
> >
> >  try:
> >  import rtems_waf.rtems as rtems
> > @@ -142,52 +142,52 @@ Configure the application using Waf's ``configure`` 
> > command:
> >
> >  .. code-block:: none
> >
> > -./waf configure --rtems=$HOME/quick-start/rtems/5 
> > --rtems-bsp=sparc/erc32
> > +./waf configure --rtems=$HOME/quick-start/rtems/6 
> > --rtems-bsp=sparc/erc32 --rtems-version=6
> >
> >  The output will be something close to:
> >
> >  .. code-block:: none
> >
> > - Setting top to   : $BASE/app/hello
> > - Setting out to   : $BASE/app/hello/build
> > - RTEMS Version: 5
> > - Architectures: sparc-rtems5
> > - Board Support Package (BSP)  : sparc-rtems5-erc32
> > - Show commands: no
> > - Long commands: no
> > - Checking for program 'sparc-rtems5-gcc'  : 
> > $BASE/rtems/5/bin/sparc-rtems5-gcc
> > - Checking for program 'sparc-rtems5-g++'  : 
> > $BASE/rtems/5/bin/sparc-rtems5-g++
> > - Checking for program 'sparc-rtems5-gcc'  : 
> > $BASE/rtems/5/bin/sparc-rtems5-gcc
> > - Checking for program 'sparc-rtems5-ld'   : 
> > $BASE/rtems/5/bin/sparc-rtems5-ld
> > - Checking for program 'sparc-rtems5-ar'   : 
> > $BASE/rtems/5/bin/sparc-rtems5-ar
> > - Checking for program 'sparc-rtems5-nm'   : 
> > $BASE/rtems/5/bin/sparc-rtems5-nm
> > - Checking for program 'sparc-rtems5-objdump' : 
> > $BASE/rtems/5/bin/sparc-rtems5-objdump
> > - Checking for program 'sparc-rtems5-objcopy' : 
> > $BASE/rtems/5/bin/sparc-rtems5-objcopy
> > - Checking for program 'sparc-rtems5-readelf' : 
> > $BASE/rtems/5/bin/sparc-rtems5-readelf
> > - Checking for program 'sparc-rtems5-strip'   : 
> > $BASE/rtems/5/bin/sparc-rtems5-strip
> > - Checking for program 'sparc-rtems5-ranlib'  : 
> > $BASE/rtems/5/bin/sparc-rtems5-ranlib
> > - Checking for program 'rtems-ld' : 
> > $BASE/rtems/5/bin/rtems-ld
> > - Checking for program 'rtems-tld': 
> > $BASE/rtems/5/bin/rtems-tld
> > - Checking for program 'rtems-syms'   : 
> > $BASE/rtems/5/bin/rtems-syms
> > - Checking for program 'rtems-bin2c'  : 
> > $BASE/rtems/5/bin/rtems-bin2c
> > - Checking for program 'tar'  : /usr/bin/tar
> > - Checking for program 'gcc, cc'  : 
> > $BASE/rtems/5/bin/sparc-rtems5-gcc
> > - Checking for program 'ar'   : 
> > $BASE/rtems/5/bin/sparc-rtems5-ar
> > - Checking for program 'g++, c++' : 
> > $BASE/rtems/5/bin/sparc-rtems5-g++
> > - Checking for program 'ar'   : 
> > $BASE/rtems/5/bin/sparc-rtems5-ar
> > - Checking for program 'gas, gcc' : 
> > $BASE/rtems/5/bin/sparc-rtems5-gcc
> > - Checking for program 'ar'   : 
> > $BASE/rtems/5/bin/sparc-rtems5-ar
> > - Checking for c flags '-MMD' : yes
> > - Checking for cxx flags '-MMD'   : yes
> > - Compiler version (sparc-rtems5-gcc) : 7.5.0 20191114 (RTEMS 
> > 5, RSB 5.1.0, Newlib fbaa096)
> > - Checking 

Re: [PATCH 1/5] hexdump-parse.c: Fix Resource leak (CID #26032)

[Joel Sherrill]

On Mon, Mar 15, 2021 at 2:46 PM Gedare Bloom  wrote:

> On Sun, Mar 14, 2021 at 8:27 PM Chris Johns  wrote:
> >
> > On 13/3/21 2:18 am, Ryan Long wrote:
> > > CID 26032: Resource leak in rtems_shell_hexdump_rewrite().
> > >
> > > Closes #4296
> > > ---
> > >  cpukit/libmisc/shell/hexdump-parse.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/cpukit/libmisc/shell/hexdump-parse.c
> b/cpukit/libmisc/shell/hexdump-parse.c
> > > index 88b9d56..5b56bbf 100644
> > > --- a/cpukit/libmisc/shell/hexdump-parse.c
> > > +++ b/cpukit/libmisc/shell/hexdump-parse.c
> > > @@ -462,6 +462,9 @@ isint2:
>  switch(fu->bcnt) {
> > >   (void)printf("\n");
> > >   }
> > >  #endif
> > > +#ifdef __rtems__
> > > + free(nextpr);
> > > +#endif
> >
> > I know this has not been done in imported code in rtems.git before but
> should we
> > adopt the libbsd standard of adding /* __rtems__ */ to the #else and
> #endif?
> >
>
> Probably, but we also clone-and-own this shell/ code, and we should
> not bother with these #ifdefs in there. I think I have said this 3
> times this past week about shell/. The upstream does not want our
> changes, and we don't import from them anymore.
>

Some of these files have massive changes and some don't. Ryan and
I looked at main_cp.c and it had at least 40 revisions since the version
we have. The same Coverity issue appeared to be present but the variable
names were changed and much clearer.

Chris imported this code initially. I'll trust his ruling on this since I
assume
he is likely to either be the one to update it eventually or have to help
someone
a lot. :)

And some of it is close enough that the ifdef's are worth it. Some isn't.
Hard call on the overall value.

--joel


> > Chris
> > ___
> > 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
>
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [PATCH 6/6] rtems-docs: Edit commands to build first app

[Gedare Bloom]

Hi Ida,

did you send the same set of patches twice, or are there changes
between the two sets?

On Sun, Mar 14, 2021 at 8:26 PM Ida Delphine  wrote:
>
> ---
>  user/start/app.rst | 104 ++---
>  1 file changed, 52 insertions(+), 52 deletions(-)
>
> diff --git a/user/start/app.rst b/user/start/app.rst
> index 8900f78..c343551 100644
> --- a/user/start/app.rst
> +++ b/user/start/app.rst
> @@ -8,7 +8,7 @@ Build Your Application
>  ==
>
>  You tested a BSP in the previous section.  We built the ``erc32`` BSP
> -and it is installed under :file:`$HOME/quick-start/rtems/5`.
> +and it is installed under :file:`$HOME/quick-start/rtems/6`.
>
>  We will now create a simple Hello World application with a Git
>  repository and using the `Waf `_ build system.
> @@ -107,7 +107,7 @@ and copy the Waf script:
>  #
>  from __future__ import print_function
>
> -rtems_version = "5"
> +rtems_version = "6"
>
>  try:
>  import rtems_waf.rtems as rtems
> @@ -142,52 +142,52 @@ Configure the application using Waf's ``configure`` 
> command:
>
>  .. code-block:: none
>
> -./waf configure --rtems=$HOME/quick-start/rtems/5 --rtems-bsp=sparc/erc32
> +./waf configure --rtems=$HOME/quick-start/rtems/6 
> --rtems-bsp=sparc/erc32 --rtems-version=6
>
>  The output will be something close to:
>
>  .. code-block:: none
>
> - Setting top to   : $BASE/app/hello
> - Setting out to   : $BASE/app/hello/build
> - RTEMS Version: 5
> - Architectures: sparc-rtems5
> - Board Support Package (BSP)  : sparc-rtems5-erc32
> - Show commands: no
> - Long commands: no
> - Checking for program 'sparc-rtems5-gcc'  : 
> $BASE/rtems/5/bin/sparc-rtems5-gcc
> - Checking for program 'sparc-rtems5-g++'  : 
> $BASE/rtems/5/bin/sparc-rtems5-g++
> - Checking for program 'sparc-rtems5-gcc'  : 
> $BASE/rtems/5/bin/sparc-rtems5-gcc
> - Checking for program 'sparc-rtems5-ld'   : 
> $BASE/rtems/5/bin/sparc-rtems5-ld
> - Checking for program 'sparc-rtems5-ar'   : 
> $BASE/rtems/5/bin/sparc-rtems5-ar
> - Checking for program 'sparc-rtems5-nm'   : 
> $BASE/rtems/5/bin/sparc-rtems5-nm
> - Checking for program 'sparc-rtems5-objdump' : 
> $BASE/rtems/5/bin/sparc-rtems5-objdump
> - Checking for program 'sparc-rtems5-objcopy' : 
> $BASE/rtems/5/bin/sparc-rtems5-objcopy
> - Checking for program 'sparc-rtems5-readelf' : 
> $BASE/rtems/5/bin/sparc-rtems5-readelf
> - Checking for program 'sparc-rtems5-strip'   : 
> $BASE/rtems/5/bin/sparc-rtems5-strip
> - Checking for program 'sparc-rtems5-ranlib'  : 
> $BASE/rtems/5/bin/sparc-rtems5-ranlib
> - Checking for program 'rtems-ld' : $BASE/rtems/5/bin/rtems-ld
> - Checking for program 'rtems-tld': 
> $BASE/rtems/5/bin/rtems-tld
> - Checking for program 'rtems-syms'   : 
> $BASE/rtems/5/bin/rtems-syms
> - Checking for program 'rtems-bin2c'  : 
> $BASE/rtems/5/bin/rtems-bin2c
> - Checking for program 'tar'  : /usr/bin/tar
> - Checking for program 'gcc, cc'  : 
> $BASE/rtems/5/bin/sparc-rtems5-gcc
> - Checking for program 'ar'   : 
> $BASE/rtems/5/bin/sparc-rtems5-ar
> - Checking for program 'g++, c++' : 
> $BASE/rtems/5/bin/sparc-rtems5-g++
> - Checking for program 'ar'   : 
> $BASE/rtems/5/bin/sparc-rtems5-ar
> - Checking for program 'gas, gcc' : 
> $BASE/rtems/5/bin/sparc-rtems5-gcc
> - Checking for program 'ar'   : 
> $BASE/rtems/5/bin/sparc-rtems5-ar
> - Checking for c flags '-MMD' : yes
> - Checking for cxx flags '-MMD'   : yes
> - Compiler version (sparc-rtems5-gcc) : 7.5.0 20191114 (RTEMS 5, 
> RSB 5.1.0, Newlib fbaa096)
> - Checking for a valid RTEMS BSP installation : yes
> - Checking for RTEMS_DEBUG: no
> - Checking for RTEMS_MULTIPROCESSING  : no
> - Checking for RTEMS_NEWLIB   : yes
> - Checking for RTEMS_POSIX_API: yes
> - Checking for RTEMS_SMP  : no
> - Checking for RTEMS_NETWORKING   : no
> - 'configure' finished successfully (0.686s)
> +Setting top to   : $BASE/app/hello
> +Setting out to   : $BASE/app/hello/build
> +RTEMS Version: 6
> +Architectures: sparc-rtems6
> +Board Support Package (BSP)  : sparc-rtems6-erc32
> +Show commands: no
> +Long commands: no
> +Checking for program 'sparc-rtems6-gcc'  : 
> 

Re: [PATCH 5/5] rtl-shell.c: Resource leak (CID #1444140)

[Gedare Bloom]

On Sun, Mar 14, 2021 at 8:27 PM Chris Johns  wrote:
>
> On 13/3/21 2:18 am, Ryan Long wrote:
> > CID 1444140: Resource leak in rtems_rtl_shell_object().
> >
> > Closes #4300
> > ---
> >  cpukit/libdl/rtl-shell.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/cpukit/libdl/rtl-shell.c b/cpukit/libdl/rtl-shell.c
> > index 9f8a136..bcecdd4 100644
> > --- a/cpukit/libdl/rtl-shell.c
> > +++ b/cpukit/libdl/rtl-shell.c
> > @@ -733,14 +733,17 @@ rtems_rtl_shell_object (const rtems_printer* printer, 
> > int argc, char* argv[])
> >  if (dlinfo (RTLD_SELF, RTLD_DI_UNRESOLVED, ) < 0)
> >  {
> >rtems_printf (printer, "error: %s: %s\n", argv[arg], dlerror ());
> > +  (void) dlclose (handle);
> >return 1;
> >  }
> >
> >  if (unresolved != 0)
> >  {
> >rtems_printf (printer, "warning: unresolved symbols present\n");
> > +  (void) dlclose (handle);
> >return 1;
> >  }
> > +  (void) dlclose (handle);
> >}
> >else if (strcmp (argv[arg], "unload") == 0)
> >{
> >
>
> The handle should be not closed in any of these cases. Have you run the 
> command
> after making these changes and played with command?
>
> This shell command is a tool to load and play with loaded modules and one
> command is to load a module and others let you play with it. The handle 
> address
> is printed on the console. Coverity may like to track and nag you about things
> like this but it is not always right.
>
> In relation to all these Coverity changes ... are all these changes being
> tested? Is the boarder context of the change being examined in relation to the
> specific change?
>

I think this is an important, and challenging, question, since some
things are not fully tested. In this case, we should identify/create
tests to exercise the undefined behavior first, and then commit a
change after testing. As the easy coverity issues are being fixed, now
we need to be more meticulous since we can't just "think through" the
modifications being proposed, such as this one (and the NULL out in
shell calls) without understanding the scope of possible invocations.
For example, maybe we need a test case for telnet shells, before we
make a change that impacts their potential useability.

> Chris
> ___
> 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 1/5] hexdump-parse.c: Fix Resource leak (CID #26032)

[Gedare Bloom]

On Sun, Mar 14, 2021 at 8:27 PM Chris Johns  wrote:
>
> On 13/3/21 2:18 am, Ryan Long wrote:
> > CID 26032: Resource leak in rtems_shell_hexdump_rewrite().
> >
> > Closes #4296
> > ---
> >  cpukit/libmisc/shell/hexdump-parse.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/cpukit/libmisc/shell/hexdump-parse.c 
> > b/cpukit/libmisc/shell/hexdump-parse.c
> > index 88b9d56..5b56bbf 100644
> > --- a/cpukit/libmisc/shell/hexdump-parse.c
> > +++ b/cpukit/libmisc/shell/hexdump-parse.c
> > @@ -462,6 +462,9 @@ isint2:   
> > switch(fu->bcnt) {
> >   (void)printf("\n");
> >   }
> >  #endif
> > +#ifdef __rtems__
> > + free(nextpr);
> > +#endif
>
> I know this has not been done in imported code in rtems.git before but should 
> we
> adopt the libbsd standard of adding /* __rtems__ */ to the #else and #endif?
>

Probably, but we also clone-and-own this shell/ code, and we should
not bother with these #ifdefs in there. I think I have said this 3
times this past week about shell/. The upstream does not want our
changes, and we don't import from them anymore.

> Chris
> ___
> 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

[PATCH v4 1/2] covoar: Fix NOP execution marking

[Alex White]

Some NOP instructions were not being marked as executed because they
are located at the end of uncovered ranges. This has been fixed.
---
 tester/covoar/CoverageMapBase.cc  |  10 +++
 tester/covoar/CoverageMapBase.h   |   4 ++
 tester/covoar/DesiredSymbols.cc   |  38 +--
 tester/covoar/DesiredSymbols.h|  11 ++-
 tester/covoar/ExecutableInfo.cc   |   2 +-
 tester/covoar/ExecutableInfo.h|   4 ++
 tester/covoar/ObjdumpProcessor.cc | 109 +++---
 7 files changed, 142 insertions(+), 36 deletions(-)

diff --git a/tester/covoar/CoverageMapBase.cc b/tester/covoar/CoverageMapBase.cc
index ad0080d..6ca5cf7 100644
--- a/tester/covoar/CoverageMapBase.cc
+++ b/tester/covoar/CoverageMapBase.cc
@@ -142,6 +142,11 @@ namespace Coverage {
 return size;
   }
 
+  uint32_t CoverageMapBase::getSizeOfRange( size_t index ) const
+  {
+return Ranges.at(index).size();
+  }
+
   bool CoverageMapBase::getBeginningOfInstruction(
 uint32_t  address,
 uint32_t* beginning
@@ -178,6 +183,11 @@ namespace Coverage {
 return Ranges.front().lowAddress;
   }
 
+  uint32_t CoverageMapBase::getLowAddressOfRange( size_t index ) const
+  {
+return Ranges.at(index).lowAddress;
+  }
+
   bool CoverageMapBase::getRange( uint32_t address, AddressRange& range ) const
   {
 for ( auto r : Ranges ) {
diff --git a/tester/covoar/CoverageMapBase.h b/tester/covoar/CoverageMapBase.h
index 6ad76d3..a58c696 100644
--- a/tester/covoar/CoverageMapBase.h
+++ b/tester/covoar/CoverageMapBase.h
@@ -156,6 +156,8 @@ namespace Coverage {
  */
 int32_t getFirstLowAddress() const;
 
+uint32_t getLowAddressOfRange( size_t index ) const;
+
 /*!
  *  This method returns true and sets the address range if
  *  the address falls with the bounds of an address range
@@ -177,6 +179,8 @@ namespace Coverage {
  */
 uint32_t getSize() const;
 
+uint32_t getSizeOfRange( size_t index ) const;
+
 /*!
  *  This method returns the address of the beginning of the
  *  instruction that contains the specified address.
diff --git a/tester/covoar/DesiredSymbols.cc b/tester/covoar/DesiredSymbols.cc
index b9a5bb7..c97b25c 100644
--- a/tester/covoar/DesiredSymbols.cc
+++ b/tester/covoar/DesiredSymbols.cc
@@ -142,7 +142,7 @@ namespace Coverage {
   CoverageMapBase* theCoverageMap = s.second.unifiedCoverageMap;
   if (theCoverageMap)
   {
-// Increment the total sizeInBytes byt the bytes in the symbol
+// Increment the total sizeInBytes by the bytes in the symbol
 stats.sizeInBytes += s.second.stats.sizeInBytes;
 
 // Now scan through the coverage map of this symbol.
@@ -202,6 +202,26 @@ namespace Coverage {
 uint32_t count;
 
 // Mark NOPs as executed
+a = s.second.stats.sizeInBytes - 1;
+count = 0;
+while (a > 0) {
+  if (theCoverageMap->isStartOfInstruction( a )) {
+break;
+  }
+
+  count++;
+
+  if (theCoverageMap->isNop( a )) {
+for (la = a; la < (a + count); la++) {
+  theCoverageMap->setWasExecuted( la );
+}
+
+count = 0;
+  }
+
+  a--;
+}
+
 endAddress = s.second.stats.sizeInBytes - 1;
 a = 0;
 while (a < endAddress) {
@@ -223,12 +243,13 @@ namespace Coverage {
   ha++;
   if ( ha >= endAddress )
 break;
-} while ( !theCoverageMap->isStartOfInstruction( ha ) );
+} while ( !theCoverageMap->isStartOfInstruction( ha ) ||
+  theCoverageMap->isNop( ha ) );
   a = ha;
 }
 
 // Now scan through the coverage map of this symbol.
-endAddress = s.second.stats.sizeInBytes - 1;
+endAddress = s.second.stats.sizeInBytesWithoutNops - 1;
 a = 0;
 while (a <= endAddress) {
   // If an address was NOT executed, find consecutive unexecuted
@@ -316,7 +337,8 @@ namespace Coverage {
   void DesiredSymbols::createCoverageMap(
 const std::string& exefileName,
 const std::string& symbolName,
-uint32_t   size
+uint32_t   size,
+uint32_t   sizeWithoutNops
   )
   {
 CoverageMapBase* aCoverageMap;
@@ -354,9 +376,10 @@ namespace Coverage {
   << '/' << size << ')'
   << std::endl;
 
-if ( itr->second.stats.sizeInBytes < size )
+if ( itr->second.stats.sizeInBytes < size ) {
   itr->second.stats.sizeInBytes = size;
-else
+  itr->second.stats.sizeInBytesWithoutNops = sizeWithoutNops;
+} else
   size = itr->second.stats.sizeInBytes;
   }
 }
@@ -376,6 +399,7 @@ namespace Coverage {
 );
   itr->second.unifiedCoverageMap = aCoverageMap;
   itr->second.stats.sizeInBytes = size;
+  itr->second.stats.sizeInBytesWithoutNops = sizeWithoutNops;
 }
   }
 
@@ -479,7 +503,7 @@ namespace 

[PATCH v4 0/2] Fix NOP recognition

[Alex White]

v4:
- Add specialized CoverageMapNotFoundError exception class to ExecutableInfo
- Catch ExecutableInfo::CoverageMapNotFoundError in Coverage::finalizeSymbol()

v3:
- Fix double increment of rangeIndex in Coverage::finalizeSymbol()

v2:
- Fix leftover debugging code in Coverage::finalizeSymbol()

This patch set fixes a couple cases where NOP instructions were showing
up as unexecuted in coverage reports.

Alex White (2):
  covoar: Fix NOP execution marking
  covoar/Target_i386: Add NOP patterns

 tester/covoar/CoverageMapBase.cc  |  10 +++
 tester/covoar/CoverageMapBase.h   |   4 ++
 tester/covoar/DesiredSymbols.cc   |  38 +--
 tester/covoar/DesiredSymbols.h|  11 ++-
 tester/covoar/ExecutableInfo.cc   |   2 +-
 tester/covoar/ExecutableInfo.h|   4 ++
 tester/covoar/ObjdumpProcessor.cc | 109 +++---
 tester/covoar/Target_i386.cc  |   9 +++
 8 files changed, 151 insertions(+), 36 deletions(-)

-- 
2.27.0

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

[PATCH v4 2/2] covoar/Target_i386: Add NOP patterns

[Alex White]

A couple of NOP patterns found with the pc686 BSP were not detected.
This has been fixed.
---
 tester/covoar/Target_i386.cc | 9 +
 1 file changed, 9 insertions(+)

diff --git a/tester/covoar/Target_i386.cc b/tester/covoar/Target_i386.cc
index e0c9c0f..4567c1e 100644
--- a/tester/covoar/Target_i386.cc
+++ b/tester/covoar/Target_i386.cc
@@ -87,6 +87,15 @@ namespace Target {
   size = 3;
   return true;
 }
+if (!strncmp( [strlen(line)-28], "lea0x0(%esi,%eiz,1),%esi", 28)) 
{
+  // Could be 4 or 7 bytes of padding.
+  if (!strncmp( [strlen(line)-32], "00", 2)) {
+size = 7;
+  } else {
+size = 4;
+  }
+  return true;
+}
 
 return false;
   }
-- 
2.27.0

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

RE: Flashdisk driver based on spidev

[Jan.Sommer]

Thanks for all the pointers Chris, I will do some more research and see if I 
get a better idea how to go about this.

Best regards,
Jan

> -Original Message-
> From: Chris Johns 
> Sent: Wednesday, March 10, 2021 7:28 PM
> To: Sommer, Jan ; devel@rtems.org
> Subject: Re: Flashdisk driver based on spidev
> 
> On 11/3/21 12:59 am, jan.som...@dlr.de wrote:
> > Hello,
> >
> > We will probably at some point need support for Micron-based NOR-flash
> devices to store data which are connected via SPI.
> > I found the flashdisk API in RTEMS and was wondering if I understand
> everything correctly.
> > My idea would be to have the layers like this:
> > spidev-driver <-- micron-flashdisk-driver <-- (libblock?) <--
> > jffs2-filesystem <-- POSIX file operations
> >
> > Another option would be to have the flashdisk-driver do the SPI
> transaction itself, but I am not sure if there is a lot performance to gain.
> > The bus setup will not change between consecutive read/write accesses to
> the memory (except for CS if multiple flash devices are present).
> > From my work with the spidev drivers, which are currently in review, the
> overhead didn't seem very big to me for these conditions considering that
> writes will always be in the kB range due to the sector size.
> > Using the spidev API would have the benefit that the same flashdisk driver
> can be used with different SPI devices.
> >
> > Are these assumptions reasonable or does someone have further
> suggestions to consider?
> >
> 
> I do not think the JFFS2 will sit on top of libblock. The flashdisk does block
> management and I am not sure you want that with JFFSv2,
> 
> I suggest a CFI based NOR flash chip driver that uses the QSPI driver. Then a
> file to glue it to JFFS2. The flash chip driver needs to provide:
> 
>  - open
>  - read   - hooked to JFFSv2
>  - write  - hooked to JFFSv2
>  - erase  - hooked to JFFSv2
>  - close  - hooked to JFFSv2 destroy
> 
> I would also considering supporting a base and size in the JFFS flash chip 
> glue.
> This lets you leave the bottom part of a boot QSPI flash to support a boot and
> factory image (anti-bricking) set of blocks. I have also used a top block to 
> hold
> factory settings in some Zynq devices.
> 
> Chris
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [PATCH v2] user/bsps: Mention fixed console baud rate for zynq

[Joel Sherrill]

If this matches the state of the got master, then ok.

On Mon, Mar 15, 2021, 7:55 AM  wrote:

> Could someone please have a look at this patch?
>
> > -Original Message-
> > From: Sommer, Jan
> > Sent: Friday, March 5, 2021 7:04 PM
> > To: devel@rtems.org
> > Cc: Sommer, Jan 
> > Subject: [PATCH v2] user/bsps: Mention fixed console baud rate for zynq
> >
> > ---
> >  user/bsps/arm/xilinx-zynq.rst | 12 
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/user/bsps/arm/xilinx-zynq.rst
> b/user/bsps/arm/xilinx-zynq.rst
> > index 365c336..29f9cb0 100644
> > --- a/user/bsps/arm/xilinx-zynq.rst
> > +++ b/user/bsps/arm/xilinx-zynq.rst
> > @@ -37,6 +37,18 @@ to return the peripheral clock. Normally this is half
> the
> > CPU  clock. This function is declared ``weak`` so you can override the
> default
> > behaviour by providing it in your application.
> >
> > +Console
> > +---
> > +
> > +The console driver for the UARTs will always be initialized to a baud
> > +rate of 115200 with 8 bit characters, 1 stop bit and no parity bits
> > +during start up.
> > +Previous configurations programmed into the hardware by the Xilinx
> > +tools or a bootloader will be overwritten.
> > +
> > +The settings for the console driver can be changed by the user
> > +application through the termios API afterwards.
> > +
> >  Debugging with xilinx_zynq_a9_qemu
> >  --
> >
> > --
> > 2.17.1
>
> ___
> 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 v2] user/bsps: Mention fixed console baud rate for zynq

[Jan.Sommer]

Could someone please have a look at this patch?

> -Original Message-
> From: Sommer, Jan
> Sent: Friday, March 5, 2021 7:04 PM
> To: devel@rtems.org
> Cc: Sommer, Jan 
> Subject: [PATCH v2] user/bsps: Mention fixed console baud rate for zynq
> 
> ---
>  user/bsps/arm/xilinx-zynq.rst | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/user/bsps/arm/xilinx-zynq.rst b/user/bsps/arm/xilinx-zynq.rst
> index 365c336..29f9cb0 100644
> --- a/user/bsps/arm/xilinx-zynq.rst
> +++ b/user/bsps/arm/xilinx-zynq.rst
> @@ -37,6 +37,18 @@ to return the peripheral clock. Normally this is half the
> CPU  clock. This function is declared ``weak`` so you can override the  
> default
> behaviour by providing it in your application.
> 
> +Console
> +---
> +
> +The console driver for the UARTs will always be initialized to a baud
> +rate of 115200 with 8 bit characters, 1 stop bit and no parity bits
> +during start up.
> +Previous configurations programmed into the hardware by the Xilinx
> +tools or a bootloader will be overwritten.
> +
> +The settings for the console driver can be changed by the user
> +application through the termios API afterwards.
> +
>  Debugging with xilinx_zynq_a9_qemu
>  --
> 
> --
> 2.17.1

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

code reformatting was Re: Regarding gsoc project 3860

[Joel Sherrill]

On Mon, Mar 15, 2021 at 4:20 AM Hesham Almatary <
hesham.almat...@cl.cam.ac.uk> wrote:

> Hello Ayushman and Ida,
>
> Usually, if multiple students really want to work on a particular
> project (and can't/don't want to choose another), there can be
> multiple proposals for the same project and we choose the best one.
> Sometimes a project can be split up between two students to work on to
> minimise conflicts.
>

There are multiple things that need to be addressed here.

First, there have been discussions on devel@ about code formatting tools.
Sebastian has posted a configuration for the indent program but offhand
I don't know where that is. It may be in the documentation.

For indent to move forward from here, its impact on the code in a directory
that is thought to follow the RTEMS style well would need to be evaluated.
Do the rules need to be tweaked to avoid changes? Is the source code
actually
just not in conformance with published rules? The process here is to
evaluate
the difference between tool output and existing code and work to close the
delta by tweaking rules and fixing code. The end is expected to be that
there
are a few places where the tool can't produce RTEMS style and we have to
discuss adopting something the tool can produce.

I don't recall if Sebastian evaluated the llvm formatter and created a
configuration
for it. In this case, creating a configuration for this tool before
evaluating the
difference in output would be the path forward. If this formatter is
better, then
I would like to see an RTEMS style added to their options.

With either tool, a factor is integrating it into the development process.
I'm
not sure what a GSoC project would do about this.

So there are two potential projects here. My question is not conflict on
project choice, it is whether either is an appropriate GSoC project. With
the shorter time frame, I think the scope of the project is in the right
ballpark.
Is there enough coding? I don't know.

--joel


>
> On Mon, 15 Mar 2021 at 09:45, Ida Delphine  wrote:
> >
> > Umm...did you bring up a discussion regarding this project earlier?
> >
> > On Mon, 15 Mar 2021, 8:10 am Ayushman Mishra, 
> wrote:
> >>
> >> AYUSHMAN MISHRA
> >>
> >> Hello Ida delphini AYUSHMAN here , Can you please select any other
> project for gsoc as I am also currently working on proposal for the same
> project
> >> https://devel.rtems.org/ticket/3860 for gsoc 2021
> >
> > ___
> > 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
>
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [PATCH] rtems-examples: Add CoreMark Benchmark

[Joel Sherrill]

On Mon, Mar 15, 2021, 5:53 AM Hesham Almatary 
wrote:

> Hello Gedare,
>
> Yeah adding Make support should be straightforward. I just assumed
> Make will be deprecated soon based on [1] and haven't bothered
> supporting it.
>
> [1]
> https://github.com/RTEMS/rtems-examples/blob/983926a7e519be85f630c620430e7e1ac3e0f4ea/README.Makefile#L32


There are projects that use it. Although it is old and it would be nice to
go to something else, that something else isn't there. It's there.foenthe
foreseeable future.

--joel


>
>
>
> On Sun, 14 Mar 2021 at 18:47, Gedare Bloom  wrote:
> >
> > Hi Hesham,
> >
> > Nice work getting this integrated in the upstream. I guess that the
> > git submodule instructions for building with waf will work for this,
> > but not Make. How hard would it be for you to integrate the submodule
> > with Make?
> >
> > This is fine with me as-is, I just want to know if we can keep it
> > supporting both build systems.
> >
> > On Sat, Mar 13, 2021 at 12:50 AM Hesham Almatary
> >  wrote:
> > >
> > > CoreMark's primary goals are simplicity and providing a method for
> > > testing only a processor's core features. It is used primarily here as
> > > a performance benchmark.
> > >
> > > Built and tested for RISC-V rv64imafdc_medany on QEMU and HW
> > > ---
> > >  .gitmodules  |  3 +++
> > >  benchmarks/coremark/coremark |  1 +
> > >  benchmarks/coremark/wscript  | 50 
> > >  benchmarks/wscript   |  2 +-
> > >  4 files changed, 55 insertions(+), 1 deletion(-)
> > >  create mode 16 benchmarks/coremark/coremark
> > >  create mode 100644 benchmarks/coremark/wscript
> > >
> > > diff --git a/.gitmodules b/.gitmodules
> > > index ae86e49..d7e52b9 100644
> > > --- a/.gitmodules
> > > +++ b/.gitmodules
> > > @@ -1,3 +1,6 @@
> > >  [submodule "rtems_waf"]
> > > path = rtems_waf
> > > url = git://git.rtems.org/rtems_waf.git
> > > +[submodule "benchmarks/coremark/coremark"]
> > > +   path = benchmarks/coremark/coremark
> > > +   url = g...@github.com:eembc/coremark.git
> > > diff --git a/benchmarks/coremark/coremark
> b/benchmarks/coremark/coremark
> > > new file mode 16
> > > index 000..1541482
> > > --- /dev/null
> > > +++ b/benchmarks/coremark/coremark
> > > @@ -0,0 +1 @@
> > > +Subproject commit 1541482bf3e6ef7f5c69f5be76b14537b60833d0
> > > diff --git a/benchmarks/coremark/wscript b/benchmarks/coremark/wscript
> > > new file mode 100644
> > > index 000..2ec5f1e
> > > --- /dev/null
> > > +++ b/benchmarks/coremark/wscript
> > > @@ -0,0 +1,50 @@
> > > +#-
> > > +# SPDX-License-Identifier: BSD-2-Clause
> > > +#
> > > +# Copyright (c) 2021 Hesham Almatary
> > > +#
> > > +# This software was developed by SRI International and the University
> of
> > > +# Cambridge Computer Laboratory (Department of Computer Science and
> > > +# Technology) under DARPA contract HR0011-18-C-0016 ("ECATS"), as
> part of the
> > > +# DARPA SSITH research programme.
> > > +#
> > > +# Redistribution and use in source and binary forms, with or without
> > > +# modification, are permitted provided that the following conditions
> > > +# are met:
> > > +# 1. Redistributions of source code must retain the above copyright
> > > +#notice, this list of conditions and the following disclaimer.
> > > +# 2. Redistributions in binary form must reproduce the above copyright
> > > +#notice, this list of conditions and the following disclaimer in
> the
> > > +#documentation and/or other materials provided with the
> distribution.
> > > +#
> > > +# THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS''
> AND
> > > +# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
> THE
> > > +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
> PURPOSE
> > > +# ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE
> LIABLE
> > > +# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> CONSEQUENTIAL
> > > +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE
> GOODS
> > > +# OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> INTERRUPTION)
> > > +# HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
> STRICT
> > > +# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
> ANY WAY
> > > +# OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY
> OF
> > > +# SUCH DAMAGE.
> > > +#
> > > +
> > > +import rtems_waf.rtems as rtems
> > > +
> > > +def build(bld):
> > > +rtems.build(bld)
> > > +
> > > +bld(features = 'c cprogram',
> > > +target = 'coremark.bin',
> > > +includes = ['coremark/', 'coremark/posix/'],
> > > +source = ['coremark/rtems/init.c',
> 'coremark/posix/core_portme.c',
> > > +  'coremark/core_list_join.c', 'coremark/core_main.c',
> > > +  'coremark/core_matrix.c', 'coremark/core_state.c',
> > > +  'coremark/core_util.c'],
> > > +
> > > +

Re: [PATCH] rtems-examples: Add CoreMark Benchmark

[Hesham Almatary]

Hello Gedare,

Yeah adding Make support should be straightforward. I just assumed
Make will be deprecated soon based on [1] and haven't bothered
supporting it.

[1] 
https://github.com/RTEMS/rtems-examples/blob/983926a7e519be85f630c620430e7e1ac3e0f4ea/README.Makefile#L32



On Sun, 14 Mar 2021 at 18:47, Gedare Bloom  wrote:
>
> Hi Hesham,
>
> Nice work getting this integrated in the upstream. I guess that the
> git submodule instructions for building with waf will work for this,
> but not Make. How hard would it be for you to integrate the submodule
> with Make?
>
> This is fine with me as-is, I just want to know if we can keep it
> supporting both build systems.
>
> On Sat, Mar 13, 2021 at 12:50 AM Hesham Almatary
>  wrote:
> >
> > CoreMark's primary goals are simplicity and providing a method for
> > testing only a processor's core features. It is used primarily here as
> > a performance benchmark.
> >
> > Built and tested for RISC-V rv64imafdc_medany on QEMU and HW
> > ---
> >  .gitmodules  |  3 +++
> >  benchmarks/coremark/coremark |  1 +
> >  benchmarks/coremark/wscript  | 50 
> >  benchmarks/wscript   |  2 +-
> >  4 files changed, 55 insertions(+), 1 deletion(-)
> >  create mode 16 benchmarks/coremark/coremark
> >  create mode 100644 benchmarks/coremark/wscript
> >
> > diff --git a/.gitmodules b/.gitmodules
> > index ae86e49..d7e52b9 100644
> > --- a/.gitmodules
> > +++ b/.gitmodules
> > @@ -1,3 +1,6 @@
> >  [submodule "rtems_waf"]
> > path = rtems_waf
> > url = git://git.rtems.org/rtems_waf.git
> > +[submodule "benchmarks/coremark/coremark"]
> > +   path = benchmarks/coremark/coremark
> > +   url = g...@github.com:eembc/coremark.git
> > diff --git a/benchmarks/coremark/coremark b/benchmarks/coremark/coremark
> > new file mode 16
> > index 000..1541482
> > --- /dev/null
> > +++ b/benchmarks/coremark/coremark
> > @@ -0,0 +1 @@
> > +Subproject commit 1541482bf3e6ef7f5c69f5be76b14537b60833d0
> > diff --git a/benchmarks/coremark/wscript b/benchmarks/coremark/wscript
> > new file mode 100644
> > index 000..2ec5f1e
> > --- /dev/null
> > +++ b/benchmarks/coremark/wscript
> > @@ -0,0 +1,50 @@
> > +#-
> > +# SPDX-License-Identifier: BSD-2-Clause
> > +#
> > +# Copyright (c) 2021 Hesham Almatary
> > +#
> > +# This software was developed by SRI International and the University of
> > +# Cambridge Computer Laboratory (Department of Computer Science and
> > +# Technology) under DARPA contract HR0011-18-C-0016 ("ECATS"), as part of 
> > the
> > +# DARPA SSITH research programme.
> > +#
> > +# Redistribution and use in source and binary forms, with or without
> > +# modification, are permitted provided that the following conditions
> > +# are met:
> > +# 1. Redistributions of source code must retain the above copyright
> > +#notice, this list of conditions and the following disclaimer.
> > +# 2. Redistributions in binary form must reproduce the above copyright
> > +#notice, this list of conditions and the following disclaimer in the
> > +#documentation and/or other materials provided with the distribution.
> > +#
> > +# THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
> > +# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> > +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR 
> > PURPOSE
> > +# ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
> > +# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR 
> > CONSEQUENTIAL
> > +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> > +# OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> > +# HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, 
> > STRICT
> > +# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> > +# OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> > +# SUCH DAMAGE.
> > +#
> > +
> > +import rtems_waf.rtems as rtems
> > +
> > +def build(bld):
> > +rtems.build(bld)
> > +
> > +bld(features = 'c cprogram',
> > +target = 'coremark.bin',
> > +includes = ['coremark/', 'coremark/posix/'],
> > +source = ['coremark/rtems/init.c', 'coremark/posix/core_portme.c',
> > +  'coremark/core_list_join.c', 'coremark/core_main.c',
> > +  'coremark/core_matrix.c', 'coremark/core_state.c',
> > +  'coremark/core_util.c'],
> > +
> > +defines = [
> > +# FLAGS_STR is used within CoreMark to print the compiler flags 
> > used
> > +'FLAGS_STR="'+' '.join([str(flag) for flag in bld.env.CFLAGS])+'"'
> > +]
> > +)
> > diff --git a/benchmarks/wscript b/benchmarks/wscript
> > index 12741e7..0947060 100644
> > --- a/benchmarks/wscript
> > +++ b/benchmarks/wscript
> > @@ -7,4 +7,4 @@ import rtems_waf.rtems as rtems
> >
> >  def build(bld):
> >  

Re: Regarding gsoc project 3860

[Hesham Almatary]

Hello Ayushman and Ida,

Usually, if multiple students really want to work on a particular
project (and can't/don't want to choose another), there can be
multiple proposals for the same project and we choose the best one.
Sometimes a project can be split up between two students to work on to
minimise conflicts.

On Mon, 15 Mar 2021 at 09:45, Ida Delphine  wrote:
>
> Umm...did you bring up a discussion regarding this project earlier?
>
> On Mon, 15 Mar 2021, 8:10 am Ayushman Mishra,  
> wrote:
>>
>> AYUSHMAN MISHRA
>>
>> Hello Ida delphini AYUSHMAN here , Can you please select any other project 
>> for gsoc as I am also currently working on proposal for the same project
>> https://devel.rtems.org/ticket/3860 for gsoc 2021
>
> ___
> 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: Regarding gsoc project 3860

[Ida Delphine]

Umm...did you bring up a discussion regarding this project earlier?

On Mon, 15 Mar 2021, 8:10 am Ayushman Mishra, 
wrote:

> AYUSHMAN MISHRA
>
> Hello Ida delphini AYUSHMAN here , Can you please select any other project
> for gsoc as I am also currently working on proposal for the same project
> https://devel.rtems.org/ticket/3860 for gsoc 2021
>
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Regarding gsoc project 3860

[Ayushman Mishra]

AYUSHMAN MISHRA

Hello Ida delphini AYUSHMAN here , Can you please select any other project
for gsoc as I am also currently working on proposal for the same project
https://devel.rtems.org/ticket/3860 for gsoc 2021
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: Current master fails make

[Richi Dubey]

Awesome, Thanks!

On Mon, Mar 15, 2021 at 12:24 PM Amaan Cheval 
wrote:

> Hey Richi,
>
> You probably need to upgrade and rebuild your toolchain using RSB to
> support these new ftw.h related tests.
>
> See this relevant mail from the list announcing this:
> https://lists.rtems.org/pipermail/users/2021-March/068264.html
>
> Hope that helps!
>
> On Mon, Mar 15, 2021, 12:02 PM Richi Dubey  wrote:
>
>> Hi,
>>
>> When I run make, I get this error
>> /home/richi/quick-start/LatestStrong/src/rtems/c/src/../../testsuites/psxtests/psxftw01/init.c:44:10:
>> fatal error: ftw.h: No such file or directory
>>44 | #include 
>>
>> I think its because of this commit;
>> https://git.rtems.org/rtems/commit/testsuites/psxtests/psxftw01/init.c?id=a26a326e55922f3d52639d361e49c3ed0c3834c0
>>
>> Please let me know how to resolve this.
>> ___
>> 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: Current master fails make

[Amaan Cheval]

Hey Richi,

You probably need to upgrade and rebuild your toolchain using RSB to
support these new ftw.h related tests.

See this relevant mail from the list announcing this:
https://lists.rtems.org/pipermail/users/2021-March/068264.html

Hope that helps!

On Mon, Mar 15, 2021, 12:02 PM Richi Dubey  wrote:

> Hi,
>
> When I run make, I get this error
> /home/richi/quick-start/LatestStrong/src/rtems/c/src/../../testsuites/psxtests/psxftw01/init.c:44:10:
> fatal error: ftw.h: No such file or directory
>44 | #include 
>
> I think its because of this commit;
> https://git.rtems.org/rtems/commit/testsuites/psxtests/psxftw01/init.c?id=a26a326e55922f3d52639d361e49c3ed0c3834c0
>
> Please let me know how to resolve this.
> ___
> 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

Current master fails make

[Richi Dubey]

Hi,

When I run make, I get this error
/home/richi/quick-start/LatestStrong/src/rtems/c/src/../../testsuites/psxtests/psxftw01/init.c:44:10:
fatal error: ftw.h: No such file or directory
   44 | #include 

I think its because of this commit;
https://git.rtems.org/rtems/commit/testsuites/psxtests/psxftw01/init.c?id=a26a326e55922f3d52639d361e49c3ed0c3834c0

Please let me know how to resolve this.
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel