Skip to content

lmsensors component: Replace fprintf with SUBDBG#389

Merged
Treece-Burgess merged 1 commit intoicl-utk-edu:masterfrom
Treece-Burgess:05-22-2025-lmsensors-subdbg
May 27, 2025
Merged

lmsensors component: Replace fprintf with SUBDBG#389
Treece-Burgess merged 1 commit intoicl-utk-edu:masterfrom
Treece-Burgess:05-22-2025-lmsensors-subdbg

Conversation

@Treece-Burgess
Copy link
Contributor

@Treece-Burgess Treece-Burgess commented May 22, 2025

Pull Request Description

Currently in the lmsensors component there are two fprintf calls. This can lead to error messages printing even when you do not set PAPI_DEBUG. As an example, on an AMD Epyc 7413 with lmsensors 3.4 the following will occur:

[tburgess@gilgamesh bin]$ ./papi_command_line lmsensors:::amdgpu-pci-e300.mem.temp3_crit

This utility lets you add events from the command line interface to see if they work.

libsensors(): Could not read event #26!
.
.
.
libsensors(): Could not read event #75!
Successfully added: lmsensors:::amdgpu-pci-e300.mem.temp3_crit

lmsensors:::amdgpu-pci-e300.mem.temp3_crit : 	94000 

This PR resolves this issue by replacing the fprintf's with SUBDBG's (Tested on an AMD Epyc 7413 with lmsensors 3.4).

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

@Treece-Burgess Treece-Burgess added status-ready-for-review PR is ready to be reviewed type-bug Issues discussing bugs or PRs fixing bugs labels May 22, 2025
@dbarry9
Copy link
Contributor

dbarry9 commented May 22, 2025

I am testing this pull request.

@dbarry9
Copy link
Contributor

dbarry9 commented May 23, 2025

On Gilgamesh using GCC 12.2.0, I get the following compiler warnings:

components/lmsensors/linux-lmsensors.c: In function 'createNativeEvents':
components/lmsensors/linux-lmsensors.c:204:18: warning: passing argument 2 of 'fprintf' from incompatible pointer type [-Wincompatible-pointer-types]
  204 |                  SUBDBG( stderr, "ERROR: Can't get label of feature %s!\n", feature->name );
      |                  ^~~~~~
      |                  |
      |                  FILE *
In file included from /home/users/dbarry/apps/lmsensors/include/sensors/sensors.h:25,
                 from components/lmsensors/linux-lmsensors.c:32:
/usr/include/stdio.h:327:44: note: expected 'const char * restrict' but argument is of type 'FILE *'
  327 |                     const char *__restrict __format, ...);
      |                     ~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~
components/lmsensors/linux-lmsensors.c: In function 'getEventValue':
components/lmsensors/linux-lmsensors.c:262:17: warning: passing argument 2 of 'fprintf' from incompatible pointer type [-Wincompatible-pointer-types]
  262 |                 SUBDBG( stderr, "libsensors(): Could not read event #%d!\n", event_id );
      |                 ^~~~~~
      |                 |
      |                 FILE *
/usr/include/stdio.h:327:44: note: expected 'const char * restrict' but argument is of type 'FILE *'
  327 |                     const char *__restrict __format, ...);
      |                     ~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~

I believe this can remedied by removing the stderr arguments.

@Treece-Burgess Treece-Burgess force-pushed the 05-22-2025-lmsensors-subdbg branch from 5d538f5 to 17e31b4 Compare May 23, 2025 19:12
@Treece-Burgess
Copy link
Contributor Author

On Gilgamesh using GCC 12.2.0, I get the following compiler warnings:

components/lmsensors/linux-lmsensors.c: In function 'createNativeEvents':
components/lmsensors/linux-lmsensors.c:204:18: warning: passing argument 2 of 'fprintf' from incompatible pointer type [-Wincompatible-pointer-types]
  204 |                  SUBDBG( stderr, "ERROR: Can't get label of feature %s!\n", feature->name );
      |                  ^~~~~~
      |                  |
      |                  FILE *
In file included from /home/users/dbarry/apps/lmsensors/include/sensors/sensors.h:25,
                 from components/lmsensors/linux-lmsensors.c:32:
/usr/include/stdio.h:327:44: note: expected 'const char * restrict' but argument is of type 'FILE *'
  327 |                     const char *__restrict __format, ...);
      |                     ~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~
components/lmsensors/linux-lmsensors.c: In function 'getEventValue':
components/lmsensors/linux-lmsensors.c:262:17: warning: passing argument 2 of 'fprintf' from incompatible pointer type [-Wincompatible-pointer-types]
  262 |                 SUBDBG( stderr, "libsensors(): Could not read event #%d!\n", event_id );
      |                 ^~~~~~
      |                 |
      |                 FILE *
/usr/include/stdio.h:327:44: note: expected 'const char * restrict' but argument is of type 'FILE *'
  327 |                     const char *__restrict __format, ...);
      |                     ~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~

I believe this can remedied by removing the stderr arguments.

@dbarry9 Thank you for catching that, yes that is correct. I have updated the PR and tested on Gilgamesh myself with GCC 12.2.0 and do not notice the compiler warning now. Please let me know if it is the same for yourself.

@dbarry9
Copy link
Contributor

dbarry9 commented May 23, 2025

@Treece-Burgess That resolved the compiler warning on my end, too.

I have now tested this pull request on Gilgamesh (AMD Zen3 + NVIDIA H100 + AMD MI250X) and Frontier (AMD Zen3 + MI250X). The utilities work as expected, and the lmsensors component tests' output is the same as in the master branch. Although, as an unrelatd issue, the lmsensors_read component test currently produces an error in the master branch and the branch in this PR:

Component index for lmsensors: 2
PAPI Error: Error Code -17,Component Index isn't set
FAILED!!!
Line # 95 Error in PAPI_start: Component Index isn't set
Some tests require special hardware, permissions, OS, compilers
or library versions. PAPI may still function perfectly on your
system without the particular feature being tested here.

@Treece-Burgess
Copy link
Contributor Author

@Treece-Burgess That resolved the compiler warning on my end, too.

I have now tested this pull request on Gilgamesh (AMD Zen3 + NVIDIA H100 + AMD MI250X) and Frontier (AMD Zen3 + MI250X). The utilities work as expected, and the lmsensors component tests' output is the same as in the master branch. Although, as an unrelatd issue, the lmsensors_read component test currently produces an error in the master branch and the branch in this PR:

Component index for lmsensors: 2
PAPI Error: Error Code -17,Component Index isn't set
FAILED!!!
Line # 95 Error in PAPI_start: Component Index isn't set
Some tests require special hardware, permissions, OS, compilers
or library versions. PAPI may still function perfectly on your
system without the particular feature being tested here.

I will take a look at the lmsensors_read test to see why it is failing.

@Treece-Burgess Treece-Burgess force-pushed the 05-22-2025-lmsensors-subdbg branch from 17e31b4 to c7f4503 Compare May 27, 2025 01:25
@Treece-Burgess
Copy link
Contributor Author

@Treece-Burgess That resolved the compiler warning on my end, too.

I have now tested this pull request on Gilgamesh (AMD Zen3 + NVIDIA H100 + AMD MI250X) and Frontier (AMD Zen3 + MI250X). The utilities work as expected, and the lmsensors component tests' output is the same as in the master branch. Although, as an unrelatd issue, the lmsensors_read component test currently produces an error in the master branch and the branch in this PR:

Component index for lmsensors: 2
PAPI Error: Error Code -17,Component Index isn't set
FAILED!!!
Line # 95 Error in PAPI_start: Component Index isn't set
Some tests require special hardware, permissions, OS, compilers
or library versions. PAPI may still function perfectly on your
system without the particular feature being tested here.

For documentation purposes, I will be creating a separate PR to handle the error of the lmsensors_read component test.

@Treece-Burgess Treece-Burgess merged commit e53a456 into icl-utk-edu:master May 27, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status-ready-for-review PR is ready to be reviewed type-bug Issues discussing bugs or PRs fixing bugs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants