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

Jump to bottom

fortune: bad assumption in find_path() #963

Merged
merged 1 commit into from
Feb 25, 2025
Merged

fortune: bad assumption in find_path() #963

merged 1 commit into from
Feb 25, 2025

Conversation

mknos
Copy link
Contributor

@mknos mknos commented Feb 25, 2025 •
edited
Loading

  • This version of fortune can be passed a file argument, which will be used as the fortune file
  • In find_path(), if the direct path check fails a loop is entered where the file argument is appended to each search directory
  • When the file argument is an absolute path, entering this loop is incorrect because the absolute path will be prefixed with the search directory path
  • fortune's debug (-d) option makes the file search easy to see
  • While here, use catfile() to join relative paths with the search directory
%perl fortune -d /dev/null # BEFORE: 2nd is_fortune_file() call shouldn't happen
opts are:
d == 1
Building file list. Containers are Percent specified and Percent unspecified

@ARGV (1 arguments):
/dev/null


trying to add item /dev/null to Percent unspecified
is_fortune_file(/dev/null) returns FALSE (can't read file)
is_fortune_file(/home/x/PerlPowerTools/fortunes//dev/null) returns FALSE (can't read file)
fortune: /dev/null not a fortune file or directory


%perl fortune -d /dev/null # AFTER: single is_fortune_file() call for abs path
opts are:
d == 1
Building file list. Containers are Percent specified and Percent unspecified

@ARGV (1 arguments):
/dev/null


trying to add item /dev/null to Percent unspecified
is_fortune_file(/dev/null) returns FALSE (can't read file)
fortune: /dev/null not a fortune file or directory

%perl fortune -d heh # AFTER: relpath converted to absolute in 2nd is_fortune_file() call
opts are:
d == 1
Building file list. Containers are Percent specified and Percent unspecified

@ARGV (1 arguments):
heh


trying to add item heh to Percent unspecified
is_fortune_file(heh) returns FALSE (can't read file)
is_fortune_file(/home/x/PerlPowerTools/fortunes/heh) returns FALSE (can't read file)
fortune: heh not a fortune file or directory

@mknos
fortune: bad assumption in find_path()
8012d36 
* This version of fortune can be passed a file argument, which will be used as the fortune file
* In find_path(), if the direct path check fails a loop is entered where the file argument is appended to each search directory
* When the file argument is an absolute path, entering this loop is incorrect because the absolute path will be prefixed with the search directory path
* fortune's debug (-d) option makes the file search easy to see
* While here, use catfile() to join relative paths with the search directory
@github-actions github-actions bot added Type: enhancement improve a feature that already exists Priority: low get to this whenever Program: fortune The fortune program labels Feb 25, 2025
@mknos mknos temporarily deployed to automated_testing February 25, 2025 03:05 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 03:05 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 03:05 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 03:05 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 03:05 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 03:05 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 03:05 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 03:05 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 03:05 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 03:05 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 03:05 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 03:05 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 03:05 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 03:05 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 03:05 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 03:05 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 03:05 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 03:05 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 03:05 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing February 25, 2025 03:05 — with GitHub Actions Inactive
@coveralls
Copy link

coveralls commented Feb 25, 2025 •
edited
Loading

Pull Request Test Coverage Report for Build 13512727840

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 72.349%
Totals Coverage Status
Change from base Build 13503527620: 0.0%
Covered Lines: 348
Relevant Lines: 481
💛 - Coveralls

@briandfoy briandfoy self-assigned this Feb 25, 2025
@briandfoy briandfoy merged commit 55fe7b2 into briandfoy:master Feb 25, 2025
23 checks passed
@github-actions github-actions bot added the Status: accepted The fix is accepted label Feb 25, 2025
@github-actions github-actions bot added the Status: accepted The fix is accepted label Feb 25, 2025
@briandfoy
Copy link
Owner

changes: do not look for an absolute path in the fortune directories

@briandfoy briandfoy removed the Priority: low get to this whenever label Feb 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@briandfoy briandfoy briandfoy approved these changes

Assignees

@briandfoy briandfoy

Labels
Program: fortune The fortune program Status: accepted The fix is accepted Type: enhancement improve a feature that already exists
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mknos @coveralls @briandfoy