nasa / osal

The Core Flight System (cFS) Operating System Abstraction Layer (OSAL)
Apache License 2.0
540 stars 213 forks source link

Clean up "-D" compile time macros used in pc-linux build #50

Closed skliper closed 4 years ago

skliper commented 4 years ago

This is the equivalent of trac 17 in the CFE PSP -- see [https://babelfish.arc.nasa.gov/trac/cfs_psp/ticket/17]. The same build flags are duplicated in the OSAL BSP.

The "pc-linux" OSAL BSP defines the following to be added to the compiler CFLAGS:

{{{-D_EL -DENDIAN=_EL -DSOFTWARE_LITTLE_BIT_ORDER -Dix86 -Dix86 -Dposix -DX86PC -D_REENTRANT -DEMBED -DOS_DEBUG_LEVEL=3}}}

These were brought into the cmake build from the original build scripts in order to be consistent just in case any code required it. However, they are unnecessary, many are not even used anywhere in CFE/OSAL, and potentially even wrong.

The reality is that with Linux, the "pc-linux" is a general purpose PSP that can most likely be used on any general-purpose development machine that runs Linux. It is not limited to only x86 PC's, and in fact works just fine on ARM, PowerPC, and Microblaze targets too.

I have successfully used the (unmodified) pc-linux BSP to execute unit tests on a BeagleBone Black (ARM) as well as an emulated PowerPC 440 based development machine. In all these cases, the "x86" macros are wrong, and on the PowerPC, the EL/ENDIAN/SOFTWARE_LITTLE_BIT_ORDER are wrong too.

To summarize - I recommend removing ALL of these macros from the pc- linux build when using the cmake scripts (the old makefiles can stay as- is).

skliper commented 4 years ago

Imported from trac issue 27. Created by jphickey on 2015-03-18T12:09:07, last modified: 2015-11-20T16:22:16

skliper commented 4 years ago

Trac comment by jphickey on 2015-04-14 12:57:48:

This changeset has been rebased to the (unapproved) 2015-04-14 baseline

This is due to a dependency on files added/changed as part of that basline. Attempting to base this change on the official 2015-02-26 OSAL is potentially possible but will result in merge conflicts when putting these back together again.

Commit [changeset:ce108a3] is ready for review

skliper commented 4 years ago

Trac comment by glimes on 2015-05-18 20:11:16:

Accept based on commit [changeset:8662839]

The only flag being removed that I ever saw used was SOFTWARE_LITTLE_BIT_ORDER, and that code could and should have been written to not need it.

If it still exists, I'll gladly provide the rewrite.

skliper commented 4 years ago

Trac comment by sstrege on 2015-05-19 10:53:38:

The SOFTWARE_BIG_BIT_ORDER flag is used in the cFE's ccsds.h file.

The SOFTWARE_LITTLE_BIT_ORDER flag is used in the cFE UTF and elf tool makefiles.

skliper commented 4 years ago

Trac comment by jphickey on 2015-05-19 11:23:50:

Replying to [comment:3 sstrege]:

The SOFTWARE_BIG_BIT_ORDER flag is used in the cFE's ccsds.h file.

The SOFTWARE_LITTLE_BIT_ORDER flag is used in the cFE UTF and elf tool makefiles.

Re: SOFTWARE_BIG_BIT_ORDER Thanks for pointing this one out - this is a new one that wasn't there in CFE 6.3.2 which is the version that I am porting all these changes from.

It appears that the related CFE_MAKE_BIG16 macro was also added to some table definitions in the SC app.

Seems relatively easy to modify SC to not need this, though - as it is just hard-coded tables. Wouldn't it be easier to define the SC table simply as uint8's, and therefore not need any macro magic?

Re: SOFTWARE_LITTLE_BIT_ORDER These may be left in the makefile due to cut-and-paste, but they are not serving any purpose as the code doesn't use them. In fact, these would be wrong if building UT for a big endian processor, which is IMO evidence why these //do// need to be purged out.

This change set is only currently removing these macros from the CMake build.

skliper commented 4 years ago

Trac comment by acudmore on 2015-06-01 14:09:56:

Approve as this applies to the CMake build.

Comment 4 looks like a good case for removing the macros in the traditional make system. But we at least have to figure out what to do with the tables.

skliper commented 4 years ago

Trac comment by jphickey on 2015-07-09 14:17:52:

== REBASED AND UPDATED ==

For the 2015-07-14 CCB review/discussion

Both of these are merged into the {{{ic-2015-07-14}}} integration candidate for evaluation.

skliper commented 4 years ago

Trac comment by glimes on 2015-07-13 12:51:23:

Approve.

skliper commented 4 years ago

Trac comment by sduran on 2015-07-14 11:49:38:

Approve (compatibility macro is good)

skliper commented 4 years ago

Trac comment by jwilmot on 2015-07-14 12:36:37:

Recommend accept.

skliper commented 4 years ago

Trac comment by glimes on 2015-07-16 10:17:16:

Included in [changeset:ec67bd4 2015-07-14 development merge], now present on Babelfish and IPAS-LIB.