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

Generalize loader searching algorithm #119

Merged
merged 2 commits into from
Aug 31, 2018
Merged

Generalize loader searching algorithm #119

merged 2 commits into from
Aug 31, 2018

Conversation

chfast
Copy link
Member

@chfast chfast commented Aug 30, 2018

Allow loader to shorten the name word by word to the point where only single word is left.

chfast added 2 commits August 30, 2018 23:24
Allow loader to shorten the name word by word to the point where only single word is left.
* - if function not found, the _base name_ is shorten by skipping the first word separated by "_":
* "interpreter",
* - then, the function of the shorter name "evmc_create_" + _base name_ is searched in the library:
* "evmc_create_interpreter",
Copy link
Member

Choose a reason for hiding this comment

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

So what exactly is the logic here? It removes the prefixes? example off example_interpreter

Copy link
Member Author

Choose a reason for hiding this comment

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

yes

Copy link
Member Author

Choose a reason for hiding this comment

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

But also a_b_c_d -> b_c_d -> c_d -> d.

Copy link
Member

Choose a reason for hiding this comment

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

What is the reasoning for that?

Copy link
Member Author

Choose a reason for hiding this comment

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

evmc-example-vm :) I want it to come up with function name evmc_create_example_vm() while previously it tried only evmc_create_evmc_example_vm() and evmc_create_vm().

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 that may not be the most sound reasoning :)

Why not just name the file example-vm (and the creator appropriately) and be done with it?

Copy link
Member Author

Choose a reason for hiding this comment

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

For 3 reasons:

  • I don't want to name the library example-vm in case it is going to be installed.
  • This code looks better than before.
  • Even the author of loader (me) was no able to remember how exactly it was working before so Host example #116 does not work.

Copy link
Member

Choose a reason for hiding this comment

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

Didn't notice the old code was doing the very same weird logic.

Copy link
Member Author

Choose a reason for hiding this comment

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

It was, but not fully. It was skipping all leading words except the last one.

@chfast chfast mentioned this pull request Aug 30, 2018
@chfast chfast merged commit 297ce06 into master Aug 31, 2018
@chfast chfast deleted the loader branch August 31, 2018 08:15
if (!shorter_name_pos)
break;

memmove(base_name, shorter_name_pos + 1, strlen(shorter_name_pos) + 1);
}

if (!create_fn)
Copy link
Member

Choose a reason for hiding this comment

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

Practically this part wouldn't be needed if in the above loop on !short_name_pos base_name is set to \0.

Copy link
Member Author

Choose a reason for hiding this comment

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

True. I will check it out.

Copy link
Member Author

Choose a reason for hiding this comment

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

No, it will get messy. I would need to add another loop terminating condition (like strlen == 0) and we don't want to check for "evmc_create_" but "evmc_create".

Also, the naming convention will change soon and we might want to try "evmc_create" first.

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