fix: c99 array designators used in C++ #1693
Conversation
Signed-off-by: Federico Aponte <[email protected]>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: federico-sysdig The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| @@ -71,6 +71,7 @@ limitations under the License. | |||
| #include <libsinsp/sinsp_suppress.h> | |||
| #include <libsinsp/state/table_registry.h> | |||
| #include <libsinsp/stats.h> | |||
| #include <libsinsp/stats_v2_resource_utilization.h> | |||
There was a problem hiding this comment.
@federico-sysdig suggestion instead of touching the stats and adding a new header file, would it be ok to move those changes to the refactor PR we have here: #1652 Open to however you would like to go about it.
There was a problem hiding this comment.
Absolutely. I am open to several ways to proceed. If you want to integrate these changes into the other PR, I can close this as superseded. I have no interest on the "paternity" of this change.
There was a problem hiding this comment.
Great let's do this then. If you leave this commit and add another one on top we can come back to it later in the other PR. Ty!
There was a problem hiding this comment.
I'm lost. What should I do? I can let this PR lag behind the other one and merge later on once it has been merged. Or you can integrate some of these changes into the other PR. Or what?
There was a problem hiding this comment.
If ok, revert anything that touched the stats, but ok to leave it in the history. I can port them over to the other PR later. That way we do not block all other changes here and keep the larger metrics refactor in one place.
There was a problem hiding this comment.
There is an array initializer in stats.cpp that uses c99 array designators, if I revert that I also need to revert the warning suppression and the PR would lose its purpose and a second PR would be needed.
This PR isn't critical. It can wait as much time as #1652 needs to be merged. I'll then patch this up for the new situation and ask you to review. Take your time, I've put this as draft for the moment.
There was a problem hiding this comment.
ok let's do this then, ty!
|
/hold until #1652 is merged. |
|
Issues go stale after 90d of inactivity. Mark the issue as fresh with Stale issues rot after an additional 30d of inactivity and eventually close. If this issue is safe to close now please do so with Provide feedback via https://github.com/falcosecurity/community. /lifecycle stale |
|
Stale issues rot after 30d of inactivity. Mark the issue as fresh with Rotten issues close after an additional 30d of inactivity. If this issue is safe to close now please do so with Provide feedback via https://github.com/falcosecurity/community. /lifecycle rotten |
|
/remove-lifecycle rotten |
|
@federico-sysdig given the changes @FedeDP and @Molter73 made to the libs metrics collector, would you mind reviewing the current code in master again and let us know what changes would still be good from your perspective? Thank you! |
@federico-sysdig any news? Ty! |
|
Issues go stale after 90d of inactivity. Mark the issue as fresh with Stale issues rot after an additional 30d of inactivity and eventually close. If this issue is safe to close now please do so with Provide feedback via https://github.com/falcosecurity/community. /lifecycle stale |
|
/remove-lifecycle stale |
What type of PR is this?
/kind cleanup
Any specific area of the project related to this PR?
/area build
Does this PR require a change in the driver versions?
No
What this PR does / why we need it:
Fixes c99 array designators used in C++ and causing a warning. There is a warning suppression for these few cases that is also removed. As some of these arrays were defined in a header file, which can be potentially included in a C++ source file, the definition has been split from the declaration and moved to a new C file. Some re-arrangement of the enums used for the array designators was also needed.
I noticed also some global variables defined in a header file. They are now declared as
inlineto avoid potential duplication when the header is used in more than one source file.Lastly, the compiler was warning about an unused data member that has been removed.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?: