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

Add rb_mod_sys_tmp function and Replace duplicate code #5063

Merged
merged 2 commits into from
Oct 30, 2021

Conversation

S-H-GAMELINKS
Copy link
Contributor

Some functions(like a rb_mod_sys_fail and rb_mod_sys_fail_str) has duplicate code.
Add rb_mod_sys_tmp function and replaced these duplicate code.

error.c Outdated
NORETURN(static void rb_mod_sys_tmp(VALUE exc, VALUE mod));

static void
rb_mod_sys_tmp(VALUE exc, VALUE mod)
Copy link
Member

Choose a reason for hiding this comment

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

What does tmp mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It meant a template for these codes

    rb_extend_object(exc, mod);
    rb_exc_raise(exc);

But, I think rb_mod_sys_fail_tmp is better now.

Copy link
Member

@nobu nobu Oct 30, 2021

Choose a reason for hiding this comment

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

"Template" for what?

sys_fail comes from "system call failure", I think.
OTOH, this function itself is no longer related with system calls, as the exception made from errno is given as an argument, therefore I don't think sys fits it.
Since the role of this function is to "raise the given exception extended with the given module", a name I though out was rb_mod_exc_raise (mod may not sound nice but could be consistent with other rb_mod_*_fail functions).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nobu
Thanks for your comment.
I cloud understanded these functions role.

I intended to mean template means "cut out common code to some function", but it's badly in this case.

rb_mod_exc_raise is good, I think. I'll rename this function.

@nobu nobu merged commit a46c220 into ruby:master Oct 30, 2021
@S-H-GAMELINKS S-H-GAMELINKS deleted the add_rb_mod_sys_tmp_function branch October 30, 2021 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants