Skip to content

Add critical-section dependency and signal module #20

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

Closed

Conversation

gmmyung
Copy link
Contributor

@gmmyung gmmyung commented Feb 8, 2024

This adds Rust implementation of signal, raise, and abort to ANSI C signals: SIGTERM, SIGSEGV, SIGINT, SIGILL, SIGABRT, SIGFPE.

A few caveats:

  • core::intrinsics::abort() is unstable, so the default handler panics.
  • Technically, SIGSEGV, SIGILL, SIGFPE can be implemented using exception handlers, but I am not sure if this breaks things when used in other crates.

@@ -9,9 +9,11 @@ readme = "README.md"
repository = "https://github.com/thejpster/tinyrlibc"

[dependencies]
critical-section = "1.1.2"
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be an optional dependency?

@thejpster
Copy link
Member

OK, I had a look at this, and I had a look at the libc crate, and I think we can do this with atomic usize instead.

@thejpster
Copy link
Member

How do you feel about #21 as an alternative?

@gmmyung
Copy link
Contributor Author

gmmyung commented Feb 9, 2024

Yes, #21 seems like a better implementation. Just one thing, there is a race condition while testing. So each test should be encapsulated in a critical section, or the test should be ran with—test-threads=1.

@gmmyung
Copy link
Contributor Author

gmmyung commented Feb 9, 2024

Also, I have excluded the signal feature from default features because it requires critical-section. Do you think it is okay to enable it by default now?

@thejpster
Copy link
Member

I'd make it optional because it takes 64 bytes of RAM.

@thejpster
Copy link
Member

I'll read up on tests with shared mutable state.

@gmmyung
Copy link
Contributor Author

gmmyung commented Feb 9, 2024

I'd make it optional because it takes 64 bytes of RAM.

I don't know if the rust linker strips unused static variables. I thought it didn't, but I'm not so sure.

@thejpster thejpster closed this Feb 9, 2024
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.

2 participants