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

greenhills support: fix the greenhills compile warning on sizeof operand #13502

Merged
merged 1 commit into from
Sep 20, 2024

Conversation

extinguish
Copy link

@extinguish extinguish commented Sep 17, 2024

Summary

fix the greenhills compile warning on sizeof operand
the following are the detailed warning info:

CC: syslog/vsyslog.c "pthread/pthread_create.c", line 443: warning #1931-D: operand of sizeof is
not a type, variable, or dereferenced pointer expression
ptcb->cmn.timeslice = MSEC2TICK(CONFIG_RR_INTERVAL);
^

CC: dirent/lib_closedir.c "sched/sched_profil.c", line 81: warning #1931-D: operand of sizeof is not a
type, variable, or dereferenced pointer expression
wd_start(&prof->timer, PROFTICK, profil_timer_handler, arg);
^

"sched/sched_profil.c", line 142: warning #1931-D: operand of sizeof is not a
type, variable, or dereferenced pointer expression
wd_start(&prof->timer, PROFTICK, profil_timer_handler, (wdparm_t)prof);
^

CC: common/arm_modifyreg8.c "sched/sched_setscheduler.c", line 165: warning #1931-D: operand of sizeof is
not a type, variable, or dereferenced pointer expression
tcb->timeslice = MSEC2TICK(CONFIG_RR_INTERVAL);
^

CC: misc/lib_utsname.c "sched/sched_unlock.c", line 275: warning #1931-D: operand of sizeof is not a
type, variable, or dereferenced pointer expression
rtcb->timeslice = MSEC2TICK(CONFIG_RR_INTERVAL);
^

"sched/sched_roundrobin.c", line 119: warning #1931-D: operand of sizeof is
not a type, variable, or dereferenced pointer expression
tcb->timeslice = MSEC2TICK(CONFIG_RR_INTERVAL);
^

CC: armv7-m/arm_fpuconfig.c cxarm: Error: No files. Try -help. CC: misc/lib_crc8ccitt.c cxarm: Error: No files. Try -help. cxarm: Error: No files. Try -help.
CC: getprime_main.c cxarm: Error: No files. Try -help. cxarm: Error: No files. Try -help.
CC: misc/lib_log2ceil.c cxarm: Error: No files. Try -help. CC: task/task_start.c "task/task_setup.c", line 423: warning #1931-D: operand of sizeof is not a
type, variable, or dereferenced pointer expression
tcb->timeslice = MSEC2TICK(CONFIG_RR_INTERVAL);
^

the reason for the warning

in greenhills compiler, do not allow transfer operand to sizeof() directly in mcaro, we need to transfer the data type of the operand to the sizeof() in macro.
in order to fix this warning, we using typeof() to encapsulate the operand before transfer to the sizeof()

Impact

has no impact on current implementation

Testing

has pass the ostest with gcc and greenhills compiler

@extinguish extinguish force-pushed the ghs_warning_fix_4 branch 3 times, most recently from 345ff9d to 847e173 Compare September 19, 2024 02:45
@lupyuen
Copy link
Member

lupyuen commented Sep 19, 2024

[Experimental Bot, please feedback here]

NuttX PR Requirements Review

This PR appears to partially meet the NuttX requirements. Here's a breakdown:

Strengths:

  • Summary: The summary clearly states the reason for the change (fixing Greenhills compile warnings) and identifies the affected part of the code. However, it lacks a concise explanation of how the change addresses the warnings.
  • Impact: The "Impact" section is entirely empty. You need to analyze and provide information for each impact category.

Weaknesses:

  • Summary:
    • Needs a brief explanation of the solution applied to fix the warnings. For example, "The fix involves casting the result of the MSEC2TICK macro to the appropriate data type expected by sizeof."
  • Impact:
    • This section is crucial for reviewers to understand the consequences of the changes. You must address each impact category (new feature, user impact, build impact, etc.) with either a "NO" or a "YES" followed by a description if applicable.
  • Testing:
    • While you provide a structure for testing logs, the actual logs are missing. You need to include the relevant logs from before and after the change to demonstrate the fix.
    • Specify the Greenhills compiler version and other relevant build host details used for testing.

Recommendations:

  1. Elaborate on the Solution: In the summary, briefly explain the technical approach taken to resolve the sizeof warnings.
  2. Complete the Impact Assessment: Carefully consider and fill out the "Impact" section, addressing each point. For instance:
    • Impact on user: Likely NO, as it's a compiler warning fix.
    • Impact on build: Potentially YES if the fix involves changes to build flags or configurations specific to Greenhills.
    • Impact on hardware: Likely NO.
    • Impact on documentation: Potentially YES if any build instructions or configuration options need updates for Greenhills.
  3. Provide Testing Logs: Include the actual "before" and "after" testing logs demonstrating the successful resolution of the warnings.
  4. Detail Testing Environment: Specify the Greenhills compiler version and any other relevant details of your testing setup.

By addressing these points, you will significantly improve the quality of your PR and make it easier for the NuttX maintainers to review and merge your contribution.

@extinguish extinguish force-pushed the ghs_warning_fix_4 branch 4 times, most recently from 80af1ca to dc8e49d Compare September 20, 2024 01:29
@github-actions github-actions bot added the Size: XS The size of the change in this PR is very small label Sep 20, 2024
@extinguish extinguish force-pushed the ghs_warning_fix_4 branch 3 times, most recently from 082117c to 2556d13 Compare September 20, 2024 07:10
the detailed warning info are:
CC:  syslog/vsyslog.c "pthread/pthread_create.c", line 443: warning apache#1931-D: operand of sizeof is
          not a type, variable, or dereferenced pointer expression
          ptcb->cmn.timeslice = MSEC2TICK(CONFIG_RR_INTERVAL);
                                ^

CC:  dirent/lib_closedir.c "sched/sched_profil.c", line 81: warning apache#1931-D: operand of sizeof is not a
          type, variable, or dereferenced pointer expression
    wd_start(&prof->timer, PROFTICK, profil_timer_handler, arg);
                           ^

"sched/sched_profil.c", line 142: warning apache#1931-D: operand of sizeof is not a
          type, variable, or dereferenced pointer expression
    wd_start(&prof->timer, PROFTICK, profil_timer_handler, (wdparm_t)prof);
                           ^

CC:  common/arm_modifyreg8.c "sched/sched_setscheduler.c", line 165: warning apache#1931-D: operand of sizeof is
          not a type, variable, or dereferenced pointer expression
            tcb->timeslice  = MSEC2TICK(CONFIG_RR_INTERVAL);
                              ^

CC:  misc/lib_utsname.c "sched/sched_unlock.c", line 275: warning apache#1931-D: operand of sizeof is not a
          type, variable, or dereferenced pointer expression
            rtcb->timeslice = MSEC2TICK(CONFIG_RR_INTERVAL);
                              ^

"sched/sched_roundrobin.c", line 119: warning apache#1931-D: operand of sizeof is
          not a type, variable, or dereferenced pointer expression
            tcb->timeslice = MSEC2TICK(CONFIG_RR_INTERVAL);
                             ^

CC:  armv7-m/arm_fpuconfig.c cxarm: Error: No files.  Try -help.
CC:  misc/lib_crc8ccitt.c cxarm: Error: No files.  Try -help.
cxarm: Error: No files.  Try -help.
CC:  getprime_main.c cxarm: Error: No files.  Try -help.
cxarm: Error: No files.  Try -help.
CC:  misc/lib_log2ceil.c cxarm: Error: No files.  Try -help.
CC:  task/task_start.c "task/task_setup.c", line 423: warning apache#1931-D: operand of sizeof is not a
          type, variable, or dereferenced pointer expression
        tcb->timeslice      = MSEC2TICK(CONFIG_RR_INTERVAL);
                              ^

Signed-off-by: guoshichao <[email protected]>
@xiaoxiang781216 xiaoxiang781216 merged commit d26357d into apache:master Sep 20, 2024
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Size: XS The size of the change in this PR is very small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants