Skip to content

pcm-bw-histogram.sh: expect pcm-memory to reside in same folder, rather than ./#961

Merged
rdementi merged 1 commit intointel:masterfrom
tomcucinotta:master
Jun 25, 2025
Merged

pcm-bw-histogram.sh: expect pcm-memory to reside in same folder, rather than ./#961
rdementi merged 1 commit intointel:masterfrom
tomcucinotta:master

Conversation

@tomcucinotta
Copy link
Copy Markdown
Contributor

As pcm-bw-histogram.sh is installed (renamed to pcm-bw-histogram) alongside the pcm-memory binary in sbin/, pcm-memory should be found in the same folder as the script, rather than the current folder. Without this patch, pcm-bw-histogram cannot be launched (as it cannot find ./pcm-memory), unless you cd to the sbin/ folder. This is inconvenient, as the script creates a temporary file, as well as an output .csv file, in the current folder. With this patch, you can just launch pcm-bw-histogram from any folder, and it will find pcm-memory in the same folder where it's been installed.

@rdementi rdementi requested a review from Copilot June 25, 2025 08:56

This comment was marked as outdated.

Copy link
Copy Markdown
Contributor

@rdementi rdementi left a comment

Choose a reason for hiding this comment

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

suggestions from copilot make sense. Please implement them.

…ther than ./

As pcm-bw-histogram.sh is installed (renamed to pcm-bw-histogram) alongside the pcm-memory binary in sbin/, `pcm-memory` should be found in the same folder as the script, rather than the current folder.
Without this patch, pcm-bw-histogram cannot be launched (as it cannot find ./pcm-memory), unless you cd to the sbin/ folder. This is inconvenient, as the script creates a temporary file, as well as an output .csv file, in the current folder.
With this patch, you can just launch pcm-bw-histogram from any folder, and it will find pcm-memory in the same folder where it's been installed.
@tomcucinotta
Copy link
Copy Markdown
Contributor Author

done, in updated (forced) commit 9f82188 in my forked master, thanks!

@rdementi rdementi requested a review from Copilot June 25, 2025 12:12
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the pcm-bw-histogram script to ensure it finds the pcm-memory binary in the same folder as the script, rather than relying on the current working directory.

  • Introduces the mydir variable to hold the script's directory.
  • Updates all pcm-memory invocations to use "$mydir/pcm-memory".
Comments suppressed due to low confidence (1)

src/pcm-bw-histogram.sh:14

  • [nitpick] Consider renaming 'mydir' to 'script_dir' to improve readability and better convey its purpose.
mydir=$(dirname "$0")

Copy link
Copy Markdown
Contributor

@rdementi rdementi left a comment

Choose a reason for hiding this comment

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

thank you!

@rdementi rdementi merged commit 1668c72 into intel:master Jun 25, 2025
29 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.

3 participants