From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/13555 Path: news.gmane.org!.POSTED!not-for-mail From: Khem Raj Newsgroups: gmane.linux.lib.musl.general Subject: Re: additional patches for musl Date: Sat, 15 Dec 2018 18:16:28 -0800 Message-ID: References: <20181216015704.GD23599@brightrain.aerifal.cx> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="0000000000002c8af8057d1a41f4" X-Trace: blaine.gmane.org 1544926488 14679 195.159.176.226 (16 Dec 2018 02:14:48 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Sun, 16 Dec 2018 02:14:48 +0000 (UTC) To: musl@lists.openwall.com Original-X-From: musl-return-13571-gllmg-musl=m.gmane.org@lists.openwall.com Sun Dec 16 03:14:44 2018 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.84_2) (envelope-from ) id 1gYLx6-0003i3-8d for gllmg-musl@m.gmane.org; Sun, 16 Dec 2018 03:14:44 +0100 Original-Received: (qmail 19677 invoked by uid 550); 16 Dec 2018 02:16:53 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 19654 invoked from network); 16 Dec 2018 02:16:52 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=aaxRCfLAgqJdyoSyXMG2r/lHeyoLqflaXgZWcUmJ/DM=; b=GTdZ7iBin8idKRVex8hAM/bkqMYdO37/d1c3SnJucB5/uOQY3mGYSifNs54s4NA0zS 1FknGcbiB0od9Mkd6rwROl8WJ6d9JKhMsAua8T1CxoE737ozHVhbZxp2cqeDvydtsUmX 8TxbblHmhhlpdzPHUyojjkMnoX5xLVAodsvhFbEaRPZaGh54eUvhc/LcS3Qd9G/iemFM 6A+h3EDRcHD70NUb3+lR1XMsDJKiKYqDkjrNY6Cpythpd3/E3W2c6HBbY3GGM54nQbsA Ho2uWtN9ppMBujZ7DbjQdkL7ESlmn8rWcBvNK4geFDstKfNsu56D/75TyQYqI4QEiwIw RymA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=aaxRCfLAgqJdyoSyXMG2r/lHeyoLqflaXgZWcUmJ/DM=; b=BvhY8jIT8HKpDrFtEraDFHrwmuJ2Pu/xbZc1HhT45P7T+25keSHwZz94GwLnWue0iV XLSvbFr4RrLAooDey/JHhdHVJPCAH2FjRNihJ34N+dSPx0IG973wD3iZlCw5p54Py+1U jIRtcrLcC3rPAqRD5uZuaKnWZ7WKmGHWZyAUbTh5TdcCyqwLFbg9yHpa4CRQ9/0Up/NJ 5bJT6GeFThj3m3A3J+D/u0vxlih0I0qDPtrTDfaMjqzVB7ygy9EFbti9C5WWxwhYSbcK rvsLPpDFvfDjQqr5uF7OnCrzRgaONKBosZHGWOKhvWMZtoHfVCXJHI5j4fF1JASf3ANu fKzA== X-Gm-Message-State: AA+aEWbzu0hE6w83bCoc4/pqwfi3T19FgRqNW9892KwAWbOEtvuUJsP3 Cxvu5vACJldtwR5s7BOqGIfaAonvI4AszvCNZIhIpw== X-Google-Smtp-Source: AFSGD/VxnjmJM0n4Lof9cpBc/xYmwX5QurRn7DcMERrpkWrB8MiKZ34nd3ejKCTY7ntryD+y32klJJDXl5Dgsm/2//o= X-Received: by 2002:ac8:36ba:: with SMTP id a55mr8493049qtc.236.1544926599610; Sat, 15 Dec 2018 18:16:39 -0800 (PST) In-Reply-To: <20181216015704.GD23599@brightrain.aerifal.cx> Xref: news.gmane.org gmane.linux.lib.musl.general:13555 Archived-At: --0000000000002c8af8057d1a41f4 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Sat, Dec 15, 2018 at 5:57 PM Rich Felker wrote: > On Sun, Dec 16, 2018 at 02:01:56AM +0200, =D1=81=D0=B5=D1=80=D0=B3=D0=B5= =D0=B9 =D0=B2=D0=BE=D0=BB=D0=BA=D0=BE=D0=B2=D0=B8=D1=87=D1=8C wrote: > > cabulertion. > > i finded additional patches for implementing some missing musl features= . > This seems to be as if you are trying to port musl to systemd I think Opposite would be a better approach qsort_r probably is a good to consider everything else I am not so sure In oepnembedded we have patched systemd to Undo these things which are being introduced here please take a look > > > > > --- a/include/stdlib.h > > +++ b/include/stdlib.h > > @@ -120,6 +120,9 @@ > > #if defined(_XOPEN_SOURCE) || defined(_GNU_SOURCE) \ > > || defined(_BSD_SOURCE) > > char *realpath (const char *__restrict, char *__restrict); > > +#ifdef _GNU_SOURCE > > +char *canonicalize_file_name (const char *__restrict); > > +#endif > > Small nit but rather than adding new #ifdef _GNU_SOURCE sections for > each declaration, any new ones should be added under the existing > section if there is one (and in stdlib.h there is). > > > long int random (void); > > void srandom (unsigned int); > > char *initstate (unsigned int, char *, size_t); > > --- a/src/misc/realpath.c > > +++ b/src/misc/realpath.c > > @@ -42,4 +42,9 @@ > > err: > > __syscall(SYS_close, fd); > > return 0; > > } > > + > > +char *canonicalize_file_name(const char *restrict filename) > > +{ > > + return realpath(filename, NULL); > > +} > > This can't go in the same file as realpath because it's not in the > reserved namespace. Further it's probably not an acceptable addition > since it's not widely used and has no advantages over the standard way > of writing the exact same thing using realpath directly. > > > --- a/include/netdb.h > > +++ b/include/netdb.h > > @@ -124,6 +124,8 @@ > > #define NO_RECOVERY 3 > > #define NO_DATA 4 > > #define NO_ADDRESS NO_DATA > > +#define NETDB_INTERNAL -1 > > +#define NETDB_SUCCESS 0 > > #endif > > > > > > --- a/include/ftw.h > > +++ b/include/ftw.h > > @@ -20,6 +20,14 @@ > > #define FTW_MOUNT 2 > > #define FTW_CHDIR 4 > > #define FTW_DEPTH 8 > > + > > +#ifdef _GNU_SOURCE > > +#define FTW_ACTIONRETVAL 16 > > +#define FTW_CONTINUE 0 > > +#define FTW_STOP 1 > > +#define FTW_SKIP_SUBTREE 2 > > +#define FTW_SKIP_SIBLINGS 3 > > +#endif > > This one and the corresponding functional change is probably worth > consideration. I've seen a few a > > > > > struct FTW { > > int base; > > --- a/src/misc/nftw.c > > +++ b/src/misc/nftw.c > > @@ -1,3 +1,8 @@ > > +#define FTW_ACTIONRETVAL 16 > > +#define FTW_CONTINUE 0 > > +#define FTW_STOP 1 > > +#define FTW_SKIP_SUBTREE 2 > > +#define FTW_SKIP_SIBLINGS 3 > > #include > > #include > > #include > > @@ -58,8 +58,20 @@ > > lev.level =3D new.level; > > lev.base =3D h ? h->base : (name=3Dstrrchr(path, '/')) ? name-pat= h : 0; > > > > - if (!(flags & FTW_DEPTH) && (r=3Dfn(path, &st, type, &lev))) > > - return r; > > + if (!(flags & FTW_DEPTH) && (r=3Dfn(path, &st, type, &lev))) { > > + if (flags & FTW_ACTIONRETVAL) > > + switch (r) { > > + case FTW_SKIP_SUBTREE: > > + h =3D NULL; > > + case FTW_CONTINUE: > > + break; > > + case FTW_SKIP_SIBLINGS: > > + case FTW_STOP: > > + return r; > > + } > > + else > > + return r; > > + } > > > > for (; h; h =3D h->chain) > > if (h->dev =3D=3D st.st_dev && h->ino =3D=3D st.st_ino) > > @@ -82,7 +94,10 @@ > > strcpy(path+j+1, de->d_name); > > if ((r=3Ddo_nftw(path, fn, fd_limit-1, > flags, &new))) { > > closedir(d); > > - return r; > > + if ((flags & FTW_ACTIONRETVAL) && > r =3D=3D FTW_SKIP_SIBLINGS) > > + break; > > + else > > + return r; > > } > > } > > closedir(d); > > @@ -93,8 +108,16 @@ > > } > > > > path[l] =3D 0; > > - if ((flags & FTW_DEPTH) && (r=3Dfn(path, &st, type, &lev))) > > - return r; > > + if ((flags & FTW_DEPTH) && (r=3Dfn(path, &st, type, &lev))) { > > + if (flags & FTW_ACTIONRETVAL) > > + switch (r) { > > + case FTW_SKIP_SIBLINGS: > > + case FTW_STOP: > > + return r; > > + } > > + else > > + return r; > > + } > > > > return 0; > > } > > > --- /dev/null > > +++ b/src/misc/parse-printf-format.c > > @@ -0,0 +1,265 @@ > > +/*** > > + Copyright 2014 Emil Renner Berthing > > + With parts from the musl C library > > + Copyright 2005-2014 Rich Felker, et al. > > + > > + This file is free software; you can redistribute it and/or modify it > > + under the terms of the GNU Lesser General Public License as publishe= d > by > > + the Free Software Foundation; either version 2.1 of the License, or > > + (at your option) any later version. > > Please do not send license-incompatible patches to musl. > > > --- /dev/null > > +++ b/include/printf.h > > @@ -0,0 +1,41 @@ > > +/*** > > + Copyright 2014 Emil Renner Berthing > > + With parts from the GNU C Library > > + Copyright 1991-2014 Free Software Foundation, Inc. > > + > > + This file is free software; you can redistribute it and/or modify it > > + under the terms of the GNU Lesser General Public License as publishe= d > by > > Likewise. > > > + the Free Software Foundation; either version 2.1 of the License, or > > + (at your option) any later version. > > + > > + This file is distributed in the hope that it will be useful, but > > + WITHOUT ANY WARRANTY; without even the implied warranty of > > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > > + Lesser General Public License for more details. > > +***/ > > + > > +#pragma once > > #pragma once is not valid C. There is a universal standard way to do this= . > > > --- a/include/stdlib.h > > +++ b/include/stdlib.h > > @@ -52,10 +52,13 @@ > > char *getenv (const char *); > > > > int system (const char *); > > > > +typedef int (*__compar_fn_t)(const void *, const void *); > > This is in the internal namespace and it's an error for any > application to be using it. > > > +typedef int (*comparison_fn_t)(const void *, const void *); > > This is presumably a GNU extension (so would need to be in the > appropriate #ifdef block if added) but it probably doesn't meet the > usefulness criterion. Any code using this non-portable construct can > trivially be fixed to be portable. > > > void *bsearch (const void *, const void *, size_t, size_t, int > (*)(const void *, const void *)); > > void qsort (void *, size_t, size_t, int (*)(const void *, const void > *)); > > +void qsort_r (void *, size_t, size_t, int (*)(const void *, const void > *, void *), void *); > > qsort_r is an open issue pending action with the BSDs and the Austin > Group for unifying and standardizing it. Historically some BSDs had an > opposite-argument-order function by the same name which is why musl > has not added it. It might be good to go now; we need to check. > > > --- a/src/stdlib/qsort.c 2017-01-01 04:27:17.000000000 +0100 > > +++ b/src/stdlib/qsort.c 2017-01-29 00:21:27.194887091 +0100 > > @@ -28,7 +28,7 @@ > > #include "atomic.h" > > #define ntz(x) a_ctz_l((x)) > > > > -typedef int (*cmpfun)(const void *, const void *); > > +typedef int (*cmpfun)(const void *, const void *, void *); > > > > static inline int pntz(size_t p[2]) { > > int r =3D ntz(p[0] - 1); > > @@ -85,7 +85,7 @@ > > p[1] >>=3D n; > > } > > > > -static void sift(unsigned char *head, size_t width, cmpfun cmp, int > pshift, size_t lp[]) > > +static void sift(unsigned char *head, size_t width, cmpfun cmp, void* > arg, int pshift, size_t lp[]) > > Passing the extra context arg all the way through everything possibly > has significant performance hit for the standard qsort function. This > needs to be measured, and if so, either 2 separate versions need to be > built, or qsort_r needs to be implemented on top of qsort using TLS. > > > [...] > > + > > +void qsort(void *base, size_t nel, size_t width, int (*cmp)(const void > *, const void *)) > > +{ > > + return qsort_r (base, nel, width, (cmpfun)cmp, NULL); > > +} > > This cast and subsequent calling the cmp function with the incorrect > signature has undefined behavior and can't be used. > > > --- a/include/stdlib.h > > +++ b/include/stdlib.h > > @@ -50,7 +50,8 @@ > > int at_quick_exit (void (*) (void)); > > _Noreturn void quick_exit (int); > > > > char *getenv (const char *); > > +char *secure_getenv (const char *); > > Addition of this function is probably ok, but it needs to be under the > proper _GNU_SOURCE test. > > > > > int system (const char *); > > > > --- /dev/null > > +++ b/src/env/secure_getenv.c > > @@ -0,0 +1,8 @@ > > +#define _DEFAULT_SOURCE 1 > > +#include > > +#include > > + > > +char *secure_getenv(const char *name) > > +{ > > + return issetugid() ? NULL : getenv(name); > > +} > > > --- a/include/string.h > > +++ b/include/string.h > > @@ -85,7 +85,21 @@ > > #endif > > > > #ifdef _GNU_SOURCE > > -#define strdupa(x) strcpy(alloca(strlen(x)+1),x) > > +#ifdef __GNUC__ > > +#define strdupa(x) (__extension__({ \ > > + const char* __old =3D (x); \ > > + size_t __len =3D strlen(x) + 1; \ > > + char* __new =3D (char*)alloca(__len); \ > > + (char*)memcpy(__new, __old, __len); \ > > + })) > > +#define strndupa(x,y) (__extension__({ \ > > + const char* __old =3D (x); \ > > + size_t __len =3D strnlen(x, (y)); \ > > + char* __new =3D (char*)alloca(__len + 1); \ > > + __new[__len] =3D 0; \ > > + (char*)memcpy(__new, __old, __len); \ > > + })) > > +#endif > > This has been discussed before and we have a pending patch somewhere > that does it in another way. I'll see if I can dig it up and comment > based on that. Ultimately stdndupa is a really bad idea, but if it's > purely inline in a header there's not much burden on us by having it. > > > --- a/include/utmpx.h > > +++ b/include/utmpx.h > > @@ -55,6 +55,12 @@ > > #define USER_PROCESS 7 > > #define DEAD_PROCESS 8 > > > > + > > +#ifdef _GNU_SOURCE > > +#define _PATH_UTMPX "/dev/null/utmp" > > +#define _PATH_WTMPX "/dev/null/wtmp" > > +#endif > > + > > #ifdef __cplusplus > > } > > #endif > > I'm not sure if this is a good idea or not. Those paths have been > problematic in the past. > > > define unimplemented macros to 0 so that systemd compiles > > --- a/include/glob.h > > +++ b/include/glob.h > > @@ -22,6 +22,7 @@ > > int glob(const char *__restrict, int, int (*)(const char *, int), > glob_t *__restrict); > > void globfree(glob_t *); > > > > +#define GLOB_BRACE 0 > > This is definitely not acceptable. It advertises the existence of a > feature we don't have, making it impossible for applications to make > correct use of #ifdef to decide whether to use that feature or whether > to use their own replacement for the glob function that provides it. > > If your goal in all this is to build systemd, a better approach is > just using the systemd patches others have already done. systemd > policy is *opposed* to writing portable code, or to making any > promises about what level of GNU extensions they require and will > require in the future. So if we just added a bunch of random stuff to > make systemd work out of the box, a few months or a year from now we'd > find that it doesn't work anymore, and we have to either add whatever > other new glibc thing they're demanding is, or accept that we added a > bunch of junk for nothing. This has been discussed a lot before and > it's not a game musl is willing to play. > > > --- a/include/regex.h > > +++ b/include/regex.h > > @@ -31,6 +31,7 @@ > > > > #define REG_NOTBOL 1 > > #define REG_NOTEOL 2 > > +#define REG_STARTEND 0 > > Likewise this. There has been a request to add it before, and I think > there's a patch somewhere doing just that. The main concern is > long-term cost. The current regex implementation has sufficient > overhead that it doesn't matter, but if we do a better one at some > point in the future it might come back to bite us. I think it's > probably a good idea to add though. > > > #define REG_OK 0 > > #define REG_NOMATCH 1 > > --- a/include/netdb.h > > +++ b/include/netdb.h > > @@ -140,6 +140,8 @@ > > #define EAI_IDN_ENCODE -105 > > #define NI_MAXHOST 255 > > #define NI_MAXSERV 32 > > +#define NI_IDN 0 > > +#define NI_IDN_USE_STD3_ASCII_RULES 0 > > #endif > > The intent is to do IDN without any special flag, in which case > defining it as 0 would possibly be the right thing to do, but at > present it's another case of wrongly advertising a feature that's not > present and breaks application logic. > > Rich > --0000000000002c8af8057d1a41f4 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


On Sat,= Dec 15, 2018 at 5:57 PM Rich Felker <dalias@libc.org> wrote:
On = Sun, Dec 16, 2018 at 02:01:56AM +0200, =D1=81=D0=B5=D1=80=D0=B3=D0=B5=D0=B9= =D0=B2=D0=BE=D0=BB=D0=BA=D0=BE=D0=B2=D0=B8=D1=87=D1=8C wrote:
> cabulertion.
> i finded additional patches for implementing some missing musl feature= s.

This seems to be= as if you are trying to port musl to systemd I think
Opposite would be a better approach qsort_r probably is a good to conside= r everything else I am not so sure=C2=A0

<= div dir=3D"auto">In oepnembedded we have patched systemd to=C2=A0
Undo these things which are being introduced here please take= a look=C2=A0



> --- a/include/stdlib.h
> +++ b/include/stdlib.h
> @@ -120,6 +120,9 @@
>=C2=A0 #if defined(_XOPEN_SOURCE) || defined(_GNU_SOURCE) \
>=C2=A0 =C2=A0|| defined(_BSD_SOURCE)
>=C2=A0 char *realpath (const char *__restrict, char *__restrict);
> +#ifdef _GNU_SOURCE
> +char *canonicalize_file_name (const char *__restrict);
> +#endif

Small nit but rather than adding new #ifdef _GNU_SOURCE sections for
each declaration, any new ones should be added under the existing
section if there is one (and in stdlib.h there is).

>=C2=A0 long int random (void);
>=C2=A0 void srandom (unsigned int);
>=C2=A0 char *initstate (unsigned int, char *, size_t);
> --- a/src/misc/realpath.c
> +++ b/src/misc/realpath.c
> @@ -42,4 +42,9 @@
>=C2=A0 err:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0__syscall(SYS_close, fd);
>=C2=A0 =C2=A0 =C2=A0 =C2=A0return 0;
>=C2=A0 }
> +
> +char *canonicalize_file_name(const char *restrict filename)
> +{
> +=C2=A0 =C2=A0 =C2=A0return realpath(filename, NULL);
> +}

This can't go in the same file as realpath because it's not in the<= br> reserved namespace. Further it's probably not an acceptable addition since it's not widely used and has no advantages over the standard way<= br> of writing the exact same thing using realpath directly.

> --- a/include/netdb.h
> +++ b/include/netdb.h
> @@ -124,6 +124,8 @@
>=C2=A0 #define NO_RECOVERY=C2=A0 =C2=A0 3
>=C2=A0 #define NO_DATA=C2=A0 =C2=A0 =C2=A0 =C2=A0 4
>=C2=A0 #define NO_ADDRESS=C2=A0 =C2=A0 =C2=A0NO_DATA
> +#define NETDB_INTERNAL -1
> +#define NETDB_SUCCESS=C2=A0 0
>=C2=A0 #endif




> --- a/include/ftw.h
> +++ b/include/ftw.h
> @@ -20,6 +20,14 @@
>=C2=A0 #define FTW_MOUNT 2
>=C2=A0 #define FTW_CHDIR 4
>=C2=A0 #define FTW_DEPTH 8
> +
> +#ifdef _GNU_SOURCE
> +#define FTW_ACTIONRETVAL 16
> +#define FTW_CONTINUE 0
> +#define FTW_STOP 1
> +#define FTW_SKIP_SUBTREE 2
> +#define FTW_SKIP_SIBLINGS 3
> +#endif

This one and the corresponding functional change is probably worth
consideration. I've seen a few a

>=C2=A0
>=C2=A0 struct FTW {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0int base;
> --- a/src/misc/nftw.c
> +++ b/src/misc/nftw.c
> @@ -1,3 +1,8 @@
> +#define FTW_ACTIONRETVAL 16
> +#define FTW_CONTINUE 0
> +#define FTW_STOP 1
> +#define FTW_SKIP_SUBTREE 2
> +#define FTW_SKIP_SIBLINGS 3
>=C2=A0 #include <ftw.h>
>=C2=A0 #include <dirent.h>
>=C2=A0 #include <sys/stat.h>
> @@ -58,8 +58,20 @@
>=C2=A0 =C2=A0 =C2=A0 =C2=A0lev.level =3D new.level;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0lev.base =3D h ? h->base : (name=3Dstrrch= r(path, '/')) ? name-path : 0;
>=C2=A0
> -=C2=A0 =C2=A0 =C2=A0if (!(flags & FTW_DEPTH) && (r=3Dfn(p= ath, &st, type, &lev)))
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return r;
> +=C2=A0 =C2=A0 =C2=A0if (!(flags & FTW_DEPTH) && (r=3Dfn(p= ath, &st, type, &lev))) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (flags & FTW_A= CTIONRETVAL)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0switch (r) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0case FTW_SKIP_SUBTREE:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0h =3D NULL;<= br> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0case FTW_CONTINUE:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0break;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0case FTW_SKIP_SIBLINGS:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0case FTW_STOP:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return r; > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0}
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0else
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0return r;
> +=C2=A0 =C2=A0 =C2=A0}
>=C2=A0
>=C2=A0 =C2=A0 =C2=A0 =C2=A0for (; h; h =3D h->chain)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (h->dev = =3D=3D st.st_dev && h->ino =3D=3D st.st_ino)
> @@ -82,7 +94,10 @@
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0strcpy(path+j+1, de->d_name); >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if ((r=3Ddo_nftw(path, fn, fd_limi= t-1, flags, &new))) {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0closed= ir(d);
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return r; > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if ((flags &= amp; FTW_ACTIONRETVAL) && r =3D=3D FTW_SKIP_SIBLINGS)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0break;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0else
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0return r;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0}
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0closedir(d);
> @@ -93,8 +108,16 @@
>=C2=A0 =C2=A0 =C2=A0 =C2=A0}
>=C2=A0
>=C2=A0 =C2=A0 =C2=A0 =C2=A0path[l] =3D 0;
> -=C2=A0 =C2=A0 =C2=A0if ((flags & FTW_DEPTH) && (r=3Dfn(pa= th, &st, type, &lev)))
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return r;
> +=C2=A0 =C2=A0 =C2=A0if ((flags & FTW_DEPTH) && (r=3Dfn(pa= th, &st, type, &lev))) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (flags & FTW_A= CTIONRETVAL)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0switch (r) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0case FTW_SKIP_SIBLINGS:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0case FTW_STOP:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return r; > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0}
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0else
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0return r;
> +=C2=A0 =C2=A0 =C2=A0}
>=C2=A0
>=C2=A0 =C2=A0 =C2=A0 =C2=A0return 0;
>=C2=A0 }

> --- /dev/null
> +++ b/src/misc/parse-printf-format.c
> @@ -0,0 +1,265 @@
> +/***
> +=C2=A0 Copyright 2014 Emil Renner Berthing
> +=C2=A0 With parts from the musl C library
> +=C2=A0 Copyright 2005-2014 Rich Felker, et al.
> +
> +=C2=A0 This file is free software; you can redistribute it and/or mod= ify it
> +=C2=A0 under the terms of the GNU Lesser General Public License as pu= blished by
> +=C2=A0 the Free Software Foundation; either version 2.1 of the Licens= e, or
> +=C2=A0 (at your option) any later version.

Please do not send license-incompatible patches to musl.

> --- /dev/null
> +++ b/include/printf.h
> @@ -0,0 +1,41 @@
> +/***
> +=C2=A0 Copyright 2014 Emil Renner Berthing
> +=C2=A0 With parts from the GNU C Library
> +=C2=A0 Copyright 1991-2014 Free Software Foundation, Inc.
> +
> +=C2=A0 This file is free software; you can redistribute it and/or mod= ify it
> +=C2=A0 under the terms of the GNU Lesser General Public License as pu= blished by

Likewise.

> +=C2=A0 the Free Software Foundation; either version 2.1 of the Licens= e, or
> +=C2=A0 (at your option) any later version.
> +
> +=C2=A0 This file is distributed in the hope that it will be useful, b= ut
> +=C2=A0 WITHOUT ANY WARRANTY; without even the implied warranty of
> +=C2=A0 MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the G= NU
> +=C2=A0 Lesser General Public License for more details.
> +***/
> +
> +#pragma once

#pragma once is not valid C. There is a universal standard way to do this.<= br>
> --- a/include/stdlib.h
> +++ b/include/stdlib.h
> @@ -52,10 +52,13 @@
>=C2=A0 char *getenv (const char *);
>=C2=A0
>=C2=A0 int system (const char *);
>=C2=A0
> +typedef int (*__compar_fn_t)(const void *, const void *);

This is in the internal namespace and it's an error for any
application to be using it.

> +typedef int (*comparison_fn_t)(const void *, const void *);

This is presumably a GNU extension (so would need to be in the
appropriate #ifdef block if added) but it probably doesn't meet the
usefulness criterion. Any code using this non-portable construct can
trivially be fixed to be portable.

>=C2=A0 void *bsearch (const void *, const void *, size_t, size_t, int (= *)(const void *, const void *));
>=C2=A0 void qsort (void *, size_t, size_t, int (*)(const void *, const = void *));
> +void qsort_r (void *, size_t, size_t, int (*)(const void *, const voi= d *, void *), void *);

qsort_r is an open issue pending action with the BSDs and the Austin
Group for unifying and standardizing it. Historically some BSDs had an
opposite-argument-order function by the same name which is why musl
has not added it. It might be good to go now; we need to check.

> --- a/src/stdlib/qsort.c=C2=A0 =C2=A0 =C2=A0 2017-01-01 04:27:17.00000= 0000 +0100
> +++ b/src/stdlib/qsort.c=C2=A0 =C2=A0 =C2=A0 2017-01-29 00:21:27.19488= 7091 +0100
> @@ -28,7 +28,7 @@
>=C2=A0 #include "atomic.h"
>=C2=A0 #define ntz(x) a_ctz_l((x))
>=C2=A0
> -typedef int (*cmpfun)(const void *, const void *);
> +typedef int (*cmpfun)(const void *, const void *, void *);
>=C2=A0
>=C2=A0 static inline int pntz(size_t p[2]) {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0int r =3D ntz(p[0] - 1);
> @@ -85,7 +85,7 @@
>=C2=A0 =C2=A0 =C2=A0 =C2=A0p[1] >>=3D n;
>=C2=A0 }
>=C2=A0
> -static void sift(unsigned char *head, size_t width, cmpfun cmp, int p= shift, size_t lp[])
> +static void sift(unsigned char *head, size_t width, cmpfun cmp, void*= arg, int pshift, size_t lp[])

Passing the extra context arg all the way through everything possibly
has significant performance hit for the standard qsort function. This
needs to be measured, and if so, either 2 separate versions need to be
built, or qsort_r needs to be implemented on top of qsort using TLS.

> [...]
> +
> +void qsort(void *base, size_t nel, size_t width, int (*cmp)(const voi= d *, const void *))
> +{
> +=C2=A0 =C2=A0 =C2=A0return qsort_r (base, nel, width, (cmpfun)cmp, NU= LL);
> +}

This cast and subsequent calling the cmp function with the incorrect
signature has undefined behavior and can't be used.

> --- a/include/stdlib.h
> +++ b/include/stdlib.h
> @@ -50,7 +50,8 @@
>=C2=A0 int at_quick_exit (void (*) (void));
>=C2=A0 _Noreturn void quick_exit (int);
>=C2=A0
>=C2=A0 char *getenv (const char *);
> +char *secure_getenv (const char *);

Addition of this function is probably ok, but it needs to be under the
proper _GNU_SOURCE test.

>=C2=A0
>=C2=A0 int system (const char *);
>=C2=A0
> --- /dev/null
> +++ b/src/env/secure_getenv.c
> @@ -0,0 +1,8 @@
> +#define _DEFAULT_SOURCE 1
> +#include <stdlib.h>
> +#include <unistd.h>
> +
> +char *secure_getenv(const char *name)
> +{
> +=C2=A0 =C2=A0 =C2=A0return issetugid() ? NULL : getenv(name);
> +}

> --- a/include/string.h
> +++ b/include/string.h
> @@ -85,7 +85,21 @@
>=C2=A0 #endif
>=C2=A0
>=C2=A0 #ifdef _GNU_SOURCE
> -#define=C2=A0 =C2=A0 =C2=A0 strdupa(x)=C2=A0 =C2=A0 =C2=A0 strcpy(all= oca(strlen(x)+1),x)
> +#ifdef __GNUC__
> +#define strdupa(x)=C2=A0 =C2=A0(__extension__({ \
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0const char* __old =3D (x); \
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0size_t __len =3D strlen(x) + 1; \
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0char* __new =3D (char*)alloca(__len); \
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0(char*)memcpy(__new, __old, __len); \
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0}))
> +#define strndupa(x,y)=C2=A0 =C2=A0 =C2=A0 =C2=A0 (__extension__({ \ > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0const char* __old =3D (x); \
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0size_t __len =3D strnlen(x, (y)); \
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0char* __new =3D (char*)alloca(__len + 1); \
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0__new[__len] =3D 0; \
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0(char*)memcpy(__new, __old, __len); \
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0}))
> +#endif

This has been discussed before and we have a pending patch somewhere
that does it in another way. I'll see if I can dig it up and comment based on that. Ultimately stdndupa is a really bad idea, but if it's purely inline in a header there's not much burden on us by having it.
> --- a/include/utmpx.h
> +++ b/include/utmpx.h
> @@ -55,6 +55,12 @@
>=C2=A0 #define USER_PROCESS=C2=A0 =C2=A0 7
>=C2=A0 #define DEAD_PROCESS=C2=A0 =C2=A0 8
>=C2=A0
> +
> +#ifdef _GNU_SOURCE
> +#define _PATH_UTMPX "/dev/null/utmp"
> +#define _PATH_WTMPX "/dev/null/wtmp"
> +#endif
> +
>=C2=A0 #ifdef __cplusplus
>=C2=A0 }
>=C2=A0 #endif

I'm not sure if this is a good idea or not. Those paths have been
problematic in the past.

> define unimplemented macros to 0 so that systemd compiles
> --- a/include/glob.h
> +++ b/include/glob.h
> @@ -22,6 +22,7 @@
>=C2=A0 int=C2=A0 glob(const char *__restrict, int, int (*)(const char *= , int), glob_t *__restrict);
>=C2=A0 void globfree(glob_t *);
>=C2=A0
> +#define GLOB_BRACE 0

This is definitely not acceptable. It advertises the existence of a
feature we don't have, making it impossible for applications to make correct use of #ifdef to decide whether to use that feature or whether
to use their own replacement for the glob function that provides it.

If your goal in all this is to build systemd, a better approach is
just using the systemd patches others have already done. systemd
policy is *opposed* to writing portable code, or to making any
promises about what level of GNU extensions they require and will
require in the future. So if we just added a bunch of random stuff to
make systemd work out of the box, a few months or a year from now we'd<= br> find that it doesn't work anymore, and we have to either add whatever other new glibc thing they're demanding is, or accept that we added a bunch of junk for nothing. This has been discussed a lot before and
it's not a game musl is willing to play.

> --- a/include/regex.h
> +++ b/include/regex.h
> @@ -31,6 +31,7 @@
>=C2=A0
>=C2=A0 #define REG_NOTBOL=C2=A0 =C2=A0 =C2=A0 1
>=C2=A0 #define REG_NOTEOL=C2=A0 =C2=A0 =C2=A0 2
> +#define REG_STARTEND=C2=A0 =C2=A0 0

Likewise this. There has been a request to add it before, and I think
there's a patch somewhere doing just that. The main concern is
long-term cost. The current regex implementation has sufficient
overhead that it doesn't matter, but if we do a better one at some
point in the future it might come back to bite us. I think it's
probably a good idea to add though.

>=C2=A0 #define REG_OK=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 0
>=C2=A0 #define REG_NOMATCH=C2=A0 =C2=A0 =C2=A01
> --- a/include/netdb.h
> +++ b/include/netdb.h
> @@ -140,6 +140,8 @@
>=C2=A0 #define EAI_IDN_ENCODE -105
>=C2=A0 #define NI_MAXHOST 255
>=C2=A0 #define NI_MAXSERV 32
> +#define NI_IDN 0
> +#define NI_IDN_USE_STD3_ASCII_RULES 0
>=C2=A0 #endif

The intent is to do IDN without any special flag, in which case
defining it as 0 would possibly be the right thing to do, but at
present it's another case of wrongly advertising a feature that's n= ot
present and breaks application logic.

Rich
--0000000000002c8af8057d1a41f4--