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

tcsh is unusable when $status is read-only #40

Open
GabrielRavier opened this issue Aug 24, 2021 · 9 comments
Open

tcsh is unusable when $status is read-only #40

GabrielRavier opened this issue Aug 24, 2021 · 9 comments

Comments

@GabrielRavier
Copy link

I don't know whether to put this in the previous issue, but this seems different enough to put it into a new one, even if it is directly related.

Anyway, doing set -r status makes tcsh completely unusable afterwards, i.e. a session looks like this:

> set -r status
> echo a
set: $status is read-only.
> /bin/echo a
set: $status is read-only.
> non-existing-command
set: $status is read-only.
> exit
set: $status is read-only.
> 
@suominen
Copy link
Member

What is the behaviour you are hoping to get from set -r status?

@GabrielRavier
Copy link
Author

GabrielRavier commented Aug 24, 2021

Well, probably something else than the current behavior. There are a few things that would be possible, in my opinion:

  1. Give an error when trying to make $status read-only, as that breaks everything
  2. Make it just freeze $status as a whole, and avoid giving random errors relating to automatic updates of it (especially in the sense that it makes the whole shell completely and utterly unusable as commands don't run at all)
  3. Have it prevent user changes to $status but not built-in changes like the one that occurs after every command

I personally don't like 3 much but 1 and 2 seem like they could both be practical in some way, at least certainly better than the current behavior (I personally have a slight preference for 2, but it's not for me to decide).

@suominen
Copy link
Member

Without a solid use-case for set -r status it doesn't seem reasonable to special case it in the code.

To me set -r status is not a sensible thing to do. I was trying to understand what you are trying to achieve. If a non-sensible action results in an unusable shell, my inclination would just be to recommend against taking the non-sensible action. :)

@GabrielRavier
Copy link
Author

GabrielRavier commented Aug 24, 2021

Well, even if the other possible use-cases I presented here don't seem very useful (though they'd all be made possible by one of the simple possible fixes to avoid breaking the shell), there's most certainly a solid use-case for having it at least not break the shell, in the same way there's a solid use-case for fixing other bugs that break or crash the shell (at least, I certainly hope you consider that worth doing). So the least that should be done, in my opinion, is making it not break the shell by either having the shell ignore errors from read-only $status or by preventing the user from making it read-only in the first place.

@suominen
Copy link
Member

I don't think the user shoud set -r status to begin with. From the manual page:

Read-only variables may not be modified or unset; attempting to do so will cause an error. Once made read-only, a variable cannot be made writable, so `set -r' should be used with caution.

If you accidentally set -r status, you can exit the shell and start a new one to recover.

In my mind this falls into the sudo rm -rf / category of things: the user needs to consider if their actions are reasonable or desirable for the state of the system.

It would seem to me that tcsh is now behaving as documented: it is displaying the error that results from making $status read-only (as opposed to becoming catatonic and/or crashing).

@GabrielRavier
Copy link
Author

GabrielRavier commented Aug 24, 2021

How is being unable to execute any commands at all (even exit) not "becoming catatonic" ? I've tried to test a few things, and apart from triggering syntax errors, putting tcsh in this state seems to just makes it a "set: $status is read-only" dispenser

@GabrielRavier
Copy link
Author

GabrielRavier commented Aug 24, 2021

It would seem to me that tcsh is now behaving as documented: it is displaying the error that results from making $status read-only

This doesn't actually seem true: It isn't mentioned anywhere that the shell will execute the equivalent of set status=0 before every command and will thus fail to execute anything at all if status is read-only, at best you could potentially infer it as a potential implementation strategy as it is said that "The shell updates [...] status when necessary" but not much else

@zoulasc
Copy link
Member

zoulasc commented Aug 25, 2021

I guess that a possible fix is to special-case $status and only warn if it is read-only.

@Jamie-Landeg-Jones
Copy link

I just tried testing this. I guess it's because I have precmd set, but I just got the error that status is read-only repeating in an endless loop until tcsh coredumped with SIGSEGV after about a minute!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants