Re: [PATCH rtems-tools 0/8] Convert and reformat pt. 4

2021-12-07 Thread Chris Johns
On 8/12/21 1:09 am, Ryan Long wrote:
> On 12/6/2021 10:09 PM, Chris Johns wrote:
>> On 4/12/21 1:47 am, Ryan Long wrote:
>>> Hi,
>>>
>>> For this series of patches, I followed this series of steps:
>>>
>>> 1. Convert a file from the C way of doing things to C++.
>>> 2. Go through all the files that had to do with the converted file and
>>> make the formatting consistent.
>> OK to push.
>>
>> It would be helpful in future to separate the white space changes from any 
>> code
>> changes.
> 
> I guess removing "void" from an empty parameter list and adding curly brackets
> around if statements counts as code changes? 

Yes.

> I didn't see any whitespace changes in the code changes.

The following is an example:

-debug.begin(executable.elf());
+debug.begin( executable.elf() );

It takes time for me to check and see if this is the same code with whitespace
changes or whitespace and code changes.

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


Re: [PATCH] score: Fix _Workspace_Initialize_for_one_area()

2021-12-07 Thread Sebastian Huber

On 07/12/2021 18:07, Sebastian Huber wrote:

In _Workspace_Initialize_for_one_area(), properly check  if there is enough
free memory available for the configured workspace size.

The bug was introduced by commit 3d0620b607ff6459fec9d30efc1e0589bbd010f9.


Please ignore this patch. Our mail server restarted and I sent it twice.

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

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

[PATCH] score: Fix _Workspace_Initialize_for_one_area()

2021-12-07 Thread Sebastian Huber
In _Workspace_Initialize_for_one_area(), properly check  if there is enough
free memory available for the configured workspace size.

The bug was introduced by commit 3d0620b607ff6459fec9d30efc1e0589bbd010f9.
---
 cpukit/include/rtems/score/wkspaceinitone.h | 34 -
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/cpukit/include/rtems/score/wkspaceinitone.h 
b/cpukit/include/rtems/score/wkspaceinitone.h
index c68e1b5db1..ce26a1cf8f 100644
--- a/cpukit/include/rtems/score/wkspaceinitone.h
+++ b/cpukit/include/rtems/score/wkspaceinitone.h
@@ -59,26 +59,29 @@ extern "C" {
  */
 RTEMS_INLINE_ROUTINE void _Workspace_Initialize_for_one_area( void )
 {
-  uintptr_t page_size;
-  uintptr_t wkspace_size;
-  uintptr_t wkspace_size_with_overhead;
-  uintptr_t available_size;
+  uintptr_t page_size;
+  uintptr_t wkspace_size;
+  uintptr_t wkspace_size_with_overhead;
+  const Memory_Information *mem;
+  Memory_Area  *area;
+  uintptr_t free_size;
+  uintptr_t available_size;
 
   page_size = CPU_HEAP_ALIGNMENT;
   wkspace_size = rtems_configuration_get_work_space_size();
   wkspace_size_with_overhead = wkspace_size + _Heap_Area_overhead( page_size );
 
-  if ( wkspace_size < wkspace_size_with_overhead ) {
-const Memory_Information *mem;
-Memory_Area  *area;
-uintptr_t free_size;
-uintptr_t size;
+  mem = _Memory_Get();
+  _Assert( _Memory_Get_count( mem ) == 1 );
 
-mem = _Memory_Get();
-_Assert( _Memory_Get_count( mem ) == 1 );
+  area = _Memory_Get_area( mem, 0 );
+  free_size = _Memory_Get_free_size( area );
 
-area = _Memory_Get_area( mem, 0 );
-free_size = _Memory_Get_free_size( area );
+  if (
+wkspace_size < wkspace_size_with_overhead &&
+free_size >= wkspace_size_with_overhead
+  ) {
+uintptr_t size;
 
 if ( rtems_configuration_get_unified_work_area() ) {
   size = free_size;
@@ -95,7 +98,10 @@ RTEMS_INLINE_ROUTINE void 
_Workspace_Initialize_for_one_area( void )
 
 _Memory_Consume( area, size );
   } else {
-/* An unsigned integer overflow happened */
+/*
+ * An unsigned integer overflow happened, or the available free memory is
+ * not enough.
+ */
 available_size = 0;
   }
 
-- 
2.26.2

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


Re: [PATCH rtems-tools 0/8] Convert and reformat pt. 4

2021-12-07 Thread Ryan Long



On 12/6/2021 10:09 PM, Chris Johns wrote:

On 4/12/21 1:47 am, Ryan Long wrote:

Hi,

For this series of patches, I followed this series of steps:

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

OK to push.

It would be helpful in future to separate the white space changes from any code
changes.


I guess removing "void" from an empty parameter list and adding curly 
brackets around if statements counts as code changes? I didn't see any 
whitespace changes in the code changes.



Thanks,

Ryan



Thanks
Chris

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


[PATCH] score: Fix _Workspace_Initialize_for_one_area()

2021-12-07 Thread Sebastian Huber
In _Workspace_Initialize_for_one_area(), properly check  if there is enough
free memory available for the configured workspace size.

The bug was introduced by commit 3d0620b607ff6459fec9d30efc1e0589bbd010f9.
---
 cpukit/include/rtems/score/wkspaceinitone.h | 34 -
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/cpukit/include/rtems/score/wkspaceinitone.h 
b/cpukit/include/rtems/score/wkspaceinitone.h
index c68e1b5db1..ce26a1cf8f 100644
--- a/cpukit/include/rtems/score/wkspaceinitone.h
+++ b/cpukit/include/rtems/score/wkspaceinitone.h
@@ -59,26 +59,29 @@ extern "C" {
  */
 RTEMS_INLINE_ROUTINE void _Workspace_Initialize_for_one_area( void )
 {
-  uintptr_t page_size;
-  uintptr_t wkspace_size;
-  uintptr_t wkspace_size_with_overhead;
-  uintptr_t available_size;
+  uintptr_t page_size;
+  uintptr_t wkspace_size;
+  uintptr_t wkspace_size_with_overhead;
+  const Memory_Information *mem;
+  Memory_Area  *area;
+  uintptr_t free_size;
+  uintptr_t available_size;
 
   page_size = CPU_HEAP_ALIGNMENT;
   wkspace_size = rtems_configuration_get_work_space_size();
   wkspace_size_with_overhead = wkspace_size + _Heap_Area_overhead( page_size );
 
-  if ( wkspace_size < wkspace_size_with_overhead ) {
-const Memory_Information *mem;
-Memory_Area  *area;
-uintptr_t free_size;
-uintptr_t size;
+  mem = _Memory_Get();
+  _Assert( _Memory_Get_count( mem ) == 1 );
 
-mem = _Memory_Get();
-_Assert( _Memory_Get_count( mem ) == 1 );
+  area = _Memory_Get_area( mem, 0 );
+  free_size = _Memory_Get_free_size( area );
 
-area = _Memory_Get_area( mem, 0 );
-free_size = _Memory_Get_free_size( area );
+  if (
+wkspace_size < wkspace_size_with_overhead &&
+free_size >= wkspace_size_with_overhead
+  ) {
+uintptr_t size;
 
 if ( rtems_configuration_get_unified_work_area() ) {
   size = free_size;
@@ -95,7 +98,10 @@ RTEMS_INLINE_ROUTINE void 
_Workspace_Initialize_for_one_area( void )
 
 _Memory_Consume( area, size );
   } else {
-/* An unsigned integer overflow happened */
+/*
+ * An unsigned integer overflow happened, or the available free memory is
+ * not enough.
+ */
 available_size = 0;
   }
 
-- 
2.26.2

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


Re: [PATCH] untar: Make behavior similar to GNU or BSD tar

2021-12-07 Thread Christian MAUDERER

Hello Chris,

Am 07.12.21 um 05:10 schrieb Chris Johns:

On 3/12/21 11:50 pm, Christian Mauderer wrote:

RTEMS untar implementation had problems with overwriting or integrating
archives into existing directory structures. This patch adapts the
behavior to mimic that of a GNU tar or BSD tar and extends the tar01
test to check for the behavior. That is:

* If a directory structure exists, the files from the archive will be
   integrated. Existing files are overwritten.


What currently happens?


The untar fails if a directory exists that is in the archive. Note that 
is mostly true if the archive contains directories and not only files. 
That means: If I have two example archives that look like follows:


  > tar tvf image-error.tar.gz
  drwxr-xr-x christian_m/domainusers 0 2021-11-26 15:31 l1/
  drwxr-xr-x christian_m/domainusers 0 2021-11-26 14:38 l1/l2/
  -rw-r--r-- christian_m/domainusers 12 2021-11-26 14:27 l1/l2/x.txt

  > tar tvf image-ok.tar.gz
  -rw-r--r-- christian_m/domainusers 12 2021-11-26 14:27 l1/l2/x.txt

The first image contains the directories l1 and l1/l2 and the file 
l1/l2/x.txt. The second contains only the l1/l2/x.txt.


With our current implementation, I would be able to extract the first 
archive one times and a second try would fail because l1 and l1/l2 
already exist. The second archive could be extracted multiple times and 
would overwrite x.txt every time.





* If a file exists and the archive contains a directory with the same
   name, the file is removed and a directory is created. In the above
   example: if l1/l2 is a file it will be overwritten with a new
   directory.


OK


* If a directory exists and the archive contains a file with the same
   name, the directory will be replaced if it is empty. If it contains
   files, the result is an error.

* An archive also can contain only a file without the parent
   directories. If in that case one of the parent directories exists as a
   file extracting the archive results in an error. In the example: if
   l1/l2 is a file and the archive doesn't contain the directories but
   only the file l1/l2/x.txt that would be an error.

* In case of an error, it is possible that the archive has been
   partially extracted.


And what was there is not recoverable.



Correct. Note that I basically just tested what GNU and BSD tar do to 
get a reference what is "expected behavior". See


   https://devel.rtems.org/ticket/4552

There would be a number of other reasonable behaviors that could be 
considered right too. But I think the default tar utilities are a good 
reference.



Functionally this is not a big change and so I am left wondering why the
original developer(s) did not do this?


I think the original behavior of the code in RTEMS was a bit different. 
I found a bug from 2019:


  https://devel.rtems.org/ticket/3823

The solution to that bug solved the bug but changed the behavior of tar 
a bit and introduced the new problem that tar can't integrate data into 
existing directory structures.




I think the changes make sense and I do not think it will break any applications
I know of that use this code.


Thanks for reviewing it. I'll push the patch soon.

Best regards

Christian



Chris



--

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

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