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

Improve make_optional_func_list and make renegotiation functions optional. #23

Closed

Conversation

haydenroche5
Copy link
Contributor

make_optional_func_list will fall back to using nm to determine if a
function is provided by libwolfssl if libwolfssl is a static library. This
commit makes it so we first check if nm is available. If it's not available,
the function will print a warning and assume all optional functions are
available.

…onal.

`make_optional_func_list` will fall back to using `nm` to determine if a
function is provided by libwolfssl if libwolfssl is a static library. This
commit makes it so we first check if `nm` is available. If it's not available,
the function will print a warning and assume all optional functions are
available.
Copy link
Member

@julek-wolfssl julek-wolfssl left a comment

Choose a reason for hiding this comment

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

Installing on a "fresh" PC I needed to run sudo apt-get install python3-dev (would be sudo apt-get install python-dev for python 2) for setup to succeed. Let's add that to the README.

  • The error I sent you directly.

@@ -43,12 +43,19 @@ def make_optional_func_list(libwolfssl_path, funcs):
except AttributeError as _:
Copy link
Member

Choose a reason for hiding this comment

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

I know this isn't part of the PR but can we just change this to except AttributeError:? The as _ isn't necessary if all we care about is exception type.

Comment on lines +56 to +58
print(("WARNING: Can't determine available libwolfssl functions."
" Assuming all optional functions are available."))
defined = funcs
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps it would be better to throw an exception here and allow the user to skip the exception (and assume all optional funcs are available) through a command line parameter? This would make error detection and debugging easier.

@LinuxJedi
Copy link
Contributor

@haydenroche5 if you do loop back on this, can you think of a way of doing it without nm? This is pretty much the only blocker for me adding Windows support (Windows has a similar tool, but the path to it is different on pretty much every VS installation).

@haydenroche5
Copy link
Contributor Author

@haydenroche5 if you do loop back on this, can you think of a way of doing it without nm? This is pretty much the only blocker for me adding Windows support (Windows has a similar tool, but the path to it is different on pretty much every VS installation).

I think if there's no easy way to get the symbols reliably on Windows, then we'll just have to assume all the optional functions are available?

@LinuxJedi
Copy link
Contributor

I think if there's no easy way to get the symbols reliably on Windows, then we'll just have to assume all the optional functions are available?

That or we have to do a macro detection thing like in wolfcrypt-py (which I'm guessing will be much more work). Probably best to just assume all optional are available in Windows.

@JacobBarthelmeh
Copy link
Contributor

#43

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.

4 participants