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

Explicitly handle control characters #240

Merged
merged 1 commit into from
Sep 18, 2017
Merged

Explicitly handle control characters #240

merged 1 commit into from
Sep 18, 2017

Conversation

mptre
Copy link
Owner

@mptre mptre commented Sep 11, 2017

The current SIGINT handler is not considered safe since it invokes
functions that are not considered asynchronous safe[1]. Quoting the
CERT C Coding Standard:

In general, it is not safe to invoke I/O functions from within signal
handlers.

Instead, do not turn control characters into signal but instead handle
the relevant ones explicitly. A pleasant side-effect is being able to
test the Ctrl-C error path.

While here, fix the broken suspend/resume behavior by resetting the
initial terminal state prior suspending and re-read it upon resume. I do
actually use suspend/resume while running pick when I forgot to check
something that will affect the choice to select.

[1] https://www.securecoding.cert.org/confluence/display/c/SIG30-C.+Call+only+asynchronous-safe+functions+within+signal+handlers

@ghost
Copy link

ghost commented Sep 17, 2017

Great work! I hope we will see pick-1.9.0 with this soon ...

@calleluks
Copy link
Collaborator

Works great on macOS 10.12.6. Thanks for fixing this @mptre! Suspend/resume works like a charm now!

The current SIGINT handler is not considered safe since it invokes
functions that are not considered asynchronous safe[1]. Quoting the
CERT C Coding Standard:

  In general, it is not safe to invoke I/O functions from within signal
  handlers.

Instead, do not turn control characters into signal but instead handle
the relevant ones explicitly. A pleasant side-effect is being able to
test the Ctrl-C error path.

While here, fix the broken suspend/resume behavior by resetting the
initial terminal state prior suspending and re-read it upon resume. I do
actually use suspend/resume while running pick when I forgot to check
something that will affect the choice to select.

[1] https://www.securecoding.cert.org/confluence/display/c/SIG30-C.+Call+only+asynchronous-safe+functions+within+signal+handlers
@mptre mptre merged commit 9886750 into master Sep 18, 2017
@mptre mptre deleted the isig branch September 18, 2017 14:39
teoljungberg added a commit to calleluks/pick.vim that referenced this pull request Sep 20, 2017
In [pick#240] support was added to handle Ctrl-C, this commit fixes that
behavior in the vim plugin to not crash with an error when that happens.

[pick#240]: mptre/pick#240
@teoljungberg
Copy link
Contributor

I opened calleluks/pick.vim#21 to resolve this in pick.vim too.

teoljungberg added a commit to calleluks/pick.vim that referenced this pull request Sep 25, 2017
In [pick#240] support was added to handle Ctrl-C, this commit fixes that
behavior in the vim plugin to not crash with an error when that happens.

[pick#240]: mptre/pick#240
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