Skip to content

Commit

Permalink
HotFix #1221, OS_Impl_Loader on RTEMS
Browse files Browse the repository at this point in the history 
- Add aliased typedef to handle renames from RTEMS 4.11 to RTEMS 5
- Cast to cpuaddress before assignment to `OS_module_address_t` elements
  • Loading branch information
@astrogeco
astrogeco committed Mar 10, 2022
1 parent 75fe674 commit 3560f99
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 7 deletions.
16 changes: 16 additions & 0 deletions src/os/rtems/inc/os-rtems.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,12 @@
#include <rtems/malloc.h>
#include <rtems/rtems/intr.h>

#ifndef _RTEMS_5_

#include <rtems/rtl/rtl.h> /* needed for RTEMS 4.11 alias */

This comment has been minimized.

Sign in to view

Copy link
@jphickey

jphickey Mar 10, 2022

Contributor

Just curious why this is needed to be wrapped in ifndef?

Both RTEMS 4.11 and 5 provide the rtl/rtl.h header, and I would think it would be needed on both.

This comment has been minimized.

Sign in to view

Copy link
@astrogeco

astrogeco Mar 10, 2022

Author Contributor

For some reason 5.0 works without it

This comment has been minimized.

Sign in to view

Copy link
@astrogeco

astrogeco Mar 10, 2022

Author Contributor

Nvm, this makes sense, Rtems 5.0 doesn't need the alias so then it doesn't need the include. So if I just move the alias over to os-impl-loader we won't need this include.

This comment has been minimized.

Sign in to view

Copy link
@astrogeco

astrogeco Mar 10, 2022

Author Contributor

I'll move it

This comment has been minimized.

Sign in to view

Copy link
@jphickey

jphickey Mar 11, 2022

Contributor

Yeah, if you move the alias typedef over to os-impl-loader.c then that would make it minimally scoped to only the code that actually needs it, and it should be only the typedef at that point (very simple). I like that the best.

#endif

#include "os-shared-globaldefs.h"

/****************************************************************************************
Expand All @@ -60,17 +66,27 @@
#define OSAL_UNRESOLV_REC_TYPE rtems_rtl_unresolv_rec
#define OSAL_UNRESOLVED_SYMBOL rtems_rtl_unresolved_symbol
#define OSAL_UNRESOLVED_ITERATE rtems_rtl_unresolved_iterate

#else

#define OSAL_HEAP_INFO_BLOCK region_information_block
#define OSAL_UNRESOLV_REC_TYPE rtems_rtl_unresolv_rec_t
#define OSAL_UNRESOLVED_SYMBOL rtems_rtl_unresolved_name
#define OSAL_UNRESOLVED_ITERATE rtems_rtl_unresolved_interate

#endif


/****************************************************************************************
TYPEDEFS
***************************************************************************************/

#ifndef _RTEMS_5_

typedef rtems_rtl_obj_t rtems_rtl_obj; /* Alias for RTEMS 4.11 */

#endif

typedef struct
{
uint32 ClockAccuracyNsec;
Expand Down
14 changes: 7 additions & 7 deletions src/os/rtems/src/os-impl-loader.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ int32 OS_ModuleUnload_Impl(const OS_object_token_t *token)
*-----------------------------------------------------------------*/
int32 OS_ModuleGetInfo_Impl(const OS_object_token_t *token, OS_module_prop_t *module_prop)
{
rtems_rtl_obj * obj;
rtems_rtl_obj *obj;
OS_impl_module_internal_record_t *impl;
int32 status = OS_ERROR;

Expand All @@ -232,12 +232,12 @@ int32 OS_ModuleGetInfo_Impl(const OS_object_token_t *token, OS_module_prop_t *mo
if (obj != NULL)
{
module_prop->addr.valid = true;
module_prop->addr.code_address = obj->text_base;
module_prop->addr.code_size = rtems_rtl_obj_text_size(obj);
module_prop->addr.data_address = obj->data_base;
module_prop->addr.data_size = rtems_rtl_obj_data_size(obj);
module_prop->addr.bss_address = obj->bss_base;
module_prop->addr.bss_size = rtems_rtl_obj_bss_size(obj);
module_prop->addr.code_address = (cpuaddr) obj->text_base;
module_prop->addr.code_size = (cpuaddr) rtems_rtl_obj_text_size(obj);
module_prop->addr.data_address = (cpuaddr) obj->data_base;
module_prop->addr.data_size = (cpuaddr) rtems_rtl_obj_data_size(obj);
module_prop->addr.bss_address = (cpuaddr) obj->bss_base;
module_prop->addr.bss_size = (cpuaddr) rtems_rtl_obj_bss_size(obj);

status = OS_SUCCESS;
}
Expand Down

3 comments on commit 3560f99

@astrogeco
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@acudmore and @jphickey here's my updated fix. It passess CI on RTEMS 4.11 and 5.0

@jphickey
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside from my comment about the ifdef around rtl/rtl.h header, is there any reason this needs to touch os-rtems.h at all at this point? Couldn't the os-impl-loader.c file just have the conditional typedef locally at file scope? That would limit the need of RTL headers and data types to just that file where its relevant.

@astrogeco
Copy link
Contributor Author

@astrogeco astrogeco commented on 3560f99 Mar 10, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only reason is I didn't know any better :)
I assumed we wanted to keep the RTEMS_5 ifdef in os-rtems.h

Please sign in to comment.