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

Fix string-format type mismatches #400

Open
wants to merge 1 commit into
base: humble
Choose a base branch
from

Conversation

cliu5764
Copy link

@cliu5764 cliu5764 commented Feb 22, 2024

  Compiler warnings fixes:
c files change string formats to match type.

@Zard-C
Copy link
Contributor

Zard-C commented Feb 22, 2024

Thanks for the fix!
Btw, your commit message didn't pass the dco check,

To add your Signed-off-by line to every commit in this branch:

Ensure you have a local copy of your branch by [checking out the pull request locally via command line](https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/checking-out-pull-requests-locally).
In your local branch, run: git rebase HEAD~1 --signoff
Force push your changes to overwrite the branch: git push --force-with-lease origin beechwoods_fixes

@Zard-C

This comment was marked as outdated.

rclc/CMakeLists.txt Outdated Show resolved Hide resolved
rclc/CMakeLists.txt Outdated Show resolved Hide resolved
rclc/src/rclc/executor.c Outdated Show resolved Hide resolved
Copy link
Contributor

@JanStaschulat JanStaschulat left a comment

Choose a reason for hiding this comment

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

High level of compiler warnings (with -Wpedantic) should be kept and the warnings should be resolved in the source code instead.

rclc/CMakeLists.txt Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 0% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 69.20%. Comparing base (65d293d) to head (c4813b5).

Files Patch % Lines
rclc/src/rclc/timer.c 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           humble     #400   +/-   ##
=======================================
  Coverage   69.20%   69.20%           
=======================================
  Files          16       16           
  Lines        2715     2715           
  Branches      765      765           
=======================================
  Hits         1879     1879           
  Misses        450      450           
  Partials      386      386           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cliu5764 cliu5764 changed the title Fix string-format type mismatches & add https://github.com/ros2/rclc Fix string-format type mismatches Mar 1, 2024
@JanStaschulat
Copy link
Contributor

there is still the compiler warning on OpenRobotics build farm CI job:

format ‘%d’ expects argument of type ‘int’, but argument 5 has type ‘size_t’ {aka ‘long unsigned int’} [-Wformat=] 1836 | "Subscription added to wait_set_subscription[%d]", | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 1837 | executor->handles[i].index); | ~~~~~~~~~~~~~~~~~~~~~~~~~~ | | | size_t {aka long unsigned int}

https://build.ros2.org/job/Hpr__rclc__ubuntu_jammy_amd64/36/gcc/fileName.-1655778739/

Copy link
Contributor

@JanStaschulat JanStaschulat left a comment

Choose a reason for hiding this comment

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

I am fine with the pull request, as there are only format string changes. However, we need to investigate, why the build job on OpenRobotics build farm creates still warnings, while the one on gitbhub does not. Ideally, there should be no warnings (see my comment with the link to the Warning messages).

@cliu5764
Copy link
Author

cliu5764 commented Mar 5, 2024

Here is the warning I see:

/b/mros/installer_no_patches/microros_ws/firmware/mcu_ws/uros/rclc/rclc/src/rclc/executor.c:1836:13: warning: format '%ld' expects argument of type 'long int', but argument 5 has type 'size_t' {aka 'unsigned int'} [-Wformat=]
 1836 |             "Subscription added to wait_set_subscription[%ld] ",
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 1837 |             executor->handles[i].index);
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                 |
      |                                 size_t {aka unsigned int}
/b/mros/installer_no_patches/microros_ws/firmware/mcu_ws/uros/rclc/rclc/src/rclc/executor.c:1836:60: note: format string is defined here
 1836 |             "Subscription added to wait_set_subscription[%ld] ",
      |                                                          ~~^
      |                                                            |
      |                                                            long int
      |                                                          %d

%d work fine on me. maybe I should use %u, what do you think?

Correct the format string in rclc_executor_spin_some and rclc_timer_init_default

Signed-off-by: Chauncy Liu <chauncy@beechwoods.com>
@cliu5764
Copy link
Author

Can someone why the build failed? Thanks

@Zard-C
Copy link
Contributor

Zard-C commented Mar 14, 2024

Can someone why the build failed? Thanks

Check this out https://build.ros2.org/job/Hpr__rclc__ubuntu_jammy_amd64/37/clang-tidy/new/fileName.-1655778739/

Build Status

@cliu5764
Copy link
Author

Can someone why the build failed? Thanks

Check this out https://build.ros2.org/job/Hpr__rclc__ubuntu_jammy_amd64/37/clang-tidy/new/fileName.-1655778739/

Build Status

I checked out the console log, I have no idea why the build got the following errors. Anyone knows?

01:58:54 No credentials specified
01:58:54  > git rev-parse fe02b94aeff734c4e120d3a063c725191ad3c43e^{commit} # timeout=10
01:58:54 [GNU C Compiler (gcc)] [-ERROR-] Can't create fingerprints for some files:
01:58:54 [GNU C Compiler (gcc)] [-ERROR-] - '/tmp/ws/src/rclc/rclc/src/rclc/timer.c', IO exception has been thrown: java.nio.file.NoSuchFileException: /tmp/ws/src/rclc/rclc/src/rclc/timer.c
01:58:54 [GNU C Compiler (gcc)] [-ERROR-] - '/tmp/ws/src/rclc/rclc/src/rclc/executor.c', IO exception has been thrown: java.nio.file.NoSuchFileException: /tmp/ws/src/rclc/rclc/src/rclc/executor.c
01:58:54 [GNU C Compiler (gcc)] [-ERROR-] - '/tmp/ws/src/rclc/rclc/src/rclc/executor.c', IO exception has been thrown: java.nio.file.NoSuchFileException: /tmp/ws/src/rclc/rclc/src/rclc/executor.c
01:58:54 [GNU C Compiler (gcc)] [-ERROR-] - '/tmp/ws/src/rclc/rclc/src/rclc/executor.c', IO exception has been thrown: java.nio.file.NoSuchFileException: /tmp/ws/src/rclc/rclc/src/rclc/executor.c
01:58:54 [GNU C Compiler (gcc)] [-ERROR-] - '/tmp/ws/src/rclc/rclc/src/rclc/executor.c', IO exception has been thrown: java.nio.file.NoSuchFileException: /tmp/ws/src/rclc/rclc/src/rclc/executor.c
01:58:54 [GNU C Compiler (gcc)] [-ERROR-] - '/tmp/ws/src/rclc/rclc/src/rclc/executor.c', IO exception has been thrown: java.nio.file.NoSuchFileException: /tmp/ws/src/rclc/rclc/src/rclc/executor.c
01:58:54 [GNU C Compiler (gcc)] [-ERROR-] - '/tmp/ws/src/rclc/rclc/src/rclc/executor.c', IO exception has been thrown: java.nio.file.NoSuchFileException: /tmp/ws/src/rclc/rclc/src/rclc/executor.c
01:58:54 [GNU C Compiler (gcc)] [-ERROR-] - '/tmp/ws/src/rclc/rclc/src/rclc/executor.c', IO exception has been thrown: java.nio.file.NoSuchFileException: /tmp/ws/src/rclc/rclc/src/rclc/executor.c
01:58:54 [GNU C Compiler (gcc)] [-ERROR-] - '/tmp/ws/src/rclc/rclc/src/rclc/executor.c', IO exception has been thrown: java.nio.file.NoSuchFileException: /tmp/ws/src/rclc/rclc/src/rclc/executor.c
01:58:55 [GNU C Compiler (gcc)] Post processing issues on 'agent-1ec510a6' with source code encoding 'UTF-8'

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.

None yet

6 participants