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

Port sig2str.c to Rust. #165

Merged
merged 4 commits into from Apr 5, 2017

Conversation

Projects
None yet
2 participants
@barrucadu
Contributor

barrucadu commented Apr 3, 2017

  • I wasn't sure what to do about sig2str.h, so I just stripped out the now-irrelevant bits and left it.
  • str2sig is only used in abbr_to_signal in signal-process in process.c. When that is rustified it may be possible to adopt a safer interface.
  • sig2str doesn't appear to be used anywhere, and isn't exposed as a lisp function. Should it just be removed?

Closes #122

@Wilfred

This comment has been minimized.

Owner

Wilfred commented Apr 4, 2017

Looking good! I've opened #166 to fix the formatting issue which is breaking the build here.

Yep, feel free to remove unused functions. Emacs has quite a few of these: the C libraries are generic libraries that have been copy-pasted in, and not all the functionality is actually used.

@Wilfred

This comment has been minimized.

Owner

Wilfred commented Apr 4, 2017

I think keeping sig2str.h is sensible, as it means the C code knows the calling convention of the functions we're exposing. As you say, we can refactor as the C codebase shrinks.

@Wilfred

Wilfred approved these changes Apr 4, 2017

barrucadu added some commits Apr 3, 2017

Delete sig2str.c and add dummy sig2str.rs
List of signals is copied from the "sig" crate, with names added. That
crate isn't used directly as it doesn't provide the names, so a lookup
table like this would be needed anyway.
Implement str2sig.
This does not copy the RTMIN/RTMAX behaviour from the old C versions.
The libc crate does not define those values, which means that the
default behaviour where RTMIN=0 and RTMAX=-1 applies.

In the default case, those code paths are never executed:

    if (0 < rtmin && ...
    if (0 < rtmax && ...
Delete unused function sig2str.
This is not used elsewhere in the C source or exposed as a lisp
function, so there is no point in keeping it.
Rename sig2str.rs to str2sig.rs.
The old name makes little sense now that the `sig2str` function is
gone.
@barrucadu

This comment has been minimized.

Contributor

barrucadu commented Apr 5, 2017

I've rebased this against master and removed the sig2str function, I'm now happy with this PR if you don't have any further suggestions.

@Wilfred Wilfred merged commit 8bdf40c into Wilfred:master Apr 5, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Wilfred

This comment has been minimized.

Owner

Wilfred commented Apr 5, 2017

Terrific, thank you :)

@barrucadu barrucadu deleted the barrucadu:sig2str branch Apr 5, 2017

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