nss_wrapper support for musl-libc

Mon Feb 20 16:08:33 UTC 2017

Hi Andreas!

On Thursday, 26 January 2017 10:19:26 CET Andreas Schneider wrote:
> On Wednesday, 25 January 2017 23:47:23 CET Dennis Schridde wrote:
> > I made a few changes to nss_wrapper to get it to run on musl-libc 1.1
> > (Alpine Linux 3.5). Please find the patch attached.
> > 
> > One test failed, without me being able to fix it: gethostent returns NULL
> > on musl-libc, but I don't know what might cause this or whether the test
> > is actually valid for musl-libc. If you could provide me with further
> > information on the purpose and expected behaviour of this function (the
> > information on the net was rather scarce) or why it might return NULL in
> > this case, I would be very glad.
> 
> The only documentation is the source. You need to dig there to see what is
> going on.

It turns out that reason is simple: The function is a stub.

https://git.musl-libc.org/cgit/musl/tree/src/network/ent.c

Thus I checked in CMakeLists.txt whether gethostent() generally returns non-
NULL pointers. Based on that I define HAVE_NONNULL_GETHOSTENT and use the 
result in tests/test_nwrap_disabled.c

> > If you want to test this, I suggest using my Dockerfile [1] as a basis. By
> > also installing "cmocka" and changing the build instructions at the bottom
> > appropriately, you can run the testsuite.
> > 
> > I would be glad if you could integrate this into your sources for the next
> > release. If you have concerns about my changes, related to coding style or
> > anything else, please do not hesitate to explain to me how I should
> > improve
> > this code to allow inclusion into your sources.
> 
> The changes to src/nss_wrapper.c are fine, if you comment #else and #endif,
> like:
> 
> #ifdef HAVE_GETPWENT_R
> ...
> #else /* HAVE_GETPWENT_R */
> ...
> #endif /* HAVE_GETPWENT_R */
> 
> This makes it easier to understand which #ifdef it is is.

I fixed that in attached patches.

The checks for HAVE_SOLARIS_GETGRENT_R were not consistent and used the 
inversion operator for the #else part. I fixed that, too, to conform to the 
style guidelines you mentioned above.

> About tests/test_getaddrinfo.c:
> 
> I think you should report those to the developers of musl-libc. These tests
> pass correctly on Linux, Solaris and BSD. All have different libc
> implementations.
> 
>  	rc = getaddrinfo(NULL, "echo", &hints, &res);
> -	assert_int_equal(rc, EAI_NONAME);
> +	if (rc != EAI_SERVICE) { // musl-libc returns EAI_SERVICE
> +		assert_int_equal(rc, EAI_NONAME);
> +	}
> 
> I would argue that EAI_NONAME is the correct error code here as it is NULL.
> EAI_SERICE is obviously the wrong return code.

Is EAI_NONAME correct, because "echo" is specified in /etc/services as 7/tcp 
or 7/udp? I.e. because a remote "node" address is necessary to communicate via 
TCP or UDP and getaddrinfo is supposed to deduct that from /etc/services?

I fixed that in attached patches. The tests will fail, until upstream resolves 
the issue. I will report it to them, as soon as I have a better understanding 
of how the function is supposed to behave.

> The changes to tests/testsuite.c are fine if comments are added. Please make
> seperate commits for each file.

Please find the patch series, one patch for each file, attached.

--Dennis
-- 
Universität Heidelberg, Universitätsrechenzentrum (URZ)
Servicebereich Future IT - Research & Education (FIRE)

Tel. +49 6221 54-4519, Fax +49 6221 54-5581
dennis.schridde at uni-heidelberg.de

http://www.urz.uni-heidelberg.de/

Ruprecht-Karls-Universität Heidelberg
Universitätsrechenzentrum
Im Neuenheimer Feld 293, 69120 Heidelberg
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-compatibility-for-musl-libc-1.1.patch
Type: text/x-patch
Size: 6473 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170220/0ad83553/0001-Add-compatibility-for-musl-libc-1.1.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Add-compatibility-for-musl-libc-1.1.patch
Type: text/x-patch
Size: 3623 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170220/0ad83553/0002-Add-compatibility-for-musl-libc-1.1.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-Add-compatibility-for-musl-libc-1.1.patch
Type: text/x-patch
Size: 2089 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170220/0ad83553/0003-Add-compatibility-for-musl-libc-1.1.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-Add-compatibility-for-musl-libc-1.1.patch
Type: text/x-patch
Size: 1726 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170220/0ad83553/0004-Add-compatibility-for-musl-libc-1.1.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-Add-compatibility-for-musl-libc-1.1.patch
Type: text/x-patch
Size: 1521 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170220/0ad83553/0005-Add-compatibility-for-musl-libc-1.1.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 691 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170220/0ad83553/signature.sig>