On 08/15/2016 10:23 AM, Chuck Lever wrote: > [ adding libtirpc-devel ] > > >> On Aug 14, 2016, at 6:13 PM, Yann E. MORIN <yann.morin.1998@xxxxxxx> wrote: >> >> Chuck, All, >> >> On 2016-08-14 14:30 -0400, Chuck Lever spake thusly: >>>> On Aug 13, 2016, at 10:05 AM, Yann E. MORIN <yann.morin.1998@xxxxxxx> wrote: >>>> >>>> The __P() macro is defined in cdefs.h, so we must include it explicitly >>>> rather than relying on it being included by another header. >>>> >>>> cdefs.h is a glibc-ism; glibc includes it almost everywhere from its own >>>> headers. So it automatically gets included for glibc. >>>> >>>> However, cdefs.h is not present in musl, so its headers do not include >>>> it. We must thus include it when we need __P() (of course, one will have >>>> to provide his own cdefs.h in this case). >>> >>> Simply adding "#include <sys/cdefs.h>" seems like the wrong approach. >>> If cdefs.h is not guaranteed to exist, the appropriate thing to do >>> is provide some autoconf machinery to define __P() in its absence. >> >> OpenEmbedded provides comaptibility headers: >> http://git.openembedded.org/openembedded-core/tree/meta/recipes-core/bsd-headers/bsd-headers >> >> In Buildroot, we're adding them too (not yet applied, WIP): >> http://lists.busybox.net/pipermail/buildroot/2016-August/169722.html >> >> Other embedded buildsystem may each have their own fix in a way or >> another... >> >> Mainstream distros are more-or-less all based on glibc, except for a few >> outliers, like Alpine Linux (also based on musl), and they've gone on >> the "remove __P()" route: >> http://git.alpinelinux.org/cgit/aports/tree/main/rpcbind/0001-Avoid-use-of-glibc-sys-cdefs.h-header.patch >> >>> On the other hand, I wonder if we need to continue to preserve K&R C >>> compatibility in this code base. Perhaps instead the uses of __P() >>> should be eliminated? >> >> I tried to provide a minimalist approach, that consists in assuming that >> cdefs.h is present. > > If cdefs.h presence cannot be guaranteed (and I think you've adequately > demonstrated that no guarantee exists), at the very least there needs > to be some autoconf logic to handle the "cdefs.h is not present" case. > IMO a strictly minimalist approach won't work here. I don't see how it *can't* exist... At lease with Fedora and RHEL cdefs.h is part of the glibc-headers rpm and without that nothing in nfs-utils is going to compile > > >> But I do agree that pre-ANSI compatibility is probably a little tiny >> wee bit excessive nowadays. Virtually all current compilers do accept >> function prototypes, nowadays... >> >> I can work on a patch that does just get rid of the use of __P(). (we >> can't really vampirise the patch from Alpine, as there's no SoB or such >> origin information on it; not that redoing the patch would be too >> difficult either...). >> >> So, what route, now? ;-) > > My preference as a reviewer and individual contributor: > > Barring any further comments here, provide two different approaches: > > 1. add autoconf logic to detect when sys/cdefs.h is not available, > and provide a substitute __P() macro. That might be as simple as > defining __P in a local auto.m4 script when it is not provided by > system headers. I thinking we should fail the configuration if sys/cdefs.h does not exist... > > 2. remove invocations of the __P() macro from the rpcbind source Any idea what could break by removing them?? > > Post both to the mailing lists and folks here can decide which is > better. > > You might not have time for all that ;-) so you could pick one and > add a strong technical argument in the patch description why that > is the best choice. > > I think I like 2. overall as it should leave the rpcbind source > code a little easier to read, no new autoconf logic is needed, and > there appears to be one distro that is already going that way. I lean more toward taking the patch as is and failing the configuration if the header file does not exist.. I see how anything could break with that approach. steved. > > Maybe there's someone with the Alpine distro that can provide an > SoB for their patch? > > > -- > Chuck Lever > > > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
