Re: openssl.cnf req defaults - default_md sha256
On Wed, 1 Oct 2014, Stuart Henderson wrote:
Over the coming months, web browsers will progressively start to first
warn for certificate chains including SHA-1 hashes, then treat them
as insecure (including disabling certain content - scripts etc).
Chrome are initially doing this for certs expiring after Jan 2017,
but will progressively slide it forward to certs expiring after
Jan 2016.
Since my previous attempt to at least show this in ssl(8) examples
for openssl req a few months ago, I've spent some time digging for
where the defaults are set in the code as a nicer place to set sane
values, but haven't tracked it down yet. Would it be OK to set it
in the default config for now? (or does anyone have an idea of where
in the code this comes from?)
Welcome to libkitchensink...
I'd need to quadruple check, however this should come from openssl/req.c
do_X509_sign() being called with a NULL digest, which calls openssl/req.c
do_sign_init() with a NULL md, which calls crypto/evp/m_sigver.c
EVP_DigestSignInit() with md being NULL, which calls crypto/evp/m_sigver.c
do_sigver_init() with type being NULL, which results in:
if (type == NULL) {
int def_nid;
if (EVP_PKEY_get_default_digest_nid(pkey, def_nid) 0)
type = EVP_get_digestbynid(def_nid);
}
EVP_PKEY_get_default_digest_nid() returns the default digest associated with
the given PKEY. Since you're using RSA, pkey_ctrl is implemented by
crypto/rsa/rsa_ameth.c rsa_pkey_ctrl(), which has:
case ASN1_PKEY_CTRL_DEFAULT_MD_NID:
*(int *)arg2 = NID_sha1;
return 1;
Catch all that?
To make SHA-256 the default for RSA, we'd have to change that from NID_sha1 to
NID_sha256...
(and yes, clearly I've spent too much time in this code base recently... :)
Index: openssl.cnf
===
RCS file: /cvs/src/lib/libcrypto/openssl.cnf,v
retrieving revision 1.1
diff -u -p -r1.1 openssl.cnf
--- openssl.cnf 11 Apr 2014 22:51:53 - 1.1
+++ openssl.cnf 30 Sep 2014 22:42:53 -
@@ -7,7 +7,8 @@
[ req ]
-default_bits = 1024
+default_bits = 2048
+default_md = sha256
default_keyfile = privkey.pem
distinguished_name = req_distinguished_name
attributes = req_attributes
--
Action without study is fatal. Study without action is futile.
-- Mary Ritter Beard
Re: openssl.cnf req defaults - default_md sha256
On Wed, 1 Oct 2014, Joel Sing wrote:
On Wed, 1 Oct 2014, Stuart Henderson wrote:
Over the coming months, web browsers will progressively start to first
warn for certificate chains including SHA-1 hashes, then treat them
as insecure (including disabling certain content - scripts etc).
Chrome are initially doing this for certs expiring after Jan 2017,
but will progressively slide it forward to certs expiring after
Jan 2016.
Since my previous attempt to at least show this in ssl(8) examples
for openssl req a few months ago, I've spent some time digging for
where the defaults are set in the code as a nicer place to set sane
values, but haven't tracked it down yet. Would it be OK to set it
in the default config for now? (or does anyone have an idea of where
in the code this comes from?)
Welcome to libkitchensink...
I'd need to quadruple check, however this should come from openssl/req.c
do_X509_sign() being called with a NULL digest, which calls openssl/req.c
do_sign_init() with a NULL md, which calls crypto/evp/m_sigver.c
EVP_DigestSignInit() with md being NULL, which calls crypto/evp/m_sigver.c
do_sigver_init() with type being NULL, which results in:
if (type == NULL) {
int def_nid;
if (EVP_PKEY_get_default_digest_nid(pkey, def_nid) 0)
type = EVP_get_digestbynid(def_nid);
}
EVP_PKEY_get_default_digest_nid() returns the default digest associated
with the given PKEY. Since you're using RSA, pkey_ctrl is implemented by
crypto/rsa/rsa_ameth.c rsa_pkey_ctrl(), which has:
case ASN1_PKEY_CTRL_DEFAULT_MD_NID:
*(int *)arg2 = NID_sha1;
return 1;
Catch all that?
To make SHA-256 the default for RSA, we'd have to change that from NID_sha1
to NID_sha256...
I should also add that the other obvious/easy fix is to initialise digest in
openssl/req.c to the SHA-256 EVP. That only changes 'openssl req' though.
(and yes, clearly I've spent too much time in this code base recently... :)
Index: openssl.cnf
===
RCS file: /cvs/src/lib/libcrypto/openssl.cnf,v
retrieving revision 1.1
diff -u -p -r1.1 openssl.cnf
--- openssl.cnf 11 Apr 2014 22:51:53 - 1.1
+++ openssl.cnf 30 Sep 2014 22:42:53 -
@@ -7,7 +7,8 @@
[ req ]
-default_bits = 1024
+default_bits = 2048
+default_md = sha256
default_keyfile= privkey.pem
distinguished_name = req_distinguished_name
attributes = req_attributes
--
Action without study is fatal. Study without action is futile.
-- Mary Ritter Beard
Re: openssl.cnf req defaults - default_md sha256
I should also add that the other obvious/easy fix is to initialise digest in openssl/req.c to the SHA-256 EVP. That only changes 'openssl req' though. (and yes, clearly I've spent too much time in this code base recently... :) Index: openssl.cnf === RCS file: /cvs/src/lib/libcrypto/openssl.cnf,v retrieving revision 1.1 diff -u -p -r1.1 openssl.cnf --- openssl.cnf 11 Apr 2014 22:51:53 - 1.1 +++ openssl.cnf 30 Sep 2014 22:42:53 - @@ -7,7 +7,8 @@ [ req ] -default_bits = 1024 +default_bits = 2048 +default_md = sha256 default_keyfile = privkey.pem distinguished_name = req_distinguished_name attributes = req_attributes The following does this, however note that the default_bits of 1024 from openssl.cnf trumps the 2048 in the define... we probably should also stop making EVP_des_ede3_cbc() the default cipher... Index: req.c === RCS file: /cvs/src/usr.bin/openssl/req.c,v retrieving revision 1.2 diff -u -p -r1.2 req.c --- req.c 28 Aug 2014 14:23:52 - 1.2 +++ req.c 1 Oct 2014 08:59:54 - @@ -97,7 +97,7 @@ #define STRING_MASKstring_mask #define UTF8_INutf8 -#define DEFAULT_KEY_LENGTH 512 +#define DEFAULT_KEY_LENGTH 2048 #define MIN_KEY_LENGTH 384 @@ -187,6 +187,7 @@ req_main(int argc, char **argv) #ifndef OPENSSL_NO_DES cipher = EVP_des_ede3_cbc(); #endif + digest = EVP_sha256(); infile = NULL; outfile = NULL; -- Action without study is fatal. Study without action is futile. -- Mary Ritter Beard
Re: [Patch] use exit() directly in usage()
On Sat, Sep 27, 2014 at 07:10:01PM +0200, Fritjof Bornebusch wrote:
Hi,
Hi,
after usage() was called, there is no where you can go.
as suggested by otto@ and @nicm, the usage() functions are marked as
__dead.
fritjof
fritjof
Index: ci.c
===
RCS file: /cvs/src/usr.bin/rcs/ci.c,v
retrieving revision 1.217
diff -u -p -r1.217 ci.c
--- ci.c19 May 2014 19:42:24 - 1.217
+++ ci.c1 Oct 2014 08:38:31 -
@@ -89,7 +89,7 @@ static voidcheckin_parsekeyword(char *
static int checkin_update(struct checkin_params *);
static int checkin_revert(struct checkin_params *);
-void
+__dead void
checkin_usage(void)
{
fprintf(stderr,
@@ -97,6 +97,8 @@ checkin_usage(void)
[-j[rev]] [-k[rev]] [-l[rev]] [-M[rev]] [-mmsg]\n
[-Nsymbol] [-nsymbol] [-r[rev]] [-sstate] [-t[str]]\n
[-u[rev]] [-wusername] [-xsuffixes] [-ztz] file ...\n);
+
+ exit(1);
}
/*
@@ -221,7 +223,6 @@ checkin_main(int argc, char **argv)
break;
default:
(usage)();
- exit(1);
}
}
@@ -231,7 +232,6 @@ checkin_main(int argc, char **argv)
if (argc == 0) {
warnx(no input file);
(usage)();
- exit(1);
}
if ((pb.username = getlogin()) == NULL)
Index: co.c
===
RCS file: /cvs/src/usr.bin/rcs/co.c,v
retrieving revision 1.117
diff -u -p -r1.117 co.c
--- co.c16 Apr 2013 20:24:45 - 1.117
+++ co.c1 Oct 2014 08:38:40 -
@@ -79,7 +79,6 @@ checkout_main(int argc, char **argv)
if (RCS_KWEXP_INVAL(kflag)) {
warnx(invalid RCS keyword substitution mode);
(usage)();
- exit(1);
}
break;
case 'l':
@@ -141,7 +140,6 @@ checkout_main(int argc, char **argv)
break;
default:
(usage)();
- exit(1);
}
}
@@ -151,7 +149,6 @@ checkout_main(int argc, char **argv)
if (argc == 0) {
warnx(no input file);
(usage)();
- exit (1);
}
if ((username = getlogin()) == NULL)
@@ -222,13 +219,15 @@ checkout_main(int argc, char **argv)
return (ret);
}
-void
+__dead void
checkout_usage(void)
{
fprintf(stderr,
usage: co [-TV] [-ddate] [-f[rev]] [-I[rev]] [-kmode] [-l[rev]]\n
[-M[rev]] [-p[rev]] [-q[rev]] [-r[rev]] [-sstate]\n
[-u[rev]] [-w[user]] [-xsuffixes] [-ztz] file ...\n);
+
+ exit(1);
}
/*
Index: merge.c
===
RCS file: /cvs/src/usr.bin/rcs/merge.c,v
retrieving revision 1.7
diff -u -p -r1.7 merge.c
--- merge.c 23 Jul 2010 21:46:05 - 1.7
+++ merge.c 1 Oct 2014 08:38:52 -
@@ -77,7 +77,6 @@ merge_main(int argc, char **argv)
exit(0);
default:
(usage)();
- exit(D_ERROR);
}
}
argc -= optind;
@@ -86,7 +85,6 @@ merge_main(int argc, char **argv)
if (argc != 3) {
warnx(%s arguments, (argc 3) ? not enough : too many);
(usage)();
- exit(D_ERROR);
}
for (; labels 3; labels++)
@@ -113,9 +111,11 @@ merge_main(int argc, char **argv)
return (status);
}
-void
+__dead void
merge_usage(void)
{
(void)fprintf(stderr,
usage: merge [-EepqV] [-L label] file1 file2 file3\n);
+
+ exit(D_ERROR);
}
Index: rcsclean.c
===
RCS file: /cvs/src/usr.bin/rcs/rcsclean.c,v
retrieving revision 1.52
diff -u -p -r1.52 rcsclean.c
--- rcsclean.c 28 Jul 2010 09:07:11 - 1.52
+++ rcsclean.c 1 Oct 2014 08:39:05 -
@@ -60,7 +60,6 @@ rcsclean_main(int argc, char **argv)
if (RCS_KWEXP_INVAL(kflag)) {
warnx(invalid RCS keyword substitution mode);
(usage)();
- exit(1);
}
break;
case 'n':
@@ -90,7 +89,6 @@ rcsclean_main(int argc, char **argv)
break;
default:
(usage)();
- exit(1);
}
}
@@ -104,7 +102,6 @@ rcsclean_main(int argc, char **argv)
if ((dirp = opendir(.)) == NULL) {
warn(opendir);
(usage)();
-
Re: openssl.cnf req defaults - default_md sha256
On 2014/10/01 18:41, Joel Sing wrote:
On Wed, 1 Oct 2014, Stuart Henderson wrote:
Over the coming months, web browsers will progressively start to first
warn for certificate chains including SHA-1 hashes, then treat them
as insecure (including disabling certain content - scripts etc).
Chrome are initially doing this for certs expiring after Jan 2017,
but will progressively slide it forward to certs expiring after
Jan 2016.
Since my previous attempt to at least show this in ssl(8) examples
for openssl req a few months ago, I've spent some time digging for
where the defaults are set in the code as a nicer place to set sane
values, but haven't tracked it down yet. Would it be OK to set it
in the default config for now? (or does anyone have an idea of where
in the code this comes from?)
Welcome to libkitchensink...
I'd need to quadruple check, however this should come from openssl/req.c
do_X509_sign() being called with a NULL digest, which calls openssl/req.c
do_sign_init() with a NULL md, which calls crypto/evp/m_sigver.c
EVP_DigestSignInit() with md being NULL, which calls crypto/evp/m_sigver.c
do_sigver_init() with type being NULL, which results in:
if (type == NULL) {
int def_nid;
if (EVP_PKEY_get_default_digest_nid(pkey, def_nid) 0)
type = EVP_get_digestbynid(def_nid);
}
EVP_PKEY_get_default_digest_nid() returns the default digest associated with
the given PKEY. Since you're using RSA, pkey_ctrl is implemented by
crypto/rsa/rsa_ameth.c rsa_pkey_ctrl(), which has:
case ASN1_PKEY_CTRL_DEFAULT_MD_NID:
*(int *)arg2 = NID_sha1;
return 1;
Catch all that?
To make SHA-256 the default for RSA, we'd have to change that from NID_sha1
to
NID_sha256...
(and yes, clearly I've spent too much time in this code base recently... :)
Bingo :) So, this has the desired effect. The default key size is of
course set somewhere *completely* different. Good results with this test:
printf '\n\n\n\n\ntest\n\n\n\n' | openssl req -new -newkey rsa \
-nodes -keyout key -noout -text -verify 2/dev/null |
grep -e Signature.Alg -e Public-Key
I've included an openssl.cnf patch again but this time with the default
hash and key size commented-out, and a number of other things not
necessary to a default file removed (if we want a more complete example
file it can go in /etc/examples, this might be useful sometime to show
people how to setup subjectAlternativeName etc, but that's out of scope
for this diff).
Index: lib/libssl/src/crypto/rsa/rsa_ameth.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/rsa/rsa_ameth.c,v
retrieving revision 1.12
diff -u -p -r1.12 rsa_ameth.c
--- lib/libssl/src/crypto/rsa/rsa_ameth.c 11 Jul 2014 12:59:10 -
1.12
+++ lib/libssl/src/crypto/rsa/rsa_ameth.c 1 Oct 2014 09:16:39 -
@@ -433,7 +433,7 @@ rsa_pkey_ctrl(EVP_PKEY *pkey, int op, lo
#endif
case ASN1_PKEY_CTRL_DEFAULT_MD_NID:
- *(int *)arg2 = NID_sha1;
+ *(int *)arg2 = NID_sha256;
return 1;
default:
Index: usr.bin/openssl/req.c
===
RCS file: /cvs/src/usr.bin/openssl/req.c,v
retrieving revision 1.2
diff -u -p -r1.2 req.c
--- usr.bin/openssl/req.c 28 Aug 2014 14:23:52 - 1.2
+++ usr.bin/openssl/req.c 1 Oct 2014 09:16:39 -
@@ -97,7 +97,7 @@
#define STRING_MASKstring_mask
#define UTF8_INutf8
-#define DEFAULT_KEY_LENGTH 512
+#define DEFAULT_KEY_LENGTH 2048
#define MIN_KEY_LENGTH 384
Index: usr.bin/openssl/openssl.1
===
RCS file: /cvs/src/usr.bin/openssl/openssl.1,v
retrieving revision 1.3
diff -u -p -r1.3 openssl.1
--- usr.bin/openssl/openssl.1 16 Sep 2014 16:05:44 - 1.3
+++ usr.bin/openssl/openssl.1 1 Oct 2014 09:16:39 -
@@ -5774,7 +5774,7 @@ They are currently ignored by
request signing utilities, but some CAs might want them.
.It Ar default_bits
This specifies the default key size in bits.
-If not specified, 512 is used.
+If not specified, 2048 is used.
It is used if the
.Fl new
option is used.
@@ -5790,10 +5790,11 @@ option.
.It Ar default_md
This option specifies the digest algorithm to use.
Possible values include
-.Ar md5
+.Ar md5 ,
+.Ar sha1
and
-.Ar sha1 .
-If not present, MD5 is used.
+.Ar sha256 .
+If not present, SHA256 is used.
This option can be overridden on the command line.
.It Ar distinguished_name
This specifies the section containing the distinguished name fields to
Index: lib/libcrypto/openssl.cnf
===
RCS file: /cvs/src/lib/libcrypto/openssl.cnf,v
retrieving revision 1.1
diff -u -p -r1.1 openssl.cnf
---
Re: openssl.cnf req defaults - default_md sha256
On 2014/10/01 19:05, Joel Sing wrote: I should also add that the other obvious/easy fix is to initialise digest in openssl/req.c to the SHA-256 EVP. That only changes 'openssl req' though. (and yes, clearly I've spent too much time in this code base recently... :) Index: openssl.cnf === RCS file: /cvs/src/lib/libcrypto/openssl.cnf,v retrieving revision 1.1 diff -u -p -r1.1 openssl.cnf --- openssl.cnf 11 Apr 2014 22:51:53 - 1.1 +++ openssl.cnf 30 Sep 2014 22:42:53 - @@ -7,7 +7,8 @@ [ req ] -default_bits = 1024 +default_bits = 2048 +default_md = sha256 default_keyfile= privkey.pem distinguished_name = req_distinguished_name attributes = req_attributes The following does this, however note that the default_bits of 1024 from openssl.cnf trumps the 2048 in the define... we probably should also stop making EVP_des_ede3_cbc() the default cipher... I think I prefer it this way (changing usr.bin/openssl rather than the library) as there's less risk of impact in unpredictable areas. How about this one? Index: usr.bin/openssl/req.c === RCS file: /cvs/src/usr.bin/openssl/req.c,v retrieving revision 1.2 diff -u -p -r1.2 req.c --- usr.bin/openssl/req.c 28 Aug 2014 14:23:52 - 1.2 +++ usr.bin/openssl/req.c 1 Oct 2014 09:51:37 - @@ -97,7 +97,7 @@ #define STRING_MASKstring_mask #define UTF8_INutf8 -#define DEFAULT_KEY_LENGTH 512 +#define DEFAULT_KEY_LENGTH 2048 #define MIN_KEY_LENGTH 384 @@ -184,9 +184,8 @@ req_main(int argc, char **argv) unsigned long chtype = MBSTRING_ASC; req_conf = NULL; -#ifndef OPENSSL_NO_DES - cipher = EVP_des_ede3_cbc(); -#endif + cipher = EVP_aes_256_cbc(); + digest = EVP_sha256(); infile = NULL; outfile = NULL; Index: lib/libcrypto/openssl.cnf === RCS file: /cvs/src/lib/libcrypto/openssl.cnf,v retrieving revision 1.1 diff -u -p -r1.1 openssl.cnf --- lib/libcrypto/openssl.cnf 11 Apr 2014 22:51:53 - 1.1 +++ lib/libcrypto/openssl.cnf 1 Oct 2014 09:51:36 - @@ -1,41 +1,20 @@ -# -# OpenSSL example configuration file. -# This is mostly being used for generation of certificate requests. -# - -RANDFILE = /dev/arandom - - [ req ] -default_bits = 1024 -default_keyfile= privkey.pem +#default_bits = 2048 +#default_md= sha256 +#default_keyfile = privkey.pem distinguished_name = req_distinguished_name attributes = req_attributes [ req_distinguished_name ] countryName= Country Name (2 letter code) -#countryName_default = AU countryName_min= 2 countryName_max= 2 - stateOrProvinceName= State or Province Name (full name) -#stateOrProvinceName_default = Some-State - localityName = Locality Name (eg, city) - 0.organizationName = Organization Name (eg, company) -#0.organizationName_default= Internet Widgits Pty Ltd - -# we can do this but it is not needed normally :-) -#1.organizationName= Second Organization Name (eg, company) -#1.organizationName_default= CryptSoft Pty Ltd - organizationalUnitName = Organizational Unit Name (eg, section) -#organizationalUnitName_default= - commonName = Common Name (eg, fully qualified host name) commonName_max = 64 - emailAddress = Email Address emailAddress_max = 64 @@ -43,23 +22,3 @@ emailAddress_max = 64 challengePassword = A challenge password challengePassword_min = 4 challengePassword_max = 20 - -unstructuredName = An optional company name - -[ x509v3_extensions ] - -nsCaRevocationUrl = http://www.cryptsoft.com/ca-crl.pem -nsComment = This is a comment - -# under ASN.1, the 0 bit would be encoded as 80 -nsCertType = 0x40 - -#nsBaseUrl -#nsRevocationUrl -#nsRenewalUrl -#nsCaPolicyUrl -#nsSslServerName -#nsCertSequence -#nsCertExt -#nsDataType - Index: usr.bin/openssl/openssl.1 === RCS file: /cvs/src/usr.bin/openssl/openssl.1,v retrieving revision 1.3 diff -u -p -r1.3 openssl.1 --- usr.bin/openssl/openssl.1 16 Sep 2014 16:05:44 - 1.3 +++ usr.bin/openssl/openssl.1 1 Oct 2014 09:51:37 - @@ -5583,7 +5583,7 @@ This gives the to write the newly created
Re: [Patch] use exit() directly in usage()
Looks good but you have missed out ident.c and rcsprog.c
On Wed, Oct 01, 2014 at 11:19:29AM +0200, Fritjof Bornebusch wrote:
On Sat, Sep 27, 2014 at 07:10:01PM +0200, Fritjof Bornebusch wrote:
Hi,
Hi,
after usage() was called, there is no where you can go.
as suggested by otto@ and @nicm, the usage() functions are marked as
__dead.
fritjof
fritjof
Index: ci.c
===
RCS file: /cvs/src/usr.bin/rcs/ci.c,v
retrieving revision 1.217
diff -u -p -r1.217 ci.c
--- ci.c 19 May 2014 19:42:24 - 1.217
+++ ci.c 1 Oct 2014 08:38:31 -
@@ -89,7 +89,7 @@ static void checkin_parsekeyword(char *
static intcheckin_update(struct checkin_params *);
static intcheckin_revert(struct checkin_params *);
-void
+__dead void
checkin_usage(void)
{
fprintf(stderr,
@@ -97,6 +97,8 @@ checkin_usage(void)
[-j[rev]] [-k[rev]] [-l[rev]] [-M[rev]] [-mmsg]\n
[-Nsymbol] [-nsymbol] [-r[rev]] [-sstate] [-t[str]]\n
[-u[rev]] [-wusername] [-xsuffixes] [-ztz] file ...\n);
+
+ exit(1);
}
/*
@@ -221,7 +223,6 @@ checkin_main(int argc, char **argv)
break;
default:
(usage)();
- exit(1);
}
}
@@ -231,7 +232,6 @@ checkin_main(int argc, char **argv)
if (argc == 0) {
warnx(no input file);
(usage)();
- exit(1);
}
if ((pb.username = getlogin()) == NULL)
Index: co.c
===
RCS file: /cvs/src/usr.bin/rcs/co.c,v
retrieving revision 1.117
diff -u -p -r1.117 co.c
--- co.c 16 Apr 2013 20:24:45 - 1.117
+++ co.c 1 Oct 2014 08:38:40 -
@@ -79,7 +79,6 @@ checkout_main(int argc, char **argv)
if (RCS_KWEXP_INVAL(kflag)) {
warnx(invalid RCS keyword substitution mode);
(usage)();
- exit(1);
}
break;
case 'l':
@@ -141,7 +140,6 @@ checkout_main(int argc, char **argv)
break;
default:
(usage)();
- exit(1);
}
}
@@ -151,7 +149,6 @@ checkout_main(int argc, char **argv)
if (argc == 0) {
warnx(no input file);
(usage)();
- exit (1);
}
if ((username = getlogin()) == NULL)
@@ -222,13 +219,15 @@ checkout_main(int argc, char **argv)
return (ret);
}
-void
+__dead void
checkout_usage(void)
{
fprintf(stderr,
usage: co [-TV] [-ddate] [-f[rev]] [-I[rev]] [-kmode] [-l[rev]]\n
[-M[rev]] [-p[rev]] [-q[rev]] [-r[rev]] [-sstate]\n
[-u[rev]] [-w[user]] [-xsuffixes] [-ztz] file ...\n);
+
+ exit(1);
}
/*
Index: merge.c
===
RCS file: /cvs/src/usr.bin/rcs/merge.c,v
retrieving revision 1.7
diff -u -p -r1.7 merge.c
--- merge.c 23 Jul 2010 21:46:05 - 1.7
+++ merge.c 1 Oct 2014 08:38:52 -
@@ -77,7 +77,6 @@ merge_main(int argc, char **argv)
exit(0);
default:
(usage)();
- exit(D_ERROR);
}
}
argc -= optind;
@@ -86,7 +85,6 @@ merge_main(int argc, char **argv)
if (argc != 3) {
warnx(%s arguments, (argc 3) ? not enough : too many);
(usage)();
- exit(D_ERROR);
}
for (; labels 3; labels++)
@@ -113,9 +111,11 @@ merge_main(int argc, char **argv)
return (status);
}
-void
+__dead void
merge_usage(void)
{
(void)fprintf(stderr,
usage: merge [-EepqV] [-L label] file1 file2 file3\n);
+
+ exit(D_ERROR);
}
Index: rcsclean.c
===
RCS file: /cvs/src/usr.bin/rcs/rcsclean.c,v
retrieving revision 1.52
diff -u -p -r1.52 rcsclean.c
--- rcsclean.c28 Jul 2010 09:07:11 - 1.52
+++ rcsclean.c1 Oct 2014 08:39:05 -
@@ -60,7 +60,6 @@ rcsclean_main(int argc, char **argv)
if (RCS_KWEXP_INVAL(kflag)) {
warnx(invalid RCS keyword substitution mode);
(usage)();
- exit(1);
}
break;
case 'n':
@@ -90,7 +89,6 @@ rcsclean_main(int argc, char **argv)
break;
default:
(usage)();
- exit(1);
}
}
@@ -104,7 +102,6 @@ rcsclean_main(int argc, char
Re: [Patch] use exit() directly in usage()
On Wed, Oct 01, 2014 at 06:41:25PM +0100, Nicholas Marriott wrote:
Looks good but you have missed out ident.c and rcsprog.c
Ups, sorry.
On Wed, Oct 01, 2014 at 11:19:29AM +0200, Fritjof Bornebusch wrote:
On Sat, Sep 27, 2014 at 07:10:01PM +0200, Fritjof Bornebusch wrote:
Hi,
Hi,
after usage() was called, there is no where you can go.
as suggested by otto@ and @nicm, the usage() functions are marked as
__dead.
fritjof
fritjof
fritjof
Index: ci.c
===
RCS file: /cvs/src/usr.bin/rcs/ci.c,v
retrieving revision 1.217
diff -u -p -r1.217 ci.c
--- ci.c19 May 2014 19:42:24 - 1.217
+++ ci.c1 Oct 2014 08:38:31 -
@@ -89,7 +89,7 @@ static voidcheckin_parsekeyword(char *
static int checkin_update(struct checkin_params *);
static int checkin_revert(struct checkin_params *);
-void
+__dead void
checkin_usage(void)
{
fprintf(stderr,
@@ -97,6 +97,8 @@ checkin_usage(void)
[-j[rev]] [-k[rev]] [-l[rev]] [-M[rev]] [-mmsg]\n
[-Nsymbol] [-nsymbol] [-r[rev]] [-sstate] [-t[str]]\n
[-u[rev]] [-wusername] [-xsuffixes] [-ztz] file ...\n);
+
+ exit(1);
}
/*
@@ -221,7 +223,6 @@ checkin_main(int argc, char **argv)
break;
default:
(usage)();
- exit(1);
}
}
@@ -231,7 +232,6 @@ checkin_main(int argc, char **argv)
if (argc == 0) {
warnx(no input file);
(usage)();
- exit(1);
}
if ((pb.username = getlogin()) == NULL)
Index: co.c
===
RCS file: /cvs/src/usr.bin/rcs/co.c,v
retrieving revision 1.117
diff -u -p -r1.117 co.c
--- co.c16 Apr 2013 20:24:45 - 1.117
+++ co.c1 Oct 2014 08:38:40 -
@@ -79,7 +79,6 @@ checkout_main(int argc, char **argv)
if (RCS_KWEXP_INVAL(kflag)) {
warnx(invalid RCS keyword substitution mode);
(usage)();
- exit(1);
}
break;
case 'l':
@@ -141,7 +140,6 @@ checkout_main(int argc, char **argv)
break;
default:
(usage)();
- exit(1);
}
}
@@ -151,7 +149,6 @@ checkout_main(int argc, char **argv)
if (argc == 0) {
warnx(no input file);
(usage)();
- exit (1);
}
if ((username = getlogin()) == NULL)
@@ -222,13 +219,15 @@ checkout_main(int argc, char **argv)
return (ret);
}
-void
+__dead void
checkout_usage(void)
{
fprintf(stderr,
usage: co [-TV] [-ddate] [-f[rev]] [-I[rev]] [-kmode] [-l[rev]]\n
[-M[rev]] [-p[rev]] [-q[rev]] [-r[rev]] [-sstate]\n
[-u[rev]] [-w[user]] [-xsuffixes] [-ztz] file ...\n);
+
+ exit(1);
}
/*
Index: merge.c
===
RCS file: /cvs/src/usr.bin/rcs/merge.c,v
retrieving revision 1.7
diff -u -p -r1.7 merge.c
--- merge.c 23 Jul 2010 21:46:05 - 1.7
+++ merge.c 1 Oct 2014 08:38:52 -
@@ -77,7 +77,6 @@ merge_main(int argc, char **argv)
exit(0);
default:
(usage)();
- exit(D_ERROR);
}
}
argc -= optind;
@@ -86,7 +85,6 @@ merge_main(int argc, char **argv)
if (argc != 3) {
warnx(%s arguments, (argc 3) ? not enough : too many);
(usage)();
- exit(D_ERROR);
}
for (; labels 3; labels++)
@@ -113,9 +111,11 @@ merge_main(int argc, char **argv)
return (status);
}
-void
+__dead void
merge_usage(void)
{
(void)fprintf(stderr,
usage: merge [-EepqV] [-L label] file1 file2 file3\n);
+
+ exit(D_ERROR);
}
Index: rcsclean.c
===
RCS file: /cvs/src/usr.bin/rcs/rcsclean.c,v
retrieving revision 1.52
diff -u -p -r1.52 rcsclean.c
--- rcsclean.c 28 Jul 2010 09:07:11 - 1.52
+++ rcsclean.c 1 Oct 2014 08:39:05 -
@@ -60,7 +60,6 @@ rcsclean_main(int argc, char **argv)
if (RCS_KWEXP_INVAL(kflag)) {
warnx(invalid RCS keyword substitution mode);
(usage)();
- exit(1);
}
break;
case 'n':
@@ -90,7 +89,6 @@ rcsclean_main(int argc, char **argv)
break;
default:
Re: minor fix in sys/arch/i386/i386/apm.c
On Fri, Sep 26, 2014 at 10:32:53PM -0400, thev...@openmailbox.org wrote: i don't know if this is considered a bug, so i sent it here instead. found this when compiling with 'option APMDEBUG': cc -Werror -Wall -Wstrict-prototypes -Wmissing-prototypes -Wno-main -Wno-uninitialized -Wstack-larger-than-2047 -fno-builtin-printf -fno-builtin-snprintf -fno-builtin-vsnprintf -fno-builtin-log -fno-builtin-log2 -fno-builtin-malloc -fno-pie -fno-stack-protector -Os -mtune=i486 -pipe -nostdinc -I../../../.. -I. -I../../../../arch -DDEBUG -DSCSITERSE -DSMALL_KERNEL -DNO_PROPOLICE -DTIMEZONE=0 -DDST=0 -DFFS -DFFS2 -DEXT2FS -DNFSCLIENT -DCD9660 -DUDF -DMSDOSFS -DINET -DINET6 -DBOOT_CONFIG -DCRYPTO -DRAMDISK_HOOKS -DMINIROOTSIZE=0xf20 -DAPMDEBUG -DPCIVERBOSE -DMAXUSERS=4 -D_KERNEL -MD -MP -c ../../../../arch/i386/i386/apm.c cc1: warnings being treated as errors ../../../../arch/i386/i386/apm.c: In function 'apmattach': ../../../../arch/i386/i386/apm.c:803: warning: format '%x' expects type 'unsigned int', but argument 4 has type 'bus_space_handle_t' ../../../../arch/i386/i386/apm.c:803: warning: format '%x' expects type 'unsigned int', but argument 7 has type 'bus_space_handle_t' ../../../../arch/i386/i386/apm.c:803: warning: format '%x' expects type 'unsigned int', but argument 10 has type 'bus_space_handle_t' ../../../../arch/i386/i386/apm.c:803: warning: format '%x' expects type 'unsigned int', but argument 14 has type 'long unsigned int' *** Error 1 in /usr/src/sys/arch/i386/compile/RAMDISK_CD (Makefile:655 'apm.o') *** Error 1 in /usr/src/distrib/i386/ramdisk_cd (../common/Makefile.inc:73 'bsd') --- apm.c.origFri Sep 19 20:40:13 2014 +++ apm.c Fri Sep 26 22:14:00 2014 @@ -800,8 +800,8 @@ apmattach(struct device *parent, struct device *self, SDT_MEMERA, SEL_KPL, 0, 0); setgdt(GAPMDATA_SEL, (void *)dh, ap-apm_data_len, SDT_MEMRWA, SEL_KPL, 1, 0); - DPRINTF((: flags %x code 32:%x/%x[%x] 16:%x/%x[%x] - data %x/%x/%x ep %x (%x:%x)\n%s, apm_flags, + DPRINTF((: flags %x code 32:%x/%lx[%x] 16:%x/%lx[%x] + data %x/%lx/%x ep %x (%x:%lx)\n%s, apm_flags, ap-apm_code32_base, ch32, ap-apm_code_len, ap-apm_code16_base, ch16, ap-apm_code16_len, ap-apm_data_base, dh, ap-apm_data_len, Thanks, committed.
[patch]lock and unlock like GnuRCS
Hi tech,
the OpenRCS rcs command produces the following output if -l and -u is
used in the same command:
$ rcs -l1.1 -u1.1 foo.txt
RCS file: foo.txt,v
1.1 locked
1.1 unlocked
$ rcs -u1.1 -l1.1 foo.txt
RCS file: foo.txt,v
1.1 locked
1.1 unlocked
I've looked at GnuRCS and it has another way to handle these parameters
(it seems the other BSDs use GnuRCS, too).
Debian 7.5:
$ rcs -l1.1 -u1.1 foo.txt
RCS file: foo.txt,v
rcs: foo.txt,v: no lock set on revision 1.1
1.1 locked
$ rcs -u1.1 -l1.1 foo.txt
Segmentation fault
Well, I think the Segmentation fault isn't that important :), but GnuRCS
does not lock and unlock a file by using the same command like OpenRCS.
I think the different implementations of RCS should share the same
behaviour:
$ rcs -l1.1 -u1.1 foo.txt
RCS file: foo.txt,v
1.1 locked
done
$ rcs -u1.1 -l1.1 foo.txt
RCS file: foo.txt,v
1.1 unlocked
done
fritjof
Index: rcsprog.c
===
RCS file: /cvs/src/usr.bin/rcs/rcsprog.c,v
retrieving revision 1.151
diff -u -p -r1.151 rcsprog.c
--- rcsprog.c 12 Jul 2011 21:00:32 - 1.151
+++ rcsprog.c 3 Aug 2014 15:42:34 -
@@ -234,9 +234,10 @@ rcs_main(int argc, char **argv)
lkmode = RCS_LOCK_STRICT;
break;
case 'l':
- /* XXX - Check with -u flag. */
- lrev = rcs_optarg;
- rcsflags |= RCSPROG_LFLAG;
+ if (!(rcsflags RCSPROG_UFLAG)) {
+ lrev = rcs_optarg;
+ rcsflags |= RCSPROG_LFLAG;
+ }
break;
case 'm':
if (logstr != NULL)
@@ -272,9 +273,10 @@ rcs_main(int argc, char **argv)
lkmode = RCS_LOCK_LOOSE;
break;
case 'u':
- /* XXX - Check with -l flag. */
- urev = rcs_optarg;
- rcsflags |= RCSPROG_UFLAG;
+ if (!(rcsflags RCSPROG_LFLAG)) {
+ urev = rcs_optarg;
+ rcsflags |= RCSPROG_UFLAG;
+ }
break;
case 'V':
printf(%s\n, rcs_version);
Re: [Patch] use exit() directly in usage()
Please do not change to EXIT_FAILURE.
Original message
From: Kent R. Spillner kspill...@acm.org
Date: 01/10/2014 23:41 (GMT+00:00)
To: Fritjof Bornebusch frit...@alokat.org
Cc: Nicholas Marriott
nicholas.marri...@gmail.com,tech@openbsd.org,o...@openbsd.org
Subject: Re: [Patch] use exit() directly in usage()
ok kspillner@. It would be nice to replace some of those exit(1) with
exit(EXIT_FAILURE), but that should be a separate diff that also updates the
err/errx's as well.
On Oct 1, 2014, at 13:06, Fritjof Bornebusch frit...@alokat.org wrote:
On Wed, Oct 01, 2014 at 06:41:25PM +0100, Nicholas Marriott wrote:
Looks good but you have missed out ident.c and rcsprog.c
Ups, sorry.
On Wed, Oct 01, 2014 at 11:19:29AM +0200, Fritjof Bornebusch wrote:
On Sat, Sep 27, 2014 at 07:10:01PM +0200, Fritjof Bornebusch wrote:
Hi,
Hi,
after usage() was called, there is no where you can go.
as suggested by otto@ and @nicm, the usage() functions are marked as
__dead.
fritjof
fritjof
fritjof
Index: ci.c
===
RCS file: /cvs/src/usr.bin/rcs/ci.c,v
retrieving revision 1.217
diff -u -p -r1.217 ci.c
--- ci.c 19 May 2014 19:42:24 - 1.217
+++ ci.c 1 Oct 2014 08:38:31 -
@@ -89,7 +89,7 @@ static void checkin_parsekeyword(char *
static int checkin_update(struct checkin_params *);
static int checkin_revert(struct checkin_params *);
-void
+__dead void
checkin_usage(void)
{
fprintf(stderr,
@@ -97,6 +97,8 @@ checkin_usage(void)
[-j[rev]] [-k[rev]] [-l[rev]] [-M[rev]] [-mmsg]\n
[-Nsymbol] [-nsymbol] [-r[rev]] [-sstate] [-t[str]]\n
[-u[rev]] [-wusername] [-xsuffixes] [-ztz] file ...\n);
+
+ exit(1);
}
/*
@@ -221,7 +223,6 @@ checkin_main(int argc, char **argv)
break;
default:
(usage)();
- exit(1);
}
}
@@ -231,7 +232,6 @@ checkin_main(int argc, char **argv)
if (argc == 0) {
warnx(no input file);
(usage)();
- exit(1);
}
if ((pb.username = getlogin()) == NULL)
Index: co.c
===
RCS file: /cvs/src/usr.bin/rcs/co.c,v
retrieving revision 1.117
diff -u -p -r1.117 co.c
--- co.c 16 Apr 2013 20:24:45 - 1.117
+++ co.c 1 Oct 2014 08:38:40 -
@@ -79,7 +79,6 @@ checkout_main(int argc, char **argv)
if (RCS_KWEXP_INVAL(kflag)) {
warnx(invalid RCS keyword substitution mode);
(usage)();
- exit(1);
}
break;
case 'l':
@@ -141,7 +140,6 @@ checkout_main(int argc, char **argv)
break;
default:
(usage)();
- exit(1);
}
}
@@ -151,7 +149,6 @@ checkout_main(int argc, char **argv)
if (argc == 0) {
warnx(no input file);
(usage)();
- exit (1);
}
if ((username = getlogin()) == NULL)
@@ -222,13 +219,15 @@ checkout_main(int argc, char **argv)
return (ret);
}
-void
+__dead void
checkout_usage(void)
{
fprintf(stderr,
usage: co [-TV] [-ddate] [-f[rev]] [-I[rev]] [-kmode] [-l[rev]]\n
[-M[rev]] [-p[rev]] [-q[rev]] [-r[rev]] [-sstate]\n
[-u[rev]] [-w[user]] [-xsuffixes] [-ztz] file ...\n);
+
+ exit(1);
}
/*
Index: merge.c
===
RCS file: /cvs/src/usr.bin/rcs/merge.c,v
retrieving revision 1.7
diff -u -p -r1.7 merge.c
--- merge.c 23 Jul 2010 21:46:05 - 1.7
+++ merge.c 1 Oct 2014 08:38:52 -
@@ -77,7 +77,6 @@ merge_main(int argc, char **argv)
exit(0);
default:
(usage)();
- exit(D_ERROR);
}
}
argc -= optind;
@@ -86,7 +85,6 @@ merge_main(int argc, char **argv)
if (argc != 3) {
warnx(%s arguments, (argc 3) ? not enough : too many);
(usage)();
- exit(D_ERROR);
}
for (; labels 3; labels++)
@@ -113,9 +111,11 @@ merge_main(int argc, char **argv)
return (status);
}
-void
+__dead void
merge_usage(void)
{
(void)fprintf(stderr,
usage: merge [-EepqV] [-L label] file1 file2 file3\n);
+
+ exit(D_ERROR);
}
Index: rcsclean.c
===
RCS file: /cvs/src/usr.bin/rcs/rcsclean.c,v
retrieving revision 1.52
diff -u -p -r1.52 rcsclean.c
--- rcsclean.c 28 Jul 2010 09:07:11 - 1.52
+++ rcsclean.c 1 Oct 2014 08:39:05 -
@@ -60,7 +60,6 @@ rcsclean_main(int argc, char **argv)
if (RCS_KWEXP_INVAL(kflag)) {
warnx(invalid RCS keyword substitution mode);
(usage)();
- exit(1);
}
break;
case 'n':
@@ -90,7 +89,6 @@ rcsclean_main(int argc, char **argv)
Re: [patch]lock and unlock like GnuRCS
The existing behaviour isn't wildly useful, makes sense to me, ok nicm
On Wed, Oct 01, 2014 at 11:33:33PM +0200, Fritjof Bornebusch wrote:
Hi tech,
the OpenRCS rcs command produces the following output if -l and -u is
used in the same command:
$ rcs -l1.1 -u1.1 foo.txt
RCS file: foo.txt,v
1.1 locked
1.1 unlocked
$ rcs -u1.1 -l1.1 foo.txt
RCS file: foo.txt,v
1.1 locked
1.1 unlocked
I've looked at GnuRCS and it has another way to handle these parameters
(it seems the other BSDs use GnuRCS, too).
Debian 7.5:
$ rcs -l1.1 -u1.1 foo.txt
RCS file: foo.txt,v
rcs: foo.txt,v: no lock set on revision 1.1
1.1 locked
$ rcs -u1.1 -l1.1 foo.txt
Segmentation fault
Well, I think the Segmentation fault isn't that important :), but GnuRCS
does not lock and unlock a file by using the same command like OpenRCS.
I think the different implementations of RCS should share the same
behaviour:
$ rcs -l1.1 -u1.1 foo.txt
RCS file: foo.txt,v
1.1 locked
done
$ rcs -u1.1 -l1.1 foo.txt
RCS file: foo.txt,v
1.1 unlocked
done
fritjof
Index: rcsprog.c
===
RCS file: /cvs/src/usr.bin/rcs/rcsprog.c,v
retrieving revision 1.151
diff -u -p -r1.151 rcsprog.c
--- rcsprog.c 12 Jul 2011 21:00:32 - 1.151
+++ rcsprog.c 3 Aug 2014 15:42:34 -
@@ -234,9 +234,10 @@ rcs_main(int argc, char **argv)
lkmode = RCS_LOCK_STRICT;
break;
case 'l':
- /* XXX - Check with -u flag. */
- lrev = rcs_optarg;
- rcsflags |= RCSPROG_LFLAG;
+ if (!(rcsflags RCSPROG_UFLAG)) {
+ lrev = rcs_optarg;
+ rcsflags |= RCSPROG_LFLAG;
+ }
break;
case 'm':
if (logstr != NULL)
@@ -272,9 +273,10 @@ rcs_main(int argc, char **argv)
lkmode = RCS_LOCK_LOOSE;
break;
case 'u':
- /* XXX - Check with -l flag. */
- urev = rcs_optarg;
- rcsflags |= RCSPROG_UFLAG;
+ if (!(rcsflags RCSPROG_LFLAG)) {
+ urev = rcs_optarg;
+ rcsflags |= RCSPROG_UFLAG;
+ }
break;
case 'V':
printf(%s\n, rcs_version);
Re: [patch]lock and unlock like GnuRCS
posix commands (like ls(1) for example) keep the last option when mutually
exclusive options are specified. does it make sense to keep rcs consistent with
that convention? also is a man page diff needed?
On Oct 1, 2014, at 7:17 PM, Nicholas Marriott nicholas.marri...@gmail.com
wrote:
The existing behaviour isn't wildly useful, makes sense to me, ok nicm
On Wed, Oct 01, 2014 at 11:33:33PM +0200, Fritjof Bornebusch wrote:
Hi tech,
the OpenRCS rcs command produces the following output if -l and -u is
used in the same command:
$ rcs -l1.1 -u1.1 foo.txt
RCS file: foo.txt,v
1.1 locked
1.1 unlocked
$ rcs -u1.1 -l1.1 foo.txt
RCS file: foo.txt,v
1.1 locked
1.1 unlocked
I've looked at GnuRCS and it has another way to handle these parameters
(it seems the other BSDs use GnuRCS, too).
Debian 7.5:
$ rcs -l1.1 -u1.1 foo.txt
RCS file: foo.txt,v
rcs: foo.txt,v: no lock set on revision 1.1
1.1 locked
$ rcs -u1.1 -l1.1 foo.txt
Segmentation fault
Well, I think the Segmentation fault isn't that important :), but GnuRCS
does not lock and unlock a file by using the same command like OpenRCS.
I think the different implementations of RCS should share the same
behaviour:
$ rcs -l1.1 -u1.1 foo.txt
RCS file: foo.txt,v
1.1 locked
done
$ rcs -u1.1 -l1.1 foo.txt
RCS file: foo.txt,v
1.1 unlocked
done
fritjof
Index: rcsprog.c
===
RCS file: /cvs/src/usr.bin/rcs/rcsprog.c,v
retrieving revision 1.151
diff -u -p -r1.151 rcsprog.c
--- rcsprog.c12 Jul 2011 21:00:32 -1.151
+++ rcsprog.c3 Aug 2014 15:42:34 -
@@ -234,9 +234,10 @@ rcs_main(int argc, char **argv)
lkmode = RCS_LOCK_STRICT;
break;
case 'l':
-/* XXX - Check with -u flag. */
-lrev = rcs_optarg;
-rcsflags |= RCSPROG_LFLAG;
+if (!(rcsflags RCSPROG_UFLAG)) {
+lrev = rcs_optarg;
+rcsflags |= RCSPROG_LFLAG;
+}
break;
case 'm':
if (logstr != NULL)
@@ -272,9 +273,10 @@ rcs_main(int argc, char **argv)
lkmode = RCS_LOCK_LOOSE;
break;
case 'u':
-/* XXX - Check with -l flag. */
-urev = rcs_optarg;
-rcsflags |= RCSPROG_UFLAG;
+if (!(rcsflags RCSPROG_LFLAG)) {
+urev = rcs_optarg;
+rcsflags |= RCSPROG_UFLAG;
+}
break;
case 'V':
printf(%s\n, rcs_version);
Re: [patch]lock and unlock like GnuRCS
Matching GNU RCS seems preferable to me but I don't feel strongly about
it.
I wouldn't mention this in the man page, it hardly seems like behaviour
anyone should (or will need to) rely on.
On Wed, Oct 01, 2014 at 07:41:52PM -0400, Daniel Dickman wrote:
posix commands (like ls(1) for example) keep the last option when mutually
exclusive options are specified. does it make sense to keep rcs consistent
with that convention? also is a man page diff needed?
On Oct 1, 2014, at 7:17 PM, Nicholas Marriott nicholas.marri...@gmail.com
wrote:
The existing behaviour isn't wildly useful, makes sense to me, ok nicm
On Wed, Oct 01, 2014 at 11:33:33PM +0200, Fritjof Bornebusch wrote:
Hi tech,
the OpenRCS rcs command produces the following output if -l and -u is
used in the same command:
$ rcs -l1.1 -u1.1 foo.txt
RCS file: foo.txt,v
1.1 locked
1.1 unlocked
$ rcs -u1.1 -l1.1 foo.txt
RCS file: foo.txt,v
1.1 locked
1.1 unlocked
I've looked at GnuRCS and it has another way to handle these parameters
(it seems the other BSDs use GnuRCS, too).
Debian 7.5:
$ rcs -l1.1 -u1.1 foo.txt
RCS file: foo.txt,v
rcs: foo.txt,v: no lock set on revision 1.1
1.1 locked
$ rcs -u1.1 -l1.1 foo.txt
Segmentation fault
Well, I think the Segmentation fault isn't that important :), but GnuRCS
does not lock and unlock a file by using the same command like OpenRCS.
I think the different implementations of RCS should share the same
behaviour:
$ rcs -l1.1 -u1.1 foo.txt
RCS file: foo.txt,v
1.1 locked
done
$ rcs -u1.1 -l1.1 foo.txt
RCS file: foo.txt,v
1.1 unlocked
done
fritjof
Index: rcsprog.c
===
RCS file: /cvs/src/usr.bin/rcs/rcsprog.c,v
retrieving revision 1.151
diff -u -p -r1.151 rcsprog.c
--- rcsprog.c12 Jul 2011 21:00:32 -1.151
+++ rcsprog.c3 Aug 2014 15:42:34 -
@@ -234,9 +234,10 @@ rcs_main(int argc, char **argv)
lkmode = RCS_LOCK_STRICT;
break;
case 'l':
-/* XXX - Check with -u flag. */
-lrev = rcs_optarg;
-rcsflags |= RCSPROG_LFLAG;
+if (!(rcsflags RCSPROG_UFLAG)) {
+lrev = rcs_optarg;
+rcsflags |= RCSPROG_LFLAG;
+}
break;
case 'm':
if (logstr != NULL)
@@ -272,9 +273,10 @@ rcs_main(int argc, char **argv)
lkmode = RCS_LOCK_LOOSE;
break;
case 'u':
-/* XXX - Check with -l flag. */
-urev = rcs_optarg;
-rcsflags |= RCSPROG_UFLAG;
+if (!(rcsflags RCSPROG_LFLAG)) {
+urev = rcs_optarg;
+rcsflags |= RCSPROG_UFLAG;
+}
break;
case 'V':
printf(%s\n, rcs_version);
Re: [patch]lock and unlock like GnuRCS
OTOH, check out what we do with rcs -L and -U...
On Thu, Oct 02, 2014 at 12:54:13AM +0100, Nicholas Marriott wrote:
Matching GNU RCS seems preferable to me but I don't feel strongly about
it.
I wouldn't mention this in the man page, it hardly seems like behaviour
anyone should (or will need to) rely on.
On Wed, Oct 01, 2014 at 07:41:52PM -0400, Daniel Dickman wrote:
posix commands (like ls(1) for example) keep the last option when mutually
exclusive options are specified. does it make sense to keep rcs consistent
with that convention? also is a man page diff needed?
On Oct 1, 2014, at 7:17 PM, Nicholas Marriott
nicholas.marri...@gmail.com wrote:
The existing behaviour isn't wildly useful, makes sense to me, ok nicm
On Wed, Oct 01, 2014 at 11:33:33PM +0200, Fritjof Bornebusch wrote:
Hi tech,
the OpenRCS rcs command produces the following output if -l and -u is
used in the same command:
$ rcs -l1.1 -u1.1 foo.txt
RCS file: foo.txt,v
1.1 locked
1.1 unlocked
$ rcs -u1.1 -l1.1 foo.txt
RCS file: foo.txt,v
1.1 locked
1.1 unlocked
I've looked at GnuRCS and it has another way to handle these parameters
(it seems the other BSDs use GnuRCS, too).
Debian 7.5:
$ rcs -l1.1 -u1.1 foo.txt
RCS file: foo.txt,v
rcs: foo.txt,v: no lock set on revision 1.1
1.1 locked
$ rcs -u1.1 -l1.1 foo.txt
Segmentation fault
Well, I think the Segmentation fault isn't that important :), but
GnuRCS
does not lock and unlock a file by using the same command like OpenRCS.
I think the different implementations of RCS should share the same
behaviour:
$ rcs -l1.1 -u1.1 foo.txt
RCS file: foo.txt,v
1.1 locked
done
$ rcs -u1.1 -l1.1 foo.txt
RCS file: foo.txt,v
1.1 unlocked
done
fritjof
Index: rcsprog.c
===
RCS file: /cvs/src/usr.bin/rcs/rcsprog.c,v
retrieving revision 1.151
diff -u -p -r1.151 rcsprog.c
--- rcsprog.c12 Jul 2011 21:00:32 -1.151
+++ rcsprog.c3 Aug 2014 15:42:34 -
@@ -234,9 +234,10 @@ rcs_main(int argc, char **argv)
lkmode = RCS_LOCK_STRICT;
break;
case 'l':
-/* XXX - Check with -u flag. */
-lrev = rcs_optarg;
-rcsflags |= RCSPROG_LFLAG;
+if (!(rcsflags RCSPROG_UFLAG)) {
+lrev = rcs_optarg;
+rcsflags |= RCSPROG_LFLAG;
+}
break;
case 'm':
if (logstr != NULL)
@@ -272,9 +273,10 @@ rcs_main(int argc, char **argv)
lkmode = RCS_LOCK_LOOSE;
break;
case 'u':
-/* XXX - Check with -l flag. */
-urev = rcs_optarg;
-rcsflags |= RCSPROG_UFLAG;
+if (!(rcsflags RCSPROG_LFLAG)) {
+urev = rcs_optarg;
+rcsflags |= RCSPROG_UFLAG;
+}
break;
case 'V':
printf(%s\n, rcs_version);
Re: [Patch] use exit() directly in usage()
ok kspillner@. It would be nice to replace some of those exit(1) with
exit(EXIT_FAILURE), but that should be a separate diff that also updates the
err/errx's as well.
On Oct 1, 2014, at 13:06, Fritjof Bornebusch frit...@alokat.org wrote:
On Wed, Oct 01, 2014 at 06:41:25PM +0100, Nicholas Marriott wrote:
Looks good but you have missed out ident.c and rcsprog.c
Ups, sorry.
On Wed, Oct 01, 2014 at 11:19:29AM +0200, Fritjof Bornebusch wrote:
On Sat, Sep 27, 2014 at 07:10:01PM +0200, Fritjof Bornebusch wrote:
Hi,
Hi,
after usage() was called, there is no where you can go.
as suggested by otto@ and @nicm, the usage() functions are marked as
__dead.
fritjof
fritjof
fritjof
Index: ci.c
===
RCS file: /cvs/src/usr.bin/rcs/ci.c,v
retrieving revision 1.217
diff -u -p -r1.217 ci.c
--- ci.c19 May 2014 19:42:24 -1.217
+++ ci.c1 Oct 2014 08:38:31 -
@@ -89,7 +89,7 @@ static void checkin_parsekeyword(char *
static int checkin_update(struct checkin_params *);
static int checkin_revert(struct checkin_params *);
-void
+__dead void
checkin_usage(void)
{
fprintf(stderr,
@@ -97,6 +97,8 @@ checkin_usage(void)
[-j[rev]] [-k[rev]] [-l[rev]] [-M[rev]] [-mmsg]\n
[-Nsymbol] [-nsymbol] [-r[rev]] [-sstate] [-t[str]]\n
[-u[rev]] [-wusername] [-xsuffixes] [-ztz] file ...\n);
+
+exit(1);
}
/*
@@ -221,7 +223,6 @@ checkin_main(int argc, char **argv)
break;
default:
(usage)();
-exit(1);
}
}
@@ -231,7 +232,6 @@ checkin_main(int argc, char **argv)
if (argc == 0) {
warnx(no input file);
(usage)();
-exit(1);
}
if ((pb.username = getlogin()) == NULL)
Index: co.c
===
RCS file: /cvs/src/usr.bin/rcs/co.c,v
retrieving revision 1.117
diff -u -p -r1.117 co.c
--- co.c16 Apr 2013 20:24:45 -1.117
+++ co.c1 Oct 2014 08:38:40 -
@@ -79,7 +79,6 @@ checkout_main(int argc, char **argv)
if (RCS_KWEXP_INVAL(kflag)) {
warnx(invalid RCS keyword substitution mode);
(usage)();
-exit(1);
}
break;
case 'l':
@@ -141,7 +140,6 @@ checkout_main(int argc, char **argv)
break;
default:
(usage)();
-exit(1);
}
}
@@ -151,7 +149,6 @@ checkout_main(int argc, char **argv)
if (argc == 0) {
warnx(no input file);
(usage)();
-exit (1);
}
if ((username = getlogin()) == NULL)
@@ -222,13 +219,15 @@ checkout_main(int argc, char **argv)
return (ret);
}
-void
+__dead void
checkout_usage(void)
{
fprintf(stderr,
usage: co [-TV] [-ddate] [-f[rev]] [-I[rev]] [-kmode] [-l[rev]]\n
[-M[rev]] [-p[rev]] [-q[rev]] [-r[rev]] [-sstate]\n
[-u[rev]] [-w[user]] [-xsuffixes] [-ztz] file ...\n);
+
+exit(1);
}
/*
Index: merge.c
===
RCS file: /cvs/src/usr.bin/rcs/merge.c,v
retrieving revision 1.7
diff -u -p -r1.7 merge.c
--- merge.c23 Jul 2010 21:46:05 -1.7
+++ merge.c1 Oct 2014 08:38:52 -
@@ -77,7 +77,6 @@ merge_main(int argc, char **argv)
exit(0);
default:
(usage)();
-exit(D_ERROR);
}
}
argc -= optind;
@@ -86,7 +85,6 @@ merge_main(int argc, char **argv)
if (argc != 3) {
warnx(%s arguments, (argc 3) ? not enough : too many);
(usage)();
-exit(D_ERROR);
}
for (; labels 3; labels++)
@@ -113,9 +111,11 @@ merge_main(int argc, char **argv)
return (status);
}
-void
+__dead void
merge_usage(void)
{
(void)fprintf(stderr,
usage: merge [-EepqV] [-L label] file1 file2 file3\n);
+
+exit(D_ERROR);
}
Index: rcsclean.c
===
RCS file: /cvs/src/usr.bin/rcs/rcsclean.c,v
retrieving revision 1.52
diff -u -p -r1.52 rcsclean.c
--- rcsclean.c28 Jul 2010 09:07:11 -1.52
+++ rcsclean.c1 Oct 2014 08:39:05 -
@@ -60,7 +60,6 @@ rcsclean_main(int argc, char **argv)
if (RCS_KWEXP_INVAL(kflag)) {
warnx(invalid RCS keyword substitution mode);
(usage)();
-exit(1);
}
break;
case 'n':
@@ -90,7 +89,6 @@ rcsclean_main(int argc, char **argv)
break;
default:
(usage)();
-exit(1);
}
}
@@ -104,7 +102,6 @@ rcsclean_main(int argc, char **argv)
if ((dirp = opendir(.)) == NULL) {
warn(opendir);
(usage)();
-exit(1);
}
while ((dp = readdir(dirp)) != NULL) {
@@
mpe patch: use rt_ifa_{add,del}
This new diff aims to simplify the mpe(4) device and also to improve
the old code that handled the installation of MPLS interface routes.
I followed what mpi@ said:
On Tue, Sep 30, 2014 at 11:00:25AM +0200, Martin Pieuchot wrote:
Hello Rafael,
On 14/09/14(Sun) 23:49, Rafael Zalamena wrote:
The following patch is just a preparation for the code that is coming to
implement the wire network interface (the VPLS datapath) to work on OpenBSD.
This code turns the mpe code that handles route and labels into some general
use functions that will be called by mpe and wire.
Would it be possible to use the new rt_ifa_add() and rt_ifa_del() instead of
keeping what is basically a copy of the old rtinit()?
In your case you want to use the lladdr's ifa and you can check for
RTF_MPLS in the flags to add the corresponding MPLS_OP_POP value.
--- patch snipped ---
Code change:
* Moved label address from softc to lladdr ifa
* Changed rt_ifa_add to default RTF_MPLS routes to do a POP and only
use rdomain 0 (MPLS only works on domain 0, and it doesn't make sense
other actions when creating MPLS route to an interface)
* Removed old code that installed mpe MPLS routes
* Conflicting labels verification is now done by routing (see rt_ifa_add())
This was tested in the setup described in:
http://2011.eurobsdcon.org/papers/jeker/MPLS.pdf
Here is the diff:
diff --git sys/net/if_mpe.c sys/net/if_mpe.c
index 74039dc..8580ef3 100644
--- sys/net/if_mpe.c
+++ sys/net/if_mpe.c
@@ -61,7 +61,6 @@ int mpeioctl(struct ifnet *, u_long, caddr_t);
void mpestart(struct ifnet *);
intmpe_clone_create(struct if_clone *, int);
intmpe_clone_destroy(struct ifnet *);
-intmpe_newlabel(struct ifnet *, int, struct shim_hdr *);
LIST_HEAD(, mpe_softc) mpeif_list;
struct if_clonempe_cloner =
@@ -84,13 +83,19 @@ mpe_clone_create(struct if_clone *ifc, int unit)
{
struct ifnet*ifp;
struct mpe_softc*mpeif;
+ struct sockaddr_mpls*smpls;
int s;
if ((mpeif = malloc(sizeof(*mpeif),
M_DEVBUF, M_NOWAIT|M_ZERO)) == NULL)
return (ENOMEM);
- mpeif-sc_shim.shim_label = 0;
+ smpls = malloc(sizeof(*smpls), M_IFADDR, M_NOWAIT | M_ZERO);
+ if (smpls == NULL) {
+ free(mpeif, M_DEVBUF, 0);
+ return (ENOMEM);
+ }
+
mpeif-sc_unit = unit;
ifp = mpeif-sc_if;
snprintf(ifp-if_xname, sizeof ifp-if_xname, mpe%d, unit);
@@ -110,6 +115,10 @@ mpe_clone_create(struct if_clone *ifc, int unit)
bpfattach(ifp-if_bpf, ifp, DLT_LOOP, sizeof(u_int32_t));
#endif
+ smpls-smpls_family = AF_MPLS;
+ smpls-smpls_len = sizeof(*smpls);
+ ifp-if_lladdr-ifa_dstaddr = smplstosa(smpls);
+
s = splnet();
LIST_INSERT_HEAD(mpeif_list, mpeif, sc_list);
splx(s);
@@ -127,6 +136,7 @@ mpe_clone_destroy(struct ifnet *ifp)
LIST_REMOVE(mpeif, sc_list);
splx(s);
+ free(ifp-if_lladdr-ifa_dstaddr, M_IFADDR, 0);
if_detach(ifp);
free(mpeif, M_DEVBUF, 0);
return (0);
@@ -278,7 +288,7 @@ int
mpeioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
{
int error;
- struct mpe_softc*ifm;
+ struct sockaddr_mpls*smpls;
struct ifreq*ifr;
struct shim_hdr shim;
@@ -303,14 +313,13 @@ mpeioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
ifp-if_mtu = ifr-ifr_mtu;
break;
case SIOCGETLABEL:
- ifm = ifp-if_softc;
+ smpls = satosmpls(ifp-if_lladdr-ifa_dstaddr);
shim.shim_label =
- ((ntohl(ifm-sc_shim.shim_label MPLS_LABEL_MASK))
+ ((ntohl(smpls-smpls_label MPLS_LABEL_MASK))
MPLS_LABEL_OFFSET);
error = copyout(shim, ifr-ifr_data, sizeof(shim));
break;
case SIOCSETLABEL:
- ifm = ifp-if_softc;
if ((error = copyin(ifr-ifr_data, shim, sizeof(shim
break;
if (shim.shim_label MPLS_LABEL_MAX ||
@@ -319,36 +328,29 @@ mpeioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
break;
}
shim.shim_label = htonl(shim.shim_label MPLS_LABEL_OFFSET);
- if (ifm-sc_shim.shim_label == shim.shim_label)
- break;
- LIST_FOREACH(ifm, mpeif_list, sc_list) {
- if (ifm != ifp-if_softc
- ifm-sc_shim.shim_label == shim.shim_label) {
- error = EEXIST;
- break;
- }
- }
- if (error)
+
+ smpls = satosmpls(ifp-if_lladdr-ifa_dstaddr);
+ if (smpls-smpls_label == shim.shim_label)
Re: [patch]lock and unlock like GnuRCS
On Wed, Oct 1, 2014 at 4:41 PM, Daniel Dickman didick...@gmail.com wrote: posix commands (like ls(1) for example) keep the last option when mutually exclusive options are specified. does it make sense to keep rcs consistent with that convention? also is a man page diff needed? RCS predates POSIX and does not follow the POSIX utility argument guidelines multiple ways. Changing OpenRCS to follow those guidelines would make it incompatible with GNU RCS and the the deployed base of scripts...and fingers... Philip Guenther
Re: [patch]lock and unlock like GnuRCS
On Wed, Oct 01, 2014 at 07:58:16PM -0700, Philip Guenther wrote: On Wed, Oct 1, 2014 at 4:41 PM, Daniel Dickman didick...@gmail.com wrote: posix commands (like ls(1) for example) keep the last option when mutually exclusive options are specified. does it make sense to keep rcs consistent with that convention? also is a man page diff needed? RCS predates POSIX and does not follow the POSIX utility argument guidelines multiple ways. Changing OpenRCS to follow those guidelines would make it incompatible with GNU RCS and the the deployed base of scripts...and fingers... Philip Guenther Agreed, -Otto
Re: [Patch] use exit() directly in usage()
On Wed, Oct 01, 2014 at 05:41:11PM -0500, Kent R. Spillner wrote:
ok kspillner@. It would be nice to replace some of those exit(1) with
exit(EXIT_FAILURE), but that should be a separate diff that also updates the
err/errx's as well.
In general EXIT_XXX is frowned upon in OpenBSD.
-Otto
On Oct 1, 2014, at 13:06, Fritjof Bornebusch frit...@alokat.org wrote:
On Wed, Oct 01, 2014 at 06:41:25PM +0100, Nicholas Marriott wrote:
Looks good but you have missed out ident.c and rcsprog.c
Ups, sorry.
On Wed, Oct 01, 2014 at 11:19:29AM +0200, Fritjof Bornebusch wrote:
On Sat, Sep 27, 2014 at 07:10:01PM +0200, Fritjof Bornebusch wrote:
Hi,
Hi,
after usage() was called, there is no where you can go.
as suggested by otto@ and @nicm, the usage() functions are marked as
__dead.
fritjof
fritjof
fritjof
Index: ci.c
===
RCS file: /cvs/src/usr.bin/rcs/ci.c,v
retrieving revision 1.217
diff -u -p -r1.217 ci.c
--- ci.c19 May 2014 19:42:24 -1.217
+++ ci.c1 Oct 2014 08:38:31 -
@@ -89,7 +89,7 @@ static void checkin_parsekeyword(char *
static int checkin_update(struct checkin_params *);
static int checkin_revert(struct checkin_params *);
-void
+__dead void
checkin_usage(void)
{
fprintf(stderr,
@@ -97,6 +97,8 @@ checkin_usage(void)
[-j[rev]] [-k[rev]] [-l[rev]] [-M[rev]] [-mmsg]\n
[-Nsymbol] [-nsymbol] [-r[rev]] [-sstate] [-t[str]]\n
[-u[rev]] [-wusername] [-xsuffixes] [-ztz] file ...\n);
+
+exit(1);
}
/*
@@ -221,7 +223,6 @@ checkin_main(int argc, char **argv)
break;
default:
(usage)();
-exit(1);
}
}
@@ -231,7 +232,6 @@ checkin_main(int argc, char **argv)
if (argc == 0) {
warnx(no input file);
(usage)();
-exit(1);
}
if ((pb.username = getlogin()) == NULL)
Index: co.c
===
RCS file: /cvs/src/usr.bin/rcs/co.c,v
retrieving revision 1.117
diff -u -p -r1.117 co.c
--- co.c16 Apr 2013 20:24:45 -1.117
+++ co.c1 Oct 2014 08:38:40 -
@@ -79,7 +79,6 @@ checkout_main(int argc, char **argv)
if (RCS_KWEXP_INVAL(kflag)) {
warnx(invalid RCS keyword substitution mode);
(usage)();
-exit(1);
}
break;
case 'l':
@@ -141,7 +140,6 @@ checkout_main(int argc, char **argv)
break;
default:
(usage)();
-exit(1);
}
}
@@ -151,7 +149,6 @@ checkout_main(int argc, char **argv)
if (argc == 0) {
warnx(no input file);
(usage)();
-exit (1);
}
if ((username = getlogin()) == NULL)
@@ -222,13 +219,15 @@ checkout_main(int argc, char **argv)
return (ret);
}
-void
+__dead void
checkout_usage(void)
{
fprintf(stderr,
usage: co [-TV] [-ddate] [-f[rev]] [-I[rev]] [-kmode] [-l[rev]]\n
[-M[rev]] [-p[rev]] [-q[rev]] [-r[rev]] [-sstate]\n
[-u[rev]] [-w[user]] [-xsuffixes] [-ztz] file ...\n);
+
+exit(1);
}
/*
Index: merge.c
===
RCS file: /cvs/src/usr.bin/rcs/merge.c,v
retrieving revision 1.7
diff -u -p -r1.7 merge.c
--- merge.c23 Jul 2010 21:46:05 -1.7
+++ merge.c1 Oct 2014 08:38:52 -
@@ -77,7 +77,6 @@ merge_main(int argc, char **argv)
exit(0);
default:
(usage)();
-exit(D_ERROR);
}
}
argc -= optind;
@@ -86,7 +85,6 @@ merge_main(int argc, char **argv)
if (argc != 3) {
warnx(%s arguments, (argc 3) ? not enough : too many);
(usage)();
-exit(D_ERROR);
}
for (; labels 3; labels++)
@@ -113,9 +111,11 @@ merge_main(int argc, char **argv)
return (status);
}
-void
+__dead void
merge_usage(void)
{
(void)fprintf(stderr,
usage: merge [-EepqV] [-L label] file1 file2 file3\n);
+
+exit(D_ERROR);
}
Index: rcsclean.c
===
RCS file: /cvs/src/usr.bin/rcs/rcsclean.c,v
retrieving revision 1.52
diff -u -p -r1.52 rcsclean.c
--- rcsclean.c28 Jul 2010 09:07:11 -1.52
+++ rcsclean.c1 Oct 2014 08:39:05 -
@@ -60,7 +60,6 @@ rcsclean_main(int argc, char **argv)
if (RCS_KWEXP_INVAL(kflag)) {
warnx(invalid RCS keyword substitution mode);
(usage)();
-exit(1);
}
break;
case 'n':
@@ -90,7 +89,6 @@ rcsclean_main(int argc, char **argv)
break;
default:
