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

switch: add libsmb2 #116

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Conversation

rhyskoedijk
Copy link
Contributor

@rhyskoedijk rhyskoedijk commented Oct 19, 2019

Unstable port of libsmb2 (built against HEAD).
Not sure if this should wait until the next upstream release, but it is working fine for my needs as-is so I thought I'd share it. I would appreciate any feedback on the PR comments below, not sure if this could be done in a better way or not.

Kerberos auth is not supported, but simple auth (username/password) works well.
Share enumeration/discovery works well too.

Basic usage:

socketInitializeDefault();

ctx = smb2_init_context();
url = smb2_parse_url(ctx, "smb://[<domain;][<username>@]<host>[:<port>]/")
smb2_set_security_mode(ctx, SMB2_NEGOTIATE_SIGNING_ENABLED);
smb2_set_user(ctx, url->user); // if auth is required
smb2_set_password(ctx, "PASSWORD"); // if auth is required
smb2_connect_share(ctx, url->server, "SHARE_NAME", NULL);

// do the thing...

smb2_disconnect_share(ctx);
smb2_destroy_url(url);
smb2_destroy_context(ctx);

socketExit();

Better examples here:
https://github.com/sahlberg/libsmb2/tree/master/examples


libsmb2_la_CPPFLAGS = -I$(abs_top_srcdir)/include \
-I$(abs_top_srcdir)/include/smb2 \
+ -I$(DEVKITPRO)/libnx/include \
Copy link
Contributor Author

@rhyskoedijk rhyskoedijk Oct 19, 2019

Choose a reason for hiding this comment

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

Not sure why this is needed, but this is the only way I could get it to compile.
The build doesn't seem to pick up the libnx includes from the -isystem already defined in switchvars.sh.

In configure.ac some of the header checks are failing.
e.g. AC_CHECK_HEADERS([arpa/inet.h]) despite that header actually existing, it reports it as missing for some reason.

Comment on lines +10 to +13
+ struct addrinfo hints;
+ memset(&hints, 0, sizeof hints);
+ hints.ai_family = AF_INET; // AF_INET or AF_INET6 to force version
+ hints.ai_socktype = SOCK_STREAM;
Copy link
Contributor Author

@rhyskoedijk rhyskoedijk Oct 19, 2019

Choose a reason for hiding this comment

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

getaddrinfo() was as always throwing error 22 without this.
I found an existing issue reporting the same problem, copy/pasted the solution. Probably a better way to fix this though...

Copy link
Member

Choose a reason for hiding this comment

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

Fwiw getaddrinfo with hints=NULL is currently broken. However, a libnx branch I'm working on (https://github.com/switchbrew/libnx/tree/socket-cleanup) fixes this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, I'll remove this patch once fixed in libnx then.

@rhyskoedijk rhyskoedijk marked this pull request as ready for review October 19, 2019 20:51
@WinterMute
Copy link
Member

readv and writev don't belong here, please remove that last merge.

@rhyskoedijk
Copy link
Contributor Author

rhyskoedijk commented Feb 10, 2020

@WinterMute

readv and writev don't belong here, please remove that last merge.

Is there somewhere in the framework where I can reference these? I wasn't able to find them last time I looked.

@WinterMute
Copy link
Member

WinterMute commented Feb 10, 2020

@WinterMute

readv and writev don't belong here, please remove that last merge.

Is there somewhere in the framework where I can reference these? I wasn't able to find them last time I looked.

I don't really know what you mean by "framework" exactly. readv and writev aren't currently available in any library we provide but they don't belong in this library and shouldn't be provided here. That last merge you made will prevent this library ever being merged here.

@WinterMute
Copy link
Member

@rhyskoedijk Unless you remove the inappropriate code from this PR I'll have to reject and close this.

See devkitPro/newlib#18

It's not possible to guarantee that these functions will work correctly but they should not be provided by libsmb2.

@rhyskoedijk
Copy link
Contributor Author

Sorry, been distracted lately.
Framework was maybe the wrong word. Basically I looked everywhere in devkita64 didn't find it and so I just put it inline to get my project to work at the time. I'm not an expert at any of this so pointers are appreciated. I'll remove readv/writev when I next get the chance.

Should I add a PR for sys/uio to newlib? or is it unlikely to be accepted given your comment regarding HOS!=POSIX?

@averne averne mentioned this pull request Mar 7, 2024
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