Bug #5876
closed
sys/regset.h pollutes name space
100%
Description
Running a compile with -D__EXTENSIONS__ and this program:
/* * Compile on OpenIndiana with: * /opt/gcc/4.4.4/bin/gcc -std=c99 -c namespace-pollution.c * namespace-pollution.c:12:2: error: #error "ERR pollutes our namespace" */ #define _XOPEN_SOURCE 600 #define __EXTENSIONS__ 1 #include <stdio.h> #include <string.h> #include <stdlib.h> #include <ctype.h> #include <limits.h> #ifdef ERR #error "ERR pollutes our namespace" #endif int x;
Updated by Gordon Ross over 6 years ago
One could easily argue that "With __EXTENSIONS__ defined, we can pollute as much as we want."
but there are quite a few interesting things applications may want to use that are only exposed when __EXTENSIONS__ is defined, so we really should come up with a way to pollute less here, at least when sys/regset.h was not specifically asked for.
Updated by Gordon Ross over 6 years ago
I temporarily made sys/regset.h unreadable. Here's how we're getting it:
In file included from /usr/include/sys/signal.h:245,
from /usr/include/sys/procset.h:42,
from /usr/include/sys/wait.h:43,
from /usr/include/stdlib.h:40,
from namespace-pollution.c:13:
/usr/include/sys/ucontext.h:36:24: error: /usr/include/sys/regset.h: Permission denied
Updated by Gordon Ross over 6 years ago
Here's a Samba bug referring to this problem:
https://bugzilla.samba.org/attachment.cgi?id=10623&action=edit
Updated by Gordon Ross about 6 years ago
- Subject changed from intel sys/regset.h pollutes name space to sys/regset.h pollutes name space
Updated by Electric Monk about 6 years ago
- Status changed from New to Closed
- % Done changed from 0 to 100
git commit 21227944c2bcc086121a5428f3f9d2496ba646f5
commit 21227944c2bcc086121a5428f3f9d2496ba646f5
Author: Gordon Ross <gwr@nexenta.com>
Date: 2015-10-17T12:33:25.000Z
5876 sys/regset.h pollutes name space
Reviewed by: Josef 'Jeff' Sipek <jeffpc@josefsipek.net>
Reviewed by: Jonathan Perkin <jperkin@joyent.com>
Reviewed by: Alexander Pyhalov <alp@sfedu.ru>
Approved by: Albert Lee <trisk@omniti.com>
Updated by Gordon Ross about 6 years ago
- % Done changed from 100 to 0
After 5876, the header file sys/regset.h is no longer
included by sys/ucontext.h, and therefore no longer
indirectly included by sys/signal.h, stdlib.h etc.
Most programs are not affected at all. Many programs
no longer need to work around "surprise" defines for
SS, CS, ERR, and the like.
A few programs will require changes as a result of
this header change, where the code expected symbols
like EAX to be defined after include <sys/ucontex.h>.
Such program are easily adapted to this change by
adding an include <sys/regset.h>. Without that,
the code will fail to compile with errors like:
"signal.c", line 79: undefined symbol: UESP "signal.c", line 80: undefined symbol: EIP "signal.c", line 81: undefined symbol: EFL "signal.c", line 82: undefined symbol: EAX "signal.c", line 83: undefined symbol: EDX
There were several examples in illumos-gate where
this change was required. Here's one example:
--- a/usr/src/cmd/csh/i386/signal.c +++ b/usr/src/cmd/csh/i386/signal.c @@ -28,6 +26,7 @@ #include <sys/types.h> #include <sys/siginfo.h> #include <sys/ucontext.h> +#include <sys/regset.h> #include <signal.h> #include "signal.h" #include <errno.h>
The new rule is basically: If your source module needs the register
name defines (EIP etc.) it needs to include <sys/regset.h>.
I hope this solves more problems than it causes.
Updated by Gordon Ross about 6 years ago
- Status changed from Closed to Feedback
- Assignee set to Gordon Ross
It turns out there are many examples of external C code that expects
the register index defines after a #include <sys/ucontext.h> so we've
decided to just back this out for the moment. Sorry about the noise
and trouble.
I'll probably try an alternative approach that might hopefully meet both of these constraints:
(a) Provide the regset index definitions to code that includes <sys/ucontext.h>
(b) Do not expose those definitions to code that only included <sys/signal.h> etc.
Updated by Gordon Ross almost 6 years ago
Here's another fix attempt, this time maintaining compatibility for applications that include <ucontext.h>
https://github.com/gwr/illumos-gate/compare/2a44663...regset5d
Updated by Electric Monk almost 6 years ago
- Status changed from Feedback to Closed
- % Done changed from 0 to 100
git commit bc0e91320069f0bcaee43e80a7ea686d9efa2d08
commit bc0e91320069f0bcaee43e80a7ea686d9efa2d08
Author: Gordon Ross <gwr@nexenta.com>
Date: 2015-12-08T01:47:26.000Z
5876 sys/regset.h pollutes name space (try 2)
Reviewed by: Josef 'Jeff' Sipek <jeffpc@josefsipek.net>
Reviewed by: Jonathan Perkin <jperkin@joyent.com>
Reviewed by: Alexander Pyhalov <alp@sfedu.ru>
Reviewed by: Garrett D'Amore <garrett@damore.org>
Approved by: Dan McDonald <danmcd@omniti.com>