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

Do you consider Darwin version ? #1

Open
Et7f3 opened this issue Oct 7, 2022 · 13 comments
Open

Do you consider Darwin version ? #1

Et7f3 opened this issue Oct 7, 2022 · 13 comments

Comments

@Et7f3
Copy link

Et7f3 commented Oct 7, 2022

When I compiled I get:

script.c:24:10: fatal error: 'endian.h' file not found
#include <endian.h>
         ^~~~~~~~~~
clang -DHAVE_CONFIG_H -I. -I..  -DKAMID_VERSION='"0.2"' -I../ -I../compat -I../lib    -MT kamiftp-ftp.o -MD -MP -MF .deps/kamiftp-ftp.Tpo -c -o kamiftp-ftp.o `test -f 'ftp.c' || echo './'`ftp.c
ftp.c:251:8: warning: implicit declaration of function 'le32toh' is invalid in C99 [-Wimplicit-function-declaration]
        len = le32toh(len);
              ^
ftp.c:275:9: warning: implicit declaration of function 'le64toh' is invalid in C99 [-Wimplicit-function-declaration]
        return le64toh(n);
               ^
ftp.c:284:9: warning: implicit declaration of function 'le32toh' is invalid in C99 [-Wimplicit-function-declaration]
        return le32toh(n);
               ^
ftp.c:293:9: warning: implicit declaration of function 'le16toh' is invalid in C99 [-Wimplicit-function-declaration]
        return le16toh(n);
               ^
ftp.c:904:50: error: use of undeclared identifier 'SOCK_CLOEXEC'
                sock = socket(res->ai_family, res->ai_socktype|SOCK_CLOEXEC,
In file included from table_static.c:25:
./kamid.h:112:14: error: use of undeclared identifier 'LOGIN_NAME_MAX'
        char            uname[LOGIN_NAME_MAX];
                              ^
1 error generated.

I ask because the description say UNIX-like. I tried to look at LOGIN_NAME_MAX and it seem to exist on Darwin:

$ getconf LOGIN_NAME_MAX
255

for endian.h I consider https://github.com/kristapsdz/oconfigure#endianh

Would you merge a PR that add Darwin support ?

@omar-polo
Copy link
Owner

Hello!

Yes, I'd be happy to merge a PR to add Darwin support. I don't have a mac, so I can't test it, but I care about portability :)
The oconfigure solution for endian.h seems fine.

Regarding LOGIN_NAME_MAX I was recently bit by it because neither FreeBSD has it. I'll commit something for it soon.

Thanks!

omar-polo added a commit that referenced this issue Oct 7, 2022
Both Linux and OpenBSD have LOGIN_NAME_MAX available when including
limits.h, FreeBSD, Darwin and possibly others don't.

FreeBSD (and maybe Darwin) have MAXLOGNAME, so try to use that if
available.  Otherwise use _POSIX_LOGIN_NAME_MAX, but only has a fallback
since it has a lower value (9 at the time of writing).

If everything fails, use 32 which is what OpenBSD use by default;
OpenSMTPd also defaults to it.

See also github issue #1
@omar-polo
Copy link
Owner

Does 20f7a94 help with LOGIN_NAME_MAX on darwin too?

@Et7f3
Copy link
Author

Et7f3 commented Oct 13, 2022

Thanks. I don't see the error again. Now the compiler can go further it show more error.

control.c:69:42: error: use of undeclared identifier 'SOCK_CLOEXEC'
        if ((fd = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK,
                                                ^
control.c:69:57: error: use of undeclared identifier 'SOCK_NONBLOCK'
        if ((fd = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK,
                                                               ^
control.c:138:16: warning: implicit declaration of function 'accept4' is invalid in C99 [-Wimplicit-function-declaration]
        if ((connfd = accept4(listenfd, (struct sockaddr *)&sun, &len,
                      ^

I checked Mac header doesn't contain those definition so I will need to fallback to the lass safer accept and fcntl.
Go for instance fallback without those flags set https://android.googlesource.com/platform/prebuilts/go/darwin-x86/+/brillo-m7-release/src/net/sock_cloexec.go

kamid.c:175:3: warning: 'daemon' is deprecated: first deprecated in macOS 10.5 - Use posix_spawn APIs instead. [-Wdeprecated-declarations]
                daemon(1, 0);
                ^
/nix/store/y9cwm5z3wimzf13b207ndyapdlgxm15h-Libsystem-1238.60.2/include/stdlib.h:288:6: note: 'daemon' has been explicitly marked deprecated here
int      daemon(int, int) __DARWIN_1050(daemon) __OSX_AVAILABLE_BUT_DEPRECATED_MSG(__MAC_10_0, __MAC_10_5, __IPHONE_2_0, __IPHONE_2_0, "Use posix_spawn APIs instead.") __WATCHOS_PROHIBITED __TVOS_PROHIBITED;
         ^

linux, freebsd, netbsd, openbsd, Darwin have a posix_spawn so we can get everybody happy.

parse.y:452:13: warning: implicit declaration of function 'reallocarray' is invalid in C99 [-Wimplicit-function-declaration]
                void *p = reallocarray(file->ungetbuf, file->ungetsize, 2);
                          ^

fixable using realloc

@omar-polo
Copy link
Owner

Right, darwin still lacks accept4 and the fancy SOCK_* flags; it's not an issue to just use accept and fcntl to set the nonblock and cloexec flags since kamid is not threaded.

The reallocarray warnings is because i forgot to include a check for it in the configure, i only have recallocarray. I'll fix that too.

Regarding daemon... I know that the posix_spawn APIs are widely available but i personally dislike them (they feel over-enginereed, confusing and plain ugly) so i'll keep using daemon(3). if daemon(3) will be ever removed on darwin, we'll find a way :)

@Et7f3
Copy link
Author

Et7f3 commented Oct 13, 2022

Darwin instead of le64toh has ntohll. Can you check in various bsd I think it is in freebsd maybe with _KERNEL guard https://github.com/apple-oss-distributions/Libc/blob/768d166d42689471e1e8fd1ddde5eee25db02381/nls/FreeBSD/msgcat.c#L62
If so I can make a PR with all those change.

omar-polo added a commit that referenced this issue Oct 13, 2022
I always forget that they're not available on darwin; reported by @Et7f3
in github issue #1
omar-polo added a commit that referenced this issue Oct 13, 2022
reported by @Et7f3 in github issue #1, thanks!
@omar-polo
Copy link
Owner

I think i've fixed the accept4 and reallocarray issue, let me know if those warnings/errors now are gone on darwin, thanks!

Regarding endian.h, freebsd has a compatible set of functions in sys/endian.h, so just adding a check for that should be enough. On darwin however it seems a bit more complex but doable: I'm looking in particular at how darwin is handled in got-portable: https://github.com/ThomasAdam/got-portable/blob/46384c6e760fdd54c263d1af7d6db58cc09a4e51/include/got_compat.h#L11-L30

Just a matter of style however, i don't like how got-portable does some compats: it looks for the operating system instead of checking for the actual presence of the feature, so I'd prefer if we add some checks for the headers (endian.h, sys/endian.h and OSByteOrder.h) and just include the first one found (plus the compats in case of darwin.)

This can be done by using AC_CHECK_HEADER in configure.ac and then adding the relative includes in compat.h and then removing all the #include <endian.h> in the rest of the project; see for e.g. how sys/tree.h is handled.

Let me know if i can help you with that, I'll be glad to merge a PR for it! Thanks!

omar-polo added a commit that referenced this issue Nov 22, 2022
FreeBSD has sys/endian.h and for MacOS we need to use the functions
from libkern/OSByteOrder.h

see github issue #1
omar-polo added a commit that referenced this issue Nov 22, 2022
FreeBSD and NetBSD have sys/endian.h, on MacOS we need to use the
functions from libkern/OSByteOrder.h

see github issue #1
@omar-polo
Copy link
Owner

I've pushed the endian branch with changes that should allow to build it on Mac. So far I've only tested on FreeBSD however.

@omar-polo
Copy link
Owner

since it fixes the build on FreeBSD (and possibly on NetBSD too) I just merged this in the main branch, let me know if it works on darwin too!

Thanks!

@hauleth
Copy link

hauleth commented Nov 3, 2024

I have checked and currently it doesn't build because of explicit_bzero call which is not available on macOS.

@omar-polo
Copy link
Owner

@hauleth oh, I see. I'll add it to the compat layer and then ask you to retry since I still don't have a mac :)

Thank you!

@hauleth
Copy link

hauleth commented Nov 4, 2024

@omar-polo you could setup macOS build on GitHub Actions and mark it as "allowed to fail". That way you would have some feedback even when you do not own macOS on your own.

Also, thanks for this project as it is probably the only project that I have found that provide 9p2000 CLI client. I want something like that as I am working on my own implementation and I want to be able to test against some 3rd party implementation.

@omar-polo
Copy link
Owner

@hauleth you're right! actually I'm already using cirrus ci to build on freebsd and macos in another project, so no excuses not to do it here too.

I've fixed some things, but it still fails on macos at the moment for other reasons. It may take me a bit still to fix it.

by the way

I want something like that as I am working on my own implementation and I want to be able to test against some 3rd party implementation.

I'm curious, and I'd like to test again other implementations as well :-) When/if you feel like, could you link your project here too? Thank you!

@hauleth
Copy link

hauleth commented Nov 5, 2024

@omar-polo it is partially public, but it is non-functional at the time of writing https://github.com/hauleth/e9p (implementation in Erlang).

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

No branches or pull requests

3 participants