Re: chown: Remove SUPPORT_DOT ifdef - it's on by default for 22 years
On Sat, May 27, 2017 at 10:58:40PM +0100, Jason McIntyre wrote:
> On Sat, May 27, 2017 at 11:45:43PM +0200, Adam Wolk wrote:
> > Index: chown.8
> > ===
> > RCS file: /cvs/src/bin/chmod/chown.8,v
> > retrieving revision 1.20
> > diff -u -p -r1.20 chown.8
> > --- chown.8 31 Dec 2015 23:38:16 - 1.20
> > +++ chown.8 27 May 2017 21:37:48 -
> > @@ -166,7 +166,12 @@ Previous versions of the
> > utility used the dot
> > .Pq Sq \&.
> > character to distinguish the group name.
> > -This has been changed to be a colon
> > +This has been changed when the utility was first
>
> s/has been/was/
>
> > +standardised in
> > +.St -p1003.2-92
> > +to be a colon
> > .Pq Sq \&:
> > -character so that user and
> > -group names may contain the dot character.
> > +character so that user and group names may contain the dot
>
> s/may/could/
> or
> s/so that user and group names may/to allow user and group names to/
>
> > +character, however the dot separator still remains supported
>
> s/however/though/
>
> > +due to widely required backwards compatibility.
> > +
>
> jmc
>
Thanks!
Included updated diffs with suggested changes applied.
Index: Makefile
===
RCS file: /cvs/src/bin/chmod/Makefile,v
retrieving revision 1.8
diff -u -p -r1.8 Makefile
--- Makefile11 Sep 2016 07:06:29 - 1.8
+++ Makefile27 May 2017 22:04:37 -
@@ -1,7 +1,6 @@
# $OpenBSD: Makefile,v 1.8 2016/09/11 07:06:29 natano Exp $
PROG= chmod
-CFLAGS+=-DSUPPORT_DOT
MAN= chmod.1 chgrp.1 chown.8 chflags.1
LINKS= ${BINDIR}/chmod ${BINDIR}/chgrp \
${BINDIR}/chmod /sbin/chown
Index: chmod.c
===
RCS file: /cvs/src/bin/chmod/chmod.c,v
retrieving revision 1.41
diff -u -p -r1.41 chmod.c
--- chmod.c 17 Feb 2017 10:14:12 - 1.41
+++ chmod.c 27 May 2017 22:04:37 -
@@ -197,14 +197,16 @@ done:
*cp++ = '\0';
gid = a_gid(cp);
}
-#ifdef SUPPORT_DOT
- /* UID and GID are separated by a dot and UID exists. */
+ /*
+* UID and GID are separated by a dot and UID exists.
+* required for backwards compatibility pre-dating POSIX.2
+* likely to stay here forever
+*/
else if ((cp = strchr(*argv, '.')) != NULL &&
(uid = a_uid(*argv, 1)) == (uid_t)-1) {
*cp++ = '\0';
gid = a_gid(cp);
}
-#endif
if (uid == (uid_t)-1)
uid = a_uid(*argv, 0);
} else
Index: chown.8
===
RCS file: /cvs/src/bin/chmod/chown.8,v
retrieving revision 1.20
diff -u -p -r1.20 chown.8
--- chown.8 31 Dec 2015 23:38:16 - 1.20
+++ chown.8 27 May 2017 22:04:37 -
@@ -166,7 +166,11 @@ Previous versions of the
utility used the dot
.Pq Sq \&.
character to distinguish the group name.
-This has been changed to be a colon
+This was changed when the utility was first standardised in
+.St -p1003.2-92
+to be a colon
.Pq Sq \&:
-character so that user and
-group names may contain the dot character.
+character to allow user and group names to contain the dot
+character, though the dot separator still remains supported
+due to widely required backwards compatibility.
+
? netstart.diff
Index: netstart
===
RCS file: /cvs/src/etc/netstart,v
retrieving revision 1.183
diff -u -p -r1.183 netstart
--- netstart7 May 2017 09:40:15 - 1.183
+++ netstart27 May 2017 18:47:51 -
@@ -99,7 +99,7 @@ ifstart() {
if [[ "${_stat[0]}${_stat[2]}${_stat[3]}" != *---00 ]]; then
echo "WARNING: $_file is insecure, fixing permissions"
chmod -LR o-rwx $_file
- chown -LR root.wheel $_file
+ chown -LR root:wheel $_file
fi
# Check for ifconfig'able interface, except if -n option is specified.
Re: chown: Remove SUPPORT_DOT ifdef - it's on by default for 22 years
On Sat, May 27, 2017 at 11:45:43PM +0200, Adam Wolk wrote: > Index: chown.8 > === > RCS file: /cvs/src/bin/chmod/chown.8,v > retrieving revision 1.20 > diff -u -p -r1.20 chown.8 > --- chown.8 31 Dec 2015 23:38:16 - 1.20 > +++ chown.8 27 May 2017 21:37:48 - > @@ -166,7 +166,12 @@ Previous versions of the > utility used the dot > .Pq Sq \&. > character to distinguish the group name. > -This has been changed to be a colon > +This has been changed when the utility was first s/has been/was/ > +standardised in > +.St -p1003.2-92 > +to be a colon > .Pq Sq \&: > -character so that user and > -group names may contain the dot character. > +character so that user and group names may contain the dot s/may/could/ or s/so that user and group names may/to allow user and group names to/ > +character, however the dot separator still remains supported s/however/though/ > +due to widely required backwards compatibility. > + jmc
Re: chown: Remove SUPPORT_DOT ifdef - it's on by default for 22 years
On Sat, May 27, 2017 at 11:01:29PM +0200, Adam Wolk wrote:
> On Sat, May 27, 2017 at 01:42:45PM -0600, Theo de Raadt wrote:
> > I agree with you. Maybe change the comment
> >
> > /* UID and GID are separated by a dot and UID exists. */
> >
> > to say a bit more on the matter, to prevent a zealot from arriving 2-3
> > years from now and proposing removal. Just a few words to hint . support
> > will stay forever.
> >
> > It seems the sentences in the man page could be changed a bit. Rather
> > than speaking about Previous versions, it could say POSIX (rev?)
> > deprecated '.' and introduced ':' as the default seperator, however '.'
> > seperator support remains for widely required backwards compat. The current
> > sentences speak a bit too strongly about '.' actually being gone.
> >
> >
>
> Updated the man page and expanded the comment in code.
>
> Attaching updated diffs, OK?
>
- style(9) the chmod.c comment
- use .St syntax to mark the standard in the man page instead of manually
hard coding the name
both issues pointed out by brynet@, thanks!
Index: Makefile
===
RCS file: /cvs/src/bin/chmod/Makefile,v
retrieving revision 1.8
diff -u -p -r1.8 Makefile
--- Makefile11 Sep 2016 07:06:29 - 1.8
+++ Makefile27 May 2017 21:37:48 -
@@ -1,7 +1,6 @@
# $OpenBSD: Makefile,v 1.8 2016/09/11 07:06:29 natano Exp $
PROG= chmod
-CFLAGS+=-DSUPPORT_DOT
MAN= chmod.1 chgrp.1 chown.8 chflags.1
LINKS= ${BINDIR}/chmod ${BINDIR}/chgrp \
${BINDIR}/chmod /sbin/chown
Index: chmod.c
===
RCS file: /cvs/src/bin/chmod/chmod.c,v
retrieving revision 1.41
diff -u -p -r1.41 chmod.c
--- chmod.c 17 Feb 2017 10:14:12 - 1.41
+++ chmod.c 27 May 2017 21:37:48 -
@@ -197,14 +197,16 @@ done:
*cp++ = '\0';
gid = a_gid(cp);
}
-#ifdef SUPPORT_DOT
- /* UID and GID are separated by a dot and UID exists. */
+ /*
+* UID and GID are separated by a dot and UID exists.
+* required for backwards compatibility pre-dating POSIX.2
+* likely to stay here forever
+*/
else if ((cp = strchr(*argv, '.')) != NULL &&
(uid = a_uid(*argv, 1)) == (uid_t)-1) {
*cp++ = '\0';
gid = a_gid(cp);
}
-#endif
if (uid == (uid_t)-1)
uid = a_uid(*argv, 0);
} else
Index: chown.8
===
RCS file: /cvs/src/bin/chmod/chown.8,v
retrieving revision 1.20
diff -u -p -r1.20 chown.8
--- chown.8 31 Dec 2015 23:38:16 - 1.20
+++ chown.8 27 May 2017 21:37:48 -
@@ -166,7 +166,12 @@ Previous versions of the
utility used the dot
.Pq Sq \&.
character to distinguish the group name.
-This has been changed to be a colon
+This has been changed when the utility was first
+standardised in
+.St -p1003.2-92
+to be a colon
.Pq Sq \&:
-character so that user and
-group names may contain the dot character.
+character so that user and group names may contain the dot
+character, however the dot separator still remains supported
+due to widely required backwards compatibility.
+
? netstart.diff
Index: netstart
===
RCS file: /cvs/src/etc/netstart,v
retrieving revision 1.183
diff -u -p -r1.183 netstart
--- netstart7 May 2017 09:40:15 - 1.183
+++ netstart27 May 2017 18:47:51 -
@@ -99,7 +99,7 @@ ifstart() {
if [[ "${_stat[0]}${_stat[2]}${_stat[3]}" != *---00 ]]; then
echo "WARNING: $_file is insecure, fixing permissions"
chmod -LR o-rwx $_file
- chown -LR root.wheel $_file
+ chown -LR root:wheel $_file
fi
# Check for ifconfig'able interface, except if -n option is specified.
Re: chown: Remove SUPPORT_DOT ifdef - it's on by default for 22 years
On Sat, May 27, 2017 at 01:42:45PM -0600, Theo de Raadt wrote:
> I agree with you. Maybe change the comment
>
> /* UID and GID are separated by a dot and UID exists. */
>
> to say a bit more on the matter, to prevent a zealot from arriving 2-3
> years from now and proposing removal. Just a few words to hint . support
> will stay forever.
>
> It seems the sentences in the man page could be changed a bit. Rather
> than speaking about Previous versions, it could say POSIX (rev?)
> deprecated '.' and introduced ':' as the default seperator, however '.'
> seperator support remains for widely required backwards compat. The current
> sentences speak a bit too strongly about '.' actually being gone.
>
>
Updated the man page and expanded the comment in code.
Attaching updated diffs, OK?
Index: Makefile
===
RCS file: /cvs/src/bin/chmod/Makefile,v
retrieving revision 1.8
diff -u -p -r1.8 Makefile
--- Makefile11 Sep 2016 07:06:29 - 1.8
+++ Makefile27 May 2017 20:53:36 -
@@ -1,7 +1,6 @@
# $OpenBSD: Makefile,v 1.8 2016/09/11 07:06:29 natano Exp $
PROG= chmod
-CFLAGS+=-DSUPPORT_DOT
MAN= chmod.1 chgrp.1 chown.8 chflags.1
LINKS= ${BINDIR}/chmod ${BINDIR}/chgrp \
${BINDIR}/chmod /sbin/chown
Index: chmod.c
===
RCS file: /cvs/src/bin/chmod/chmod.c,v
retrieving revision 1.41
diff -u -p -r1.41 chmod.c
--- chmod.c 17 Feb 2017 10:14:12 - 1.41
+++ chmod.c 27 May 2017 20:53:36 -
@@ -197,14 +197,14 @@ done:
*cp++ = '\0';
gid = a_gid(cp);
}
-#ifdef SUPPORT_DOT
- /* UID and GID are separated by a dot and UID exists. */
+ /* UID and GID are separated by a dot and UID exists.
+* required for backwards compatibility pre-dating POSIX.2
+* likely to stay here forever */
else if ((cp = strchr(*argv, '.')) != NULL &&
(uid = a_uid(*argv, 1)) == (uid_t)-1) {
*cp++ = '\0';
gid = a_gid(cp);
}
-#endif
if (uid == (uid_t)-1)
uid = a_uid(*argv, 0);
} else
Index: chown.8
===
RCS file: /cvs/src/bin/chmod/chown.8,v
retrieving revision 1.20
diff -u -p -r1.20 chown.8
--- chown.8 31 Dec 2015 23:38:16 - 1.20
+++ chown.8 27 May 2017 20:53:36 -
@@ -166,7 +166,12 @@ Previous versions of the
utility used the dot
.Pq Sq \&.
character to distinguish the group name.
-This has been changed to be a colon
+This has been changed when the utility was first
+standardised in POSIX.2 (IEEE Std 1003.2-1992)
+to be a colon
.Pq Sq \&:
character so that user and
-group names may contain the dot character.
+group names may contain the dot character, however
+the dot separator still remains supported due to
+widely required backwards compatibility.
+
? netstart.diff
Index: netstart
===
RCS file: /cvs/src/etc/netstart,v
retrieving revision 1.183
diff -u -p -r1.183 netstart
--- netstart7 May 2017 09:40:15 - 1.183
+++ netstart27 May 2017 18:47:51 -
@@ -99,7 +99,7 @@ ifstart() {
if [[ "${_stat[0]}${_stat[2]}${_stat[3]}" != *---00 ]]; then
echo "WARNING: $_file is insecure, fixing permissions"
chmod -LR o-rwx $_file
- chown -LR root.wheel $_file
+ chown -LR root:wheel $_file
fi
# Check for ifconfig'able interface, except if -n option is specified.
Re: chown: Remove SUPPORT_DOT ifdef - it's on by default for 22 years
I agree with you. Maybe change the comment /* UID and GID are separated by a dot and UID exists. */ to say a bit more on the matter, to prevent a zealot from arriving 2-3 years from now and proposing removal. Just a few words to hint . support will stay forever. It seems the sentences in the man page could be changed a bit. Rather than speaking about Previous versions, it could say POSIX (rev?) deprecated '.' and introduced ':' as the default seperator, however '.' seperator support remains for widely required backwards compat. The current sentences speak a bit too strongly about '.' actually being gone.
chown: Remove SUPPORT_DOT ifdef - it's on by default for 22 years
Hi tech@,
I stumbled on SUPPORT_DOT while reading /usr/src/bin/chmod.c, got curious
and started doing some research.
POSIX changed the separator from . to : to make the utility properly work with
usernames containing a dot. The standard doesn't forbid keeping the dot handling
for backwards compatiblity.
The code is currently #ifdef'ed in. I assume the reason was to phase it out
sometime in the future.
The code was there and enabled with CFLAGS back in 1995
https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.sbin/chown/Attic/Makefile?rev=1.1=text/x-cvsweb-markup
There were some attempts to weed it out but as far as I see they were abandonned
or stopped.
Back in 2001, by disabling the compat and trying to build the base system, no
followup email (that I can find):
https://marc.info/?l=openbsd-tech=99647882113533=2
Discussion that brought SUPPORT_DOT into the topic. Mostly people argumenting if
man pages shold be altered (sorry, can't find the thread on marc.info):
http://misc.openbsd.narkive.com/4ejjhI6O/in-username
I think it's unlikely at this point that this backwards support will go away.
Linux, Mac, NetBSD and FreeBSD all support the compat, people seem to be using a
mix of both (including in our base where ie. /etc/netstart uses the dot
notation).
I suggest dropping the ifdef and define. It's been built enabled by default for
22 years. I'm also adding a diff for /etc/netstart to switch it to the :
separator. It's one less strchr call, though that obviously doesn't make
much difference performance wise in this case.
Feedback? OK's?
? chmod
? support_dot.diff
Index: Makefile
===
RCS file: /cvs/src/bin/chmod/Makefile,v
retrieving revision 1.8
diff -u -p -r1.8 Makefile
--- Makefile11 Sep 2016 07:06:29 - 1.8
+++ Makefile27 May 2017 18:39:17 -
@@ -1,7 +1,6 @@
# $OpenBSD: Makefile,v 1.8 2016/09/11 07:06:29 natano Exp $
PROG= chmod
-CFLAGS+=-DSUPPORT_DOT
MAN= chmod.1 chgrp.1 chown.8 chflags.1
LINKS= ${BINDIR}/chmod ${BINDIR}/chgrp \
${BINDIR}/chmod /sbin/chown
Index: chmod.c
===
RCS file: /cvs/src/bin/chmod/chmod.c,v
retrieving revision 1.41
diff -u -p -r1.41 chmod.c
--- chmod.c 17 Feb 2017 10:14:12 - 1.41
+++ chmod.c 27 May 2017 18:39:17 -
@@ -197,14 +197,12 @@ done:
*cp++ = '\0';
gid = a_gid(cp);
}
-#ifdef SUPPORT_DOT
/* UID and GID are separated by a dot and UID exists. */
else if ((cp = strchr(*argv, '.')) != NULL &&
(uid = a_uid(*argv, 1)) == (uid_t)-1) {
*cp++ = '\0';
gid = a_gid(cp);
}
-#endif
if (uid == (uid_t)-1)
uid = a_uid(*argv, 0);
} else
? netstart.diff
Index: netstart
===
RCS file: /cvs/src/etc/netstart,v
retrieving revision 1.183
diff -u -p -r1.183 netstart
--- netstart7 May 2017 09:40:15 - 1.183
+++ netstart27 May 2017 18:47:51 -
@@ -99,7 +99,7 @@ ifstart() {
if [[ "${_stat[0]}${_stat[2]}${_stat[3]}" != *---00 ]]; then
echo "WARNING: $_file is insecure, fixing permissions"
chmod -LR o-rwx $_file
- chown -LR root.wheel $_file
+ chown -LR root:wheel $_file
fi
# Check for ifconfig'able interface, except if -n option is specified.
