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.

Sign up for GitHub

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 buffer size declaration in syspapi #1366

Closed
wants to merge 1 commit into from
Closed

Fix buffer size declaration in syspapi #1366

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ldms/src/sampler/syspapi/syspapi_sampler.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ typedef struct syspapi_metric_s {
int init_rc; /* if 0, attr is good for perf_event_open() */
struct perf_event_attr attr; /* perf attribute */
char papi_name[256]; /* metric name in PAPI */
char pfm_name[256]; /* metric name in perfmon (PAPI native) */
char pfm_name[PAPI_HUGE_STR_LEN]; /* metric name in perfmon (PAPI native) */
Copy link
Collaborator

Choose a reason for hiding this comment

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

@baallan, I don't believe that this field is used anywhere. We should just remove it instead of consuming 1k for every papi metric.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@tom95858 it is copied at line 251 into a buffer which is just 256 bytes while the papi source string is up to 1024. The alternative is to make the structure's pfm_name field separately allocated and freed, which would be a much bigger change.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@baallan, after it is copied in there it is never used again. My suggestion is to just remove the field instead of wasting 1k for every event.

int pfd[]; /* one perf fd per CPU */
} *syspapi_metric_t;

Expand Down
Loading