Skip to content

Reject making LINENO, OLDPWD, OPTARG, OPTIND, and PWD readonly #170

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

Merged
merged 1 commit into from
May 12, 2025

Conversation

magicant
Copy link
Owner

POSIX.1-2024 XBD 8.1 clarifies the allowed behavior of the shell when the following variables are made read-only:

  • LINENO
  • OLDPWD
  • OPTARG
  • OPTIND
  • PWD

The readonly built-in now refuses to make the above variables read-only in the POSIXly-correct mode because it is the most restrictive behavior allowed by the standard.

Summary by Copilot

This pull request introduces changes to prevent specific variables from being made read-only in POSIXly-correct mode. The updates include modifications to documentation, tests, and the codebase to enforce this restriction and provide appropriate error handling.

Changes to functionality:

  • Updated the readonly built-in to disallow making the variables $LINENO, $OLDPWD, $OPTARG, $OPTIND, and $PWD read-only in POSIXly-correct mode. [1] [2]

Documentation updates:

  • Updated doc/_readonly.txt and doc/posix.txt to reflect the restriction on making specific variables read-only in POSIXly-correct mode. [1] [2]
  • Updated the corresponding Japanese documentation files, doc/ja/_readonly.txt and doc/ja/posix.txt, with the same information. [1] [2]

Test additions:

  • Added new test cases in tests/readonly-y.tst to verify that attempting to make the restricted variables read-only results in appropriate error messages.

Codebase enhancements:

  • Introduced a new helper function can_make_readonly in variable.c to determine if a variable can be made read-only based on the POSIXly-correct mode. [1] [2]

News updates:

  • Updated the NEWS and NEWS.ja files to document the new behavior of the readonly built-in in POSIXly-correct mode. [1] [2]

@magicant magicant added this to the 2.59 milestone May 12, 2025
@magicant magicant requested a review from Copilot May 12, 2025 13:58
@magicant magicant self-assigned this May 12, 2025
@magicant magicant added the enhancement New feature or request label May 12, 2025
Copy link

@Copilot 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 pull request updates the shell’s behavior in POSIXly-correct mode by preventing certain internal variables from being made read-only. Key changes include introducing a helper function (can_make_readonly) to check allowed variables, updating the typeset built-in error handling, and revising documentation and tests accordingly.

Files not reviewed (7)
  • NEWS: Language not supported
  • NEWS.ja: Language not supported
  • doc/_readonly.txt: Language not supported
  • doc/ja/_readonly.txt: Language not supported
  • doc/ja/posix.txt: Language not supported
  • doc/posix.txt: Language not supported
  • tests/readonly-y.tst: Language not supported
Comments suppressed due to low confidence (1)

variable.c:1760

  • [nitpick] Consider enhancing the error message with additional context (e.g. indicating that the restriction is due to POSIXly-correct mode) to improve user feedback.
xerror(0, Ngt("$%ls cannot be read-only"), arg);

POSIX.1-2024 XBD 8.1 clarifies the allowed behavior of the shell when
the following variables are made read-only:

- LINENO
- OLDPWD
- OPTARG
- OPTIND
- PWD

The readonly built-in now refuses to make the above variables read-only
in the POSIXly-correct mode because it is the most restrictive behavior
allowed by the standard.
@magicant magicant changed the title Keep LINENO, OLDPWD, OPTARG, OPTIND, and PWD writable Reject making LINENO, OLDPWD, OPTARG, OPTIND, and PWD readonly May 12, 2025
@magicant magicant mentioned this pull request May 12, 2025
56 tasks
@magicant magicant merged commit da18740 into trunk May 12, 2025
5 checks passed
@magicant magicant deleted the readonly branch May 12, 2025 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant