Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove CONFIG_CAN_PASS_STRUCT #766

Merged
merged 3 commits into from
Apr 11, 2020
Merged

Remove CONFIG_CAN_PASS_STRUCT #766

merged 3 commits into from
Apr 11, 2020

Conversation

patacongo
Copy link
Contributor

This commit resolves issue #620

Remove CONFIG_CAN_PASS_STRUCTS #620

The configuration option CONFIG_CAN_PASS_STRUCTS was added many years ago to support an old version of the SDCC compiler. That compiler is currently used only with the Z80 and Z180 targets. The limitation of that old compiler was that it could not pass structures or unions as either inputs or outputs. For example:

#ifdef CONFIG_CAN_PASS_STRUCTS
struct mallinfo mallinfo(void);
#else
int      mallinfo(FAR struct mallinfo *info);
#endif

And even leads to violation of a few POSIX interfaces like:

#ifdef CONFIG_CAN_PASS_STRUCTS
int  sigqueue(int pid, int signo, union sigval value);
#else
int  sigqueue(int pid, int signo, FAR void *sival_ptr);
#endif

This breaks the 1st INVIOLABLES rule:

Strict POSIX compliance

o Strict conformance to the portable standard OS interface as defined at
OpenGroup.org.
o A deeply embedded system requires some special support. Special
support must be minimized.
o The portable interface must never be compromised only for the sake of
expediency.
o Expediency or even improved performance are not justifications for
violation of the strict POSIX interface

Also, it appears that the current SDCC compilers have resolve this issue and so, perhaps, this is no longer a problem: z88dk/z88dk#1132

NOTE: This commit cannot pass the PR checks because it depends on matching changes to the apps/ directory.

This commit resolves issue apache#620:

Remove CONFIG_CAN_PASS_STRUCTS apache#620

The configuration option CONFIG_CAN_PASS_STRUCTS was added many years ago to support an old version of the SDCC compiler. That compiler is currently used only with the Z80 and Z180 targets. The limitation of that old compiler was that it could not pass structures or unions as either inputs or outputs. For example:

    #ifdef CONFIG_CAN_PASS_STRUCTS
    struct mallinfo mallinfo(void);
    #else
    int      mallinfo(FAR struct mallinfo *info);
    #endif

And even leads to violation of a few POSIX interfaces like:

    #ifdef CONFIG_CAN_PASS_STRUCTS
    int  sigqueue(int pid, int signo, union sigval value);
    #else
    int  sigqueue(int pid, int signo, FAR void *sival_ptr);
    #endif

This breaks the 1st INVIOLABLES rule:

Strict POSIX compliance
-----------------------

  o Strict conformance to the portable standard OS interface as defined at
    OpenGroup.org.
  o A deeply embedded system requires some special support.  Special
    support must be minimized.
  o The portable interface must never be compromised only for the sake of
    expediency.
  o Expediency or even improved performance are not justifications for
   violation of the strict POSIX interface

Also, it appears that the current SDCC compilers have resolve this issue and so, perhaps, this is no longer a problem: z88dk/z88dk#1132

NOTE:  This commit cannot pass the PR checks because it depends on matching changes to the apps/ directory.
@xiaoxiang781216 xiaoxiang781216 linked an issue Apr 11, 2020 that may be closed by this pull request
Run all files modified by PR 766 through nxstyle and fix any resulting complaints.

NOTE:  Numerous "Mixed case identifier" errors in arch/arm/src/cxd56xx/cxd56_gnss.c were not fixed because this problem is of much larger scope than this file.
@patacongo
Copy link
Contributor Author

patacongo commented Apr 11, 2020

Merge-related notes:

  1. This commit cannot pass the PR checks because it depends on matching changes to the apps/ directory. That is PR apps/ PR 170

NOTE: This PR depends apps PR 170, but apps PR 170 should not depend on this PR. Therefore the strategy here is (1) merge apps/ PR 170 first, then this PR should pass its checks (with the following exception).

  1. Numerous "Mixed case identifier" errors in arch/arm/src/cxd56xx/cxd56_gnss.c were not fixed because this problem is of much larger scope than this file.

Modified boards/z80/z180/p112/README.txt and boards/z80/z80/z80sim/README.txt:

IMPORTANT NOTE as of 2020-4-11:  Support for CONFIG_CAN_PASS_STRUCTS was removed in NuttX-9.1.  This was necessary to enforce some POSIX interface compliance but also means that ALL older SDCC versions will no long build with NuttX.  I have been told that the newest SDCC compilers can indeed pass structure and union parameters and return values.  If that is correct, then perhaps the newer SDCC compilers will be used.  Otherwise, it will be necessary to use some other, more compliant compiler.
@Ouss4 Ouss4 merged commit 8b87baa into apache:master Apr 11, 2020
@patacongo patacongo deleted the canpass branch April 11, 2020 20:32
Hrompic pushed a commit to Hrompic/incubator-nuttx that referenced this pull request Nov 10, 2021
This is the companion to PR apache#766.  It removes the CONFIG_CAN_PASS_STRUCT option as recommended by Issue apache#620

NuttX PR apache#766 depends on PR being in place but not vice versa.  This PR should be merge-able without apache#766 and then PR apache#766 should also pass its checks.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove CONFIG_CAN_PASS_STRUCTS
3 participants