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
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 13 additions & 8 deletions error.c
Original file line number Diff line number Diff line change
Expand Up @@ -3188,36 +3188,41 @@ rb_syserr_new_path_in(const char *func_name, int n, VALUE path)
}
#endif

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.

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

void
rb_mod_sys_fail(VALUE mod, const char *mesg)
{
VALUE exc = make_errno_exc(mesg);
rb_extend_object(exc, mod);
rb_exc_raise(exc);
rb_mod_sys_tmp(exc, mod);
}

void
rb_mod_sys_fail_str(VALUE mod, VALUE mesg)
{
VALUE exc = make_errno_exc_str(mesg);
rb_extend_object(exc, mod);
rb_exc_raise(exc);
rb_mod_sys_tmp(exc, mod);
}

void
rb_mod_syserr_fail(VALUE mod, int e, const char *mesg)
{
VALUE exc = rb_syserr_new(e, mesg);
rb_extend_object(exc, mod);
rb_exc_raise(exc);
rb_mod_sys_tmp(exc, mod);
}

void
rb_mod_syserr_fail_str(VALUE mod, int e, VALUE mesg)
{
VALUE exc = rb_syserr_new_str(e, mesg);
rb_extend_object(exc, mod);
rb_exc_raise(exc);
rb_mod_sys_tmp(exc, mod);
}

static void
Expand Down