| Message ID | 20191212202252.2786262-3-raj.khem@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [oe,meta-oe] xterm: Fix latent issue found with musl | expand |
On Thu, Dec 12, 2019 at 12:22:52PM -0800, Khem Raj wrote: >... > +Upstream-Status: Pending Not upstreamed OE-only musl patches create a technical debt. >... > +there is no test to define HAVE_GRANTPT_PTY_ISATTY Is the definition in xterm_io.h not working? >... > +_POSIX_SOURCE is app-defined not system This is true for musl, not for glibc. The handling of _POSIX_SOURCE and _POSIX_VERSION differs between musl and glibc due to glibc supporting older POSIX versions, and glibc also supporting developers of portable code to select some specific older POSIX version. >... > +-#if defined(_POSIX_SOURCE) || defined(SVR4) || defined(__convex__) || defined(__SCO__) || defined(__QNX__) > ++#if defined(_POSIX_VERSION) || defined(SVR4) || defined(__convex__) || defined(__SCO__) || defined(__QNX__) > + int pgrp = setsid(); /* variable may not be used... */ > + #else > + int pgrp = getpid(); >... So this was caused by musl not supporting older versions of POSIX, where _POSIX_SOURCE was part of the standard. The proper fix would be an autoconf test for setsid(). cu Adrian -- _______________________________________________ Openembedded-devel mailing list Openembedded-devel@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-devel
On Thu, Dec 12, 2019 at 8:37 PM Adrian Bunk <bunk@stusta.de> wrote: > > On Thu, Dec 12, 2019 at 12:22:52PM -0800, Khem Raj wrote: > >... > > +Upstream-Status: Pending > > Not upstreamed OE-only musl patches create a technical debt. > > >... > > +there is no test to define HAVE_GRANTPT_PTY_ISATTY > > Is the definition in xterm_io.h not working? > > >... > > +_POSIX_SOURCE is app-defined not system > > This is true for musl, not for glibc. > > The handling of _POSIX_SOURCE and _POSIX_VERSION differs between musl > and glibc due to glibc supporting older POSIX versions, and glibc also > supporting developers of portable code to select some specific older > POSIX version. > No thats not right. _POSIX_SOURCE (like all FTMs) is defined by the application to request a feature/standards profile it's not libc telling you "this is posix" or anything like that. _POSIX_VERSION tells you that > >... > > +-#if defined(_POSIX_SOURCE) || defined(SVR4) || defined(__convex__) || defined(__SCO__) || defined(__QNX__) > > ++#if defined(_POSIX_VERSION) || defined(SVR4) || defined(__convex__) || defined(__SCO__) || defined(__QNX__) > > + int pgrp = setsid(); /* variable may not be used... */ > > + #else > > + int pgrp = getpid(); > >... > > So this was caused by musl not supporting older versions of POSIX, > where _POSIX_SOURCE was part of the standard. > see above. > The proper fix would be an autoconf test for setsid(). > Thats a good idea. I have sent a v2 which adds a configure test got setsid() > cu > Adrian -- _______________________________________________ Openembedded-devel mailing list Openembedded-devel@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-devel
On Fri, Dec 13, 2019 at 01:55:33PM -0800, Khem Raj wrote: > On Thu, Dec 12, 2019 at 8:37 PM Adrian Bunk <bunk@stusta.de> wrote: > > On Thu, Dec 12, 2019 at 12:22:52PM -0800, Khem Raj wrote: >... > > > +_POSIX_SOURCE is app-defined not system > > > > This is true for musl, not for glibc. > > > > The handling of _POSIX_SOURCE and _POSIX_VERSION differs between musl > > and glibc due to glibc supporting older POSIX versions, and glibc also > > supporting developers of portable code to select some specific older > > POSIX version. > > > No thats not right. > _POSIX_SOURCE (like all FTMs) is > defined by the application to request a feature/standards profile > it's not libc telling you "this is posix" or anything like that. It is also defined when _POSIX_C_SOURCE is defined, or _GNU_SOURCE, or when the default non-strict gcc mode is used. The latter means that it is in practice nearly always defined when the libc supports applications written against older versions of POSIX. > _POSIX_VERSION tells you that >... _POSIX_VERSION being defined tells you that you have unistd.h included. With glibc the value of _POSIX_VERSION depends on what feature/standards profile the application has requested. cu Adrian -- _______________________________________________ Openembedded-devel mailing list Openembedded-devel@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-devel
On 12/13/19 2:58 PM, Adrian Bunk wrote: > On Fri, Dec 13, 2019 at 01:55:33PM -0800, Khem Raj wrote: >> On Thu, Dec 12, 2019 at 8:37 PM Adrian Bunk <bunk@stusta.de> wrote: >>> On Thu, Dec 12, 2019 at 12:22:52PM -0800, Khem Raj wrote: >> ... >>>> +_POSIX_SOURCE is app-defined not system >>> >>> This is true for musl, not for glibc. >>> >>> The handling of _POSIX_SOURCE and _POSIX_VERSION differs between musl >>> and glibc due to glibc supporting older POSIX versions, and glibc also >>> supporting developers of portable code to select some specific older >>> POSIX version. >>> >> No thats not right. >> _POSIX_SOURCE (like all FTMs) is >> defined by the application to request a feature/standards profile >> it's not libc telling you "this is posix" or anything like that. > > It is also defined when _POSIX_C_SOURCE is defined, or _GNU_SOURCE, > or when the default non-strict gcc mode is used. > > The latter means that it is in practice nearly always defined when the > libc supports applications written against older versions of POSIX. > but these macros are not used in this case so its irrelevant here. >> _POSIX_VERSION tells you that >> ... > > _POSIX_VERSION being defined tells you that you have unistd.h included. > > With glibc the value of _POSIX_VERSION depends on what feature/standards > profile the application has requested. I would not look into how glibc does something and call it standard. It guards these definitions by its internal defines ( prefixed with __) which is internal to a libc implementation, applications are not exposed to private namespace. > > cu > Adrian > -- _______________________________________________ Openembedded-devel mailing list Openembedded-devel@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-devel
On Fri, Dec 13, 2019 at 09:55:49PM -0800, Khem Raj wrote: > On 12/13/19 2:58 PM, Adrian Bunk wrote: > > On Fri, Dec 13, 2019 at 01:55:33PM -0800, Khem Raj wrote: > > > On Thu, Dec 12, 2019 at 8:37 PM Adrian Bunk <bunk@stusta.de> wrote: > > > > On Thu, Dec 12, 2019 at 12:22:52PM -0800, Khem Raj wrote: > > > ... > > > > > +_POSIX_SOURCE is app-defined not system > > > > > > > > This is true for musl, not for glibc. > > > > > > > > The handling of _POSIX_SOURCE and _POSIX_VERSION differs between musl > > > > and glibc due to glibc supporting older POSIX versions, and glibc also > > > > supporting developers of portable code to select some specific older > > > > POSIX version. > > > > > > > No thats not right. > > > _POSIX_SOURCE (like all FTMs) is > > > defined by the application to request a feature/standards profile > > > it's not libc telling you "this is posix" or anything like that. > > > > It is also defined when _POSIX_C_SOURCE is defined, or _GNU_SOURCE, > > or when the default non-strict gcc mode is used. > > > > The latter means that it is in practice nearly always defined when the > > libc supports applications written against older versions of POSIX. > > > but these macros are not used in this case so its irrelevant here. No macro has to be used, with gcc default settings _POSIX_SOURCE is defined: $ cat test.c #include <unistd.h> int tmp = _POSIX_SOURCE; $ gcc -O2 -Wall -c test.c $ musl is different, since it does not aim at fully supporting applications that were written against an older version of the POSIX standard. > > > _POSIX_VERSION tells you that > > > ... > > > > _POSIX_VERSION being defined tells you that you have unistd.h included. > > > > With glibc the value of _POSIX_VERSION depends on what feature/standards > > profile the application has requested. > > I would not look into how glibc does something and call it standard. I would not look into how musl does something and call it standard. I checked what musl and glibc do and what the standard says. > It guards these definitions by its internal defines ( prefixed with __) which > is internal to a libc implementation, applications are not exposed to > private namespace. _POSIX_SOURCE is defined in POSIX.1-1990 and mentioned in later versions of POSIX. _POSIX_VERSION is defined in all(?) versions of POSIX. cu Adrian -- _______________________________________________ Openembedded-devel mailing list Openembedded-devel@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-devel
On Sat, Dec 14, 2019 at 6:21 AM Adrian Bunk <bunk@stusta.de> wrote: > On Fri, Dec 13, 2019 at 09:55:49PM -0800, Khem Raj wrote: > > On 12/13/19 2:58 PM, Adrian Bunk wrote: > > > On Fri, Dec 13, 2019 at 01:55:33PM -0800, Khem Raj wrote: > > > > On Thu, Dec 12, 2019 at 8:37 PM Adrian Bunk <bunk@stusta.de> wrote: > > > > > On Thu, Dec 12, 2019 at 12:22:52PM -0800, Khem Raj wrote: > > > > ... > > > > > > +_POSIX_SOURCE is app-defined not system > > > > > > > > > > This is true for musl, not for glibc. > > > > > > > > > > The handling of _POSIX_SOURCE and _POSIX_VERSION differs between > musl > > > > > and glibc due to glibc supporting older POSIX versions, and glibc > also > > > > > supporting developers of portable code to select some specific > older > > > > > POSIX version. > > > > > > > > > No thats not right. > > > > _POSIX_SOURCE (like all FTMs) is > > > > defined by the application to request a feature/standards profile > > > > it's not libc telling you "this is posix" or anything like that. > > > > > > It is also defined when _POSIX_C_SOURCE is defined, or _GNU_SOURCE, > > > or when the default non-strict gcc mode is used. > > > > > > The latter means that it is in practice nearly always defined when the > > > libc supports applications written against older versions of POSIX. > > > > > but these macros are not used in this case so its irrelevant here. > > No macro has to be used, with gcc default settings _POSIX_SOURCE > is defined: Again this was what I said originally was not correct > > > $ cat test.c > #include <unistd.h> > int tmp = _POSIX_SOURCE; > $ gcc -O2 -Wall -c test.c > $ > > musl is different, since it does not aim at fully supporting > applications that were written against an older version of > the POSIX standard. > > > > > _POSIX_VERSION tells you that > > > > ... > > > > > > _POSIX_VERSION being defined tells you that you have unistd.h included. > > > > > > With glibc the value of _POSIX_VERSION depends on what > feature/standards > > > profile the application has requested. > > > > I would not look into how glibc does something and call it standard. > > I would not look into how musl does something and call it standard. Who said that effort is to find common solution > > > I checked what musl and glibc do and what the standard says. > > > It guards these definitions by its internal defines ( prefixed with __) > which > > is internal to a libc implementation, applications are not exposed to > > private namespace. > > _POSIX_SOURCE is defined in POSIX.1-1990 and mentioned in later versions > of POSIX. > > _POSIX_VERSION is defined in all(?) versions of POSIX. > Ok thank you > > cu > Adrian > -- _______________________________________________ Openembedded-devel mailing list Openembedded-devel@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-devel
diff --git a/meta-oe/recipes-graphics/xorg-app/xterm/posix_ptys.patch b/meta-oe/recipes-graphics/xorg-app/xterm/posix_ptys.patch new file mode 100644 index 0000000000..54020d53e9 --- /dev/null +++ b/meta-oe/recipes-graphics/xorg-app/xterm/posix_ptys.patch @@ -0,0 +1,29 @@ +there is no test to define HAVE_GRANTPT_PTY_ISATTY and +_POSIX_SOURCE is app-defined not system +This fix ptys and launching xterm + +Upstream-Status: Pending + +Suggested By Rich Felker +Signed-off-by: Khem Raj <raj.khem@gmail.com> + +--- a/main.c ++++ b/main.c +@@ -2892,7 +2892,7 @@ get_pty(int *pty, char *from GCC_UNUSED) + close(opened_tty); + opened_tty = -1; + } +-#elif defined(HAVE_POSIX_OPENPT) && defined(HAVE_PTSNAME) && defined(HAVE_GRANTPT_PTY_ISATTY) ++#elif defined(HAVE_POSIX_OPENPT) && defined(HAVE_PTSNAME) + if ((*pty = posix_openpt(O_RDWR)) >= 0) { + char *name = ptsname(*pty); + if (name != 0) { +@@ -4040,7 +4040,7 @@ spawnXTerm(XtermWidget xw, unsigned line + /* + * now in child process + */ +-#if defined(_POSIX_SOURCE) || defined(SVR4) || defined(__convex__) || defined(__SCO__) || defined(__QNX__) ++#if defined(_POSIX_VERSION) || defined(SVR4) || defined(__convex__) || defined(__SCO__) || defined(__QNX__) + int pgrp = setsid(); /* variable may not be used... */ + #else + int pgrp = getpid(); diff --git a/meta-oe/recipes-graphics/xorg-app/xterm_351.bb b/meta-oe/recipes-graphics/xorg-app/xterm_351.bb index 394d2cb9de..abfda8a5fa 100644 --- a/meta-oe/recipes-graphics/xorg-app/xterm_351.bb +++ b/meta-oe/recipes-graphics/xorg-app/xterm_351.bb @@ -4,7 +4,9 @@ DEPENDS = "libxaw xorgproto libxext libxau libxinerama libxpm ncurses" LIC_FILES_CHKSUM = "file://xterm.h;beginline=3;endline=31;md5=c7faceb872d90115e7c0ad90e90c390d" -SRC_URI = "http://invisible-mirror.net/archives/${BPN}/${BP}.tgz" +SRC_URI = "http://invisible-mirror.net/archives/${BPN}/${BP}.tgz \ + file://posix_ptys.patch \ + " SRC_URI[md5sum] = "a07edfbee2e2f4c6a9ddbf834fa4bbec" SRC_URI[sha256sum] = "760a8a10221c9c9744afd86db87c7ad95bbf9be4f5f525fecf39125f0d2a6e16"
[YOCTO #13691] Signed-off-by: Khem Raj <raj.khem@gmail.com> Cc: Ross Burton <ross.burton@intel.com> Cc: Armin Kuster <akuster808@gmail.com> --- .../xorg-app/xterm/posix_ptys.patch | 29 +++++++++++++++++++ .../recipes-graphics/xorg-app/xterm_351.bb | 4 ++- 2 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 meta-oe/recipes-graphics/xorg-app/xterm/posix_ptys.patch -- 2.24.1 -- _______________________________________________ Openembedded-devel mailing list Openembedded-devel@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-devel