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

core: cast variables to avoid fpermissive errors #7456

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

Conversation

Minipada
Copy link

@Minipada Minipada commented May 22, 2023

While using the C API for another project, I encountered some -permissive errors.
To simply put it, some statements should be casted and this is what I did in this PR.

errors:

/root/ros2_data_collection/install/fluent_bit_vendor/include/fluent-bit/flb_output.h:698:40: error: invalid conversion from ‘void*’ to ‘char*’ [-fpermissive]
  698 |                             flb_realloc(p_buf, p_size + serialized_context_size);
      |                             ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                        |
      |                                        void*
/root/ros2_data_collection/install/fluent_bit_vendor/include/fluent-bit/flb_output.h:736:49: error: invalid conversion from ‘void*’ to ‘char*’ [-fpermissive]
  736 |                                                 p_buf,
      |                                                 ^~~~~
      |                                                 |
      |                                                 void*

/root/ros2_data_collection/install/fluent_bit_vendor/include/fluent-bit/flb_input.h:476:33: error: invalid conversion from ‘void*’ to ‘flb_libco_in_params*’ [-fpermissive]
  476 |     params = pthread_getspecific(libco_in_param_key);
      |              ~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~
      |                                 |
      |                                 void*
/root/ros2_data_collection/install/fluent_bit_vendor/include/fluent-bit/flb_input.h:478:28: error: invalid conversion from ‘void*’ to ‘flb_libco_in_params*’ [-fpermissive]
  478 |         params = flb_calloc(1, sizeof(struct flb_libco_in_params));
      |                  ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                            |
      |                            void*
/root/ros2_data_collection/install/fluent_bit_vendor/include/fluent-bit/flb_input.h: In function ‘void input_pre_cb_collect()’:
/root/ros2_data_collection/install/fluent_bit_vendor/include/fluent-bit/flb_input.h:500:33: error: invalid conversion from ‘void*’ to ‘flb_libco_in_params*’ [-fpermissive]
  500 |     params = pthread_getspecific(libco_in_param_key);
      |              ~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~
      |                                 |
      |                                 void*
/root/ros2_data_collection/install/fluent_bit_vendor/include/fluent-bit/flb_input.h:502:28: error: invalid conversion from ‘void*’ to ‘flb_libco_in_params*’ [-fpermissive]
  502 |         params = flb_calloc(1, sizeof(struct flb_libco_in_params));
      |                  ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                            |
      |                            void*


Testing
Before we can approve your change; please submit the following in a comment:

  • Example configuration file for the change
  • Debug log output from testing the change
  • Attached Valgrind output that shows no leaks or memory corruption was found

If this is a change to packaging of containers or native binaries then please confirm it works for all targets.

  • Run local packaging test showing all targets (including any new ones) build.
  • Set ok-package-test label to test for all targets (requires maintainer to do).

Documentation

  • Documentation required for this feature

Backporting

  • Backport to latest stable release.

Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.

@Minipada
Copy link
Author

if (params == NULL) {
params = flb_calloc(1, sizeof(struct flb_libco_in_params));
params = (struct flb_libco_in_params *) flb_calloc(1, sizeof(struct flb_libco_in_params));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you please break this and other lines that exceed the 80 column limit?

Copy link
Author

Choose a reason for hiding this comment

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

Hi, I just pushed some changes. I am not sure if I needed the backslash before the newline but it compiles.

@leonardo-albertovich
Copy link
Collaborator

Thanks a lot for the contribution!

As for that monkey message, that was definitively not intentional. It seems like I missed that debug artifact but I'll open a PR to remove it now. Thank you for pointing it out!

@@ -695,7 +695,7 @@ struct flb_output_flush *flb_output_flush_create(struct flb_task *task,
if ((serialization_buffer_offset +
serialized_context_size) > p_size) {
resized_serialization_buffer = \
flb_realloc(p_buf, p_size + serialized_context_size);
(char*) flb_realloc(p_buf, p_size + serialized_context_size);
Copy link
Author

Choose a reason for hiding this comment

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

I am not sure how I could do less than 80 chars here so i did not touch

@Minipada Minipada temporarily deployed to pr May 23, 2023 09:00 — with GitHub Actions Inactive
@Minipada Minipada temporarily deployed to pr May 23, 2023 09:00 — with GitHub Actions Inactive
@Minipada Minipada temporarily deployed to pr May 23, 2023 09:00 — with GitHub Actions Inactive
@Minipada Minipada temporarily deployed to pr May 23, 2023 09:25 — with GitHub Actions Inactive
Signed-off-by: David Bensoussan <d.bensoussan@proton.me>
@Minipada Minipada temporarily deployed to pr May 23, 2023 13:49 — with GitHub Actions Inactive
@Minipada Minipada temporarily deployed to pr May 23, 2023 13:49 — with GitHub Actions Inactive
@Minipada Minipada temporarily deployed to pr May 23, 2023 13:49 — with GitHub Actions Inactive
@Minipada Minipada changed the title cast variables to avoid fpermissive errors core: cast variables to avoid fpermissive errors May 23, 2023
@Minipada Minipada temporarily deployed to pr May 23, 2023 14:17 — with GitHub Actions Inactive
@Minipada
Copy link
Author

So, I was quite confused about this test result and I have the same result on my pc, also with the master branch...

My system: Ubuntu 22.04

Commands:

export CC=/usr/bin/clang
export CXX=/usr/bin/clang++

cmake -DFLB_TESTS_INTERNAL=On -DFLB_TESTS_RUNTIME=On -DFLB_EXAMPLES=Off -DFLB_OUT_PGSQL=On -DFLB_SHARED_LIB=On -DFLB_PROXY_GO=On -DSYSTEMD_UNITDIR=/usr/lib/systemd/system -DSANITIZE_ADDRESS=On ..

make -j

Could you confirm this is an upstream bug?

@leonardo-albertovich
Copy link
Collaborator

Which test result?

@Minipada
Copy link
Author

Sorry, the one failing in CI:

https://github.com/fluent/fluent-bit/actions/runs/5055636573/jobs/9077785996?pr=7456

I tried locally with:
./bin/flb-it-stream_processor

@leonardo-albertovich
Copy link
Collaborator

From what I can see in that log it doesn't sound like it's related to your PR but we should test master at head and the previous tags so we find the offending PR and fix it.

I won't be able to do that today but it would be really helpful if you tested those.

@Minipada
Copy link
Author

Sorry if I was not clear but I already ran the test and it failed on master :/

@leonardo-albertovich
Copy link
Collaborator

Yes, I expected it to fail on master, I wanted to get to the bottom of it but I guess it doesn't really matter for this PR.

@Minipada
Copy link
Author

Is there anything else blocking to move forward with this PR?

@leonardo-albertovich
Copy link
Collaborator

leonardo-albertovich commented May 29, 2023

Not on my side, I have already approved it and @edsiper will probably merge it within the next release cycle.

Copy link
Contributor

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the Stale label Dec 10, 2023
@github-actions github-actions bot removed the Stale label Aug 16, 2024
Copy link
Contributor

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the Stale label Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants