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

[WIP] add unix function to fetch metacall custom lib path #240

Closed
wants to merge 2 commits into from

Conversation

burnerlee
Copy link
Contributor

No description provided.

@viferga
Copy link
Member

viferga commented Jan 26, 2022

I need you to delete all you wrote in metacallcli and squash the commits. Save the code in a backup before squashing, other contributors have lost the changes due to this.

I am writing you a test so you can verify the issue.

Also use make clang-format in order to lint the code.

Other recommendations, change this:

  • metacall_lib_name[17] by metacall_lib_name[] and in other arrays too. It's simpler and the compiler sets the size automatically.
  • The struct metacall_lib_data can be a static struct shared between the dynlink implementation but not exposed to the external details. You can define the string as MAX_PATH or PATH_MAX (depending on platform), so you don't need to malloc or free it.
  • When you define things like:
#define _GNU_SOURCE
// .. some includes ...
#include <link.h>

It is better if you put the definition of _GNU_SOURCE just on top of the header you want to affect, in this case link.h, and also it's recommended to do this:

#ifndef _GNU_SOURCE
#define _GNU_SOURCE
#endif
#include <link.h>
  • Also try to enable sanitizers for better debugging: cmake -DOPTION_BUILD_SANITIZER=ON .., this works mostly on Linux, I haven't tried on windows.
  • If there's any function which is duplicated between the implementations, you can put it in an intermediate file so it is not duplicated between them.

@viferga
Copy link
Member

viferga commented Jan 26, 2022

Here's your test: 082a78a

Uncomment this for testing:

// ASSERT_EQ((int)0, strcmp(METACALL_LIBRARY_PATH, dynlink_get_metacall_lib_path()));

For testing use:

make -j$(nproc) metacall-dynlink-test
ctest -VV -R metacall-dynlink-test

@viferga
Copy link
Member

viferga commented Feb 1, 2022

@burnerlee I have done a base for you:

8ad085f

You can implement windows and macos (not sure if beos has support for it). I have tried to follow your design but abstract it in such a way that code can be reused between implementations.

You can create a new PR if you prefer it, based on my last changes and updating with your current code.

Check out also how do I generate the names of the libraries because there is a function already present for that.

@viferga
Copy link
Member

viferga commented Feb 8, 2022

Closed: #244

@viferga viferga closed this Feb 8, 2022
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