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

Sysdetect: fix snprintf behavior #42

Merged

Conversation

gcongiu
Copy link
Contributor

@gcongiu gcongiu commented Jul 6, 2023

Pull Request Description

The sysdetect component makes a wrong usage of snprintf. In a few instances, snprintf is used to copy a string of size X to a string of size PAPI_MAX_STR_LEN. However, the sysdetect code sets the n argument of snprintf, indicating the target string length, to the length of the source string (i.e., X). Additionally, sometimes the code null-terminates the target string, which is not necessary because snprintf always null-terminates the target string.

Author Checklist

  • Description
    Why this PR exists. Reference all relevant information, including background, issues, test failures, etc
  • Commits
    Commits are self contained and only do one thing
    Commits have a header of the form: module: short description
    Commits have a body (whenever relevant) containing a detailed description of the addressed problem and its solution
  • Tests
    The PR needs to pass all the tests

@gcongiu gcongiu changed the title Null terminate snprintf strings Fix snprintf behavior in sysdetect component Jul 6, 2023
@gcongiu gcongiu added the do not merge The PR should not be merged label Jul 6, 2023
@gcongiu gcongiu force-pushed the 2023.07.06_sysdetect-snprintf-fix branch from a20c0e3 to 30c3c90 Compare July 6, 2023 16:31
@gcongiu gcongiu removed the do not merge The PR should not be merged label Jul 6, 2023
@gcongiu gcongiu requested a review from adanalis July 7, 2023 08:25
@gcongiu gcongiu changed the title Fix snprintf behavior in sysdetect component Sysdetect: Fix snprintf behavior Jul 7, 2023
@gcongiu gcongiu changed the title Sysdetect: Fix snprintf behavior Sysdetect: fix snprintf behavior Jul 7, 2023
@gcongiu gcongiu force-pushed the 2023.07.06_sysdetect-snprintf-fix branch from f490b07 to 7d12543 Compare July 12, 2023 09:38
@gcongiu gcongiu force-pushed the 2023.07.06_sysdetect-snprintf-fix branch from 7d12543 to b3503ba Compare August 30, 2023 14:00
@gcongiu
Copy link
Contributor Author

gcongiu commented Aug 30, 2023

@jagode can you approve?

@gcongiu gcongiu requested a review from jagode August 30, 2023 14:00
snprintf will always null terminate the output string regarless
characters from input string being dropped (i.e. if the output string is
shorter than the input string).
snprintf will always null terminate the output string regarless
characters from input string being dropped (i.e. if the output string is
shorter than the input string).
The n argument in snprintf specifies the length of the output string not
the one of the input string.
The n argument in snprintf specifies the length of the output string not
the one of the input string.
@gcongiu gcongiu force-pushed the 2023.07.06_sysdetect-snprintf-fix branch from b3503ba to f77f787 Compare September 7, 2023 17:33
@gcongiu gcongiu merged commit b9e9fc9 into icl-utk-edu:master Sep 7, 2023
12 of 14 checks passed
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.

2 participants