align options from ping6 and ping
Hi,
based on Theo's recent comment I thought I could take a shot at it, but
it seems that my tree is currently broken as I can't compile it:
/usr/src/sbin/ping6/ping6.c:745: error: implicit declaration of function
'pledge'
But I thought it might still be worthwhile to share the diff.
Basically I did the following:
- renamed ping6 option -m to -D
- renamed ping6 option -t to -o (as I would otherwise have a naming conflict)
- renamed ping6 option -h to -t
- renamed ping6 option -w to -O (-w is used by ping for a different matter)
I also wanted to change hoplimit to be ttl so it matches ping's code but ttl
is already in use within ping6.
If this would be ok I will gladly to the ping.8 diff.
Frank.
Index: sbin/ping6/ping6.c
===
RCS file: /cvs/src/sbin/ping6/ping6.c,v
retrieving revision 1.121
diff -u -p -u -r1.121 ping6.c
--- sbin/ping6/ping6.c 12 Oct 2015 18:32:18 - 1.121
+++ sbin/ping6/ping6.c 13 Oct 2015 11:19:41 -
@@ -177,7 +177,7 @@ char DOT = '.';
char *hostname;
int ident; /* process id to identify our packets */
u_int8_t nonce[8]; /* nonce field for node information */
-int hoplimit = -1; /* hoplimit */
+int maxttl = -1; /* maxttl */
/* counters */
long npackets; /* max packets to transmit */
@@ -255,7 +255,7 @@ main(int argc, char *argv[])
int usepktinfo = 0;
struct in6_pktinfo *pktinfo = NULL;
double intval;
- int mflag = 0;
+ int Dflag = 0;
uid_t uid;
u_int rtableid = 0;
@@ -270,7 +270,7 @@ main(int argc, char *argv[])
preload = 0;
datap = &outpack[ICMP6ECHOLEN + ICMP6ECHOTMLEN];
while ((ch = getopt(argc, argv,
- "a:b:c:dEefHg:h:I:i:l:mnNp:qS:s:tvV:w")) != -1) {
+ "a:b:c:DdEefg:H:I:i:l:NnOp:qS:s:tV:v")) != -1) {
switch (ch) {
case 'a':
{
@@ -325,6 +325,9 @@ main(int argc, char *argv[])
"number of packets to transmit is %s: %s",
errstr, optarg);
break;
+ case 'D':
+ Dflag++;
+ break;
case 'd':
options |= F_SO_DEBUG;
break;
@@ -348,11 +351,6 @@ main(int argc, char *argv[])
case 'H':
options |= F_HOSTNAME;
break;
- case 'h': /* hoplimit */
- hoplimit = strtonum(optarg, 0, IPV6_MAXHLIM, &errstr);
- if (errstr)
- errx(1, "hoplimit is %s: %s", errstr, optarg);
- break;
case 'I':
ifname = optarg;
options |= F_INTERFACE;
@@ -388,15 +386,20 @@ main(int argc, char *argv[])
errx(1, "preload value is %s: %s", errstr,
optarg);
break;
- case 'm':
- mflag++;
- break;
case 'n':
options &= ~F_HOSTNAME;
break;
case 'N':
options |= F_NIGROUP;
break;
+ case 'O':
+ options &= ~F_NOUSERDATA;
+ options |= F_FQDN;
+ break;
+ case 'o':
+ options &= ~F_NOUSERDATA;
+ options |= F_SUPTYPES;
+ break;
case 'p': /* fill buffer with user pattern */
options |= F_PINGFILLED;
fill((char *)datap, optarg);
@@ -429,9 +432,10 @@ main(int argc, char *argv[])
errx(1, "datalen value is %s: %s", errstr,
optarg);
break;
- case 't':
- options &= ~F_NOUSERDATA;
- options |= F_SUPTYPES;
+ case 't': /* maxttl */
+ maxttl = strtonum(optarg, 0, IPV6_MAXHLIM, &errstr);
+ if (errstr)
+ errx(1, "maxttl is %s: %s", errstr, optarg);
break;
case 'v':
options |= F_VERBOSE;
@@ -446,10 +450,6 @@ main(int argc, char *argv[])
sizeof(rtableid)) == -1)
err(1, "setsockopt SO_RTABLE");
break;
- case 'w':
- options &= ~F_NOUSERDATA;
- options |= F_FQDN;
- break;
default:
usage
typo in OPENBSD-RELAYD-MIB.txt
Regards,
Index: OPENBSD-RELAYD-MIB.txt
===
RCS file: /cvs/src/share/snmp/OPENBSD-RELAYD-MIB.txt,v
retrieving revision 1.1
diff -u -p -r1.1 OPENBSD-RELAYD-MIB.txt
--- OPENBSD-RELAYD-MIB.txt 19 Nov 2014 10:24:39 - 1.1
+++ OPENBSD-RELAYD-MIB.txt 14 Oct 2015 03:04:27 -
@@ -31,7 +31,7 @@ IMPORTS
MODULE-COMPLIANCE, OBJECT-GROUP
FROM SNMPv2-CONF;
-relaydMIBOjbects MODULE-IDENTITY
+relaydMIBObjects MODULE-IDENTITY
LAST-UPDATED"20140312Z"
ORGANIZATION"OpenBSD"
CONTACT-INFO""
@@ -40,7 +40,7 @@ relaydMIBOjbects MODULE-IDENTITY
DESCRIPTION "MIB describing relayd(8) information"
::= { openBSD 3 }
-relaydInfo OBJECT IDENTIFIER ::= { relaydMIBOjbects 2 }
+relaydInfo OBJECT IDENTIFIER ::= { relaydMIBObjects 2 }
--
-- "show redirects"
[patch] tcpdump segfault on malformed BGP AS_PATH update
Fix a segfault when printing a malformed BGP AS_PATH update due to ASN
extraction.
Better AS size extraction from AS paths: better heuristics (see
bgp_attr_get_as_size).
Also fixes output support for 4-byte ASNs. For example;
(AS_PATH[T] {500.500 513.65211})
becomes:
(AS_PATH[T] {500 500} 65211)
Collapses bgp_attr_print switch 2-byte and 4-byte path cases.
bgp_attr_get_as_size function copied from tcpdump Git repo master.
This brings behavior in-line with newer versions on Linux.
Finally, minor output tweak: add a space after BGP update attribute
ORIGINATOR_ID's flags to make consistent with other attributes.
Index: print-bgp.c
===
RCS file: /cvs/src/usr.sbin/tcpdump/print-bgp.c,v
retrieving revision 1.17
diff -u -p -r1.17 print-bgp.c
--- print-bgp.c 16 Jan 2015 06:40:21 - 1.17
+++ print-bgp.c 14 Oct 2015 00:15:13 -
@@ -140,6 +140,9 @@ struct bgp_attr {
#define BGP_CONFED_AS_SEQUENCE 3 /* draft-ietf-idr-rfc3065bis-01 */
#define BGP_CONFED_AS_SET 4 /* draft-ietf-idr-rfc3065bis-01 */
+#define BGP_AS_SEG_TYPE_MINBGP_AS_SET
+#define BGP_AS_SEG_TYPE_MAXBGP_CONFED_AS_SET
+
static struct tok bgp_as_path_segment_open_values[] = {
{ BGP_AS_SET, " {" },
{ BGP_AS_SEQUENCE, " " },
@@ -400,6 +403,63 @@ trunc:
}
#endif
+/*
+ * bgp_attr_get_as_size
+ *
+ * Try to find the size of the ASs encoded in an as-path. It is not obvious, as
+ * both Old speakers that do not support 4 byte AS, and the new speakers that
do
+ * support, exchange AS-Path with the same path-attribute type value 0x02.
+ */
+static int
+bgp_attr_get_as_size (u_int8_t bgpa_type, const u_char *pptr, int len)
+{
+const u_char *tptr = pptr;
+
+/*
+ * If the path attribute is the optional AS4 path type, then we already
+ * know, that ASs must be encoded in 4 byte format.
+ */
+if (bgpa_type == BGPTYPE_AS4_PATH) {
+return 4;
+}
+
+/*
+ * Let us assume that ASs are of 2 bytes in size, and check if the AS-Path
+ * TLV is good. If not, ask the caller to try with AS encoded as 4 bytes
+ * each.
+ */
+while (tptr < pptr + len) {
+TCHECK(tptr[0]);
+
+/*
+ * If we do not find a valid segment type, our guess might be wrong.
+ */
+if (tptr[0] < BGP_AS_SEG_TYPE_MIN || tptr[0] > BGP_AS_SEG_TYPE_MAX) {
+goto trunc;
+}
+TCHECK(tptr[1]);
+tptr += 2 + tptr[1] * 2;
+}
+
+/*
+ * If we correctly reached end of the AS path attribute data content,
+ * then most likely ASs were indeed encoded as 2 bytes.
+ */
+if (tptr == pptr + len) {
+return 2;
+}
+
+trunc:
+
+/*
+ * We can come here, either we did not have enough data, or if we
+ * try to decode 4 byte ASs in 2 byte format. Either way, return 4,
+ * so that calller can try to decode each AS as of 4 bytes. If indeed
+ * there was not enough data, it will crib and end the parse anyways.
+ */
+ return 4;
+}
+
static int
bgp_attr_print(const struct bgp_attr *attr, const u_char *dat, int len)
{
@@ -413,7 +473,6 @@ bgp_attr_print(const struct bgp_attr *at
p = dat;
tlen = len;
- asn_bytes = 0;
switch (attr->bgpa_type) {
case BGPTYPE_ORIGIN:
@@ -425,17 +484,8 @@ bgp_attr_print(const struct bgp_attr *at
}
break;
case BGPTYPE_AS4_PATH:
- asn_bytes = 4;
/* FALLTHROUGH */
case BGPTYPE_AS_PATH:
- /*
-* 2-byte speakers will receive AS4_PATH as well AS_PATH (2-byte).
-* 4-byte speakers will only receive AS_PATH but it will be 4-byte.
-* To identify which is the case, compare the length of the path
-* segment value in bytes, with the path segment length from the
-* message (counted in # of AS)
-*/
-
if (len % 2) {
printf(" invalid len");
break;
@@ -444,11 +494,19 @@ bgp_attr_print(const struct bgp_attr *at
printf(" empty");
break;
}
+
+ /*
+ * BGP updates exchanged between New speakers that support 4
+ * byte AS, ASs are always encoded in 4 bytes. There is no
+ * definitive way to find this, just by the packet's
+ * contents. So, check for packet's TLV's sanity assuming
+ * 2 bytes first, and it does not pass, assume that ASs are
+ * encoded in 4 bytes format and move on.
+ */
+asn_bytes = bgp_attr_get_as_size(attr->bgpa_type, dat, len);
+
while (p < dat + len) {
TCHECK(p[0]);
- if (asn_bytes == 0) {
- asn_bytes = (len-2)/p[1];
-
Re: preparing pfi_kif to MP world
> On 13 Oct 2015, at 21:12, Mike Belopuhov wrote:
>
> On Tue, Oct 13, 2015 at 20:36 +1000, David Gwynne wrote:
>>
>>> On 12 Oct 2015, at 12:00 AM, Alexandr Nedvedicky
>>> wrote:
>>>
>>> Hello,
>>>
>>> patch below introduces struct refcnt to pfi_kif structure. Patch also
>>> changes
>>> pfi_kif_get() function to always return a reference to pfi_kif instance.
>>>
>>> Furthermore existing functions pfi_kif_ref()/pfi_kif_unref() are thrown away
>>> in favor of pfi_kif_take()/pfi_kif_rele(), which follow naming convention
>>> set by refcnt_init(9). Patch also removes kif reference types (enum
>>> pfi_kif_refs).
>>>
>>> Patch also updates if_pfsync.c file. I'm bit puzzled with test as follows
>>> in pfsync_in_clr():
>>>
>>> 770 for (i = 0; i < count; i++) {
>>> 771 clr = (struct pfsync_clr *)buf + len * i;
>>> 772 creatorid = clr->creatorid;
>>> 773
>>> 774 if (clr->ifname[0] == '\0') {
>>> ...
>>> 783 } else {
>>> 784 if (pfi_kif_get(clr->ifname) == NULL)
>>> 789 continue;
>>> 790
>>>
>>> I could not make any sense of line 784. Are we just making sure particular
>>> kif
>>> object exists for clr->ifname? Note pfi_kif_get() creates kif for us if it
>>> does not exist. If we want to query for kif existence only patch offers
>>> pfi_kif_check() function. I need some advice here if we should keep, what's
>>> currently in patch or switch to pfi_kif_check().
>>
>> my name is probably against that code.
>>
>> i didn't realise that pfi_kif_get creates a kif on demand. however,
>> it also uses malloc(, , M_NOWAIT) to try and allocate it, which can
>> fail and cause pfi_kif_get to return NULL.
>>
>
> Revision 1.102 used to get the kif and check the state against it:
>
> if (si->s->creatorid == creatorid &&
> si->s->kif == kif) {
>
> Then in the rev 1.103 you have removed the check. The question is
> why did you do that? (-:
oh. i have no idea. it was a long time ago :(
putting it back makes sense to me, but id like the opinions of people who are
more aware of pf internals to agree with me too.
dlg
Re: sdiff repledge after fork/execvp and drop proc/exec
On Tue, Oct 13, 2015 at 05:29:37PM -0400, Rob Pierce wrote:
> More restrictive pledge(s) can be done if tmpdir is equal or subordinate to
> _PATH_TMP by using the "tmppath" request instead of the "wpath cpath" duo.
>
> For now just do the obvious repledge after the fork/execvp and drop proc
> and exec.
>
> Rob
>
> Index: sdiff.c
> ===
> RCS file: /cvs/src/usr.bin/sdiff/sdiff.c,v
> retrieving revision 1.33
> diff -u -p -r1.33 sdiff.c
> --- sdiff.c 10 Oct 2015 19:03:08 - 1.33
> +++ sdiff.c 13 Oct 2015 21:16:47 -
> @@ -314,6 +314,9 @@ main(int argc, char **argv)
> err(2, "could not fork");
> }
>
> + if (pledge("stdio rpath wpath cpath", NULL) == -1)
> + err(1, "pledge");
> +
> /* parent */
> /* We don't write to the pipe. */
> close(fd[1]);
I forgot to mention that we also need to consider the location of the output
file if the "-o" option is specified when considering tmppath or wpath/cpath
in the pledge(s).
Re: patch: native ed support
On Tue, Oct 13, 2015 at 06:43:31PM +0200, Tobias Stoeckmann wrote:
> - Check for our new /.// substitution expression
> - Be more restrictive about command parsing, so we do not
> allow commands like "10insert" instead of "10i".
And now, fix regression which I introduced with that diff, of course
it takes strncmp() now instead of strcmp() for s-subsitution. The
latest regress test, t17, covers this. This version passes just fine.
Thought about changing the fatal-calls to return -1 in get_command,
so we would be more relaxed towards garbage at the end of file that
looks like ed commands, e.g. "1,1a" (illegal, "a" takes only one
address, not a range).
The problem would be though that patch gets straight back into the
ed routine because it says "looks like an ed command", which basically
means an endless loop. Let's see if there is an ed-user who'll
complain about fatal() usage here.
Q: What does patch currently do with these illegal ed commands?
A: It passes it to ed. And ed continues after printing "?"...
Index: Makefile
===
RCS file: /cvs/src/usr.bin/patch/Makefile,v
retrieving revision 1.4
diff -u -p -r1.4 Makefile
--- Makefile16 May 2005 15:22:46 - 1.4
+++ Makefile13 Oct 2015 21:31:52 -
@@ -1,6 +1,6 @@
# $OpenBSD: Makefile,v 1.4 2005/05/16 15:22:46 espie Exp $
PROG= patch
-SRCS= patch.c pch.c inp.c util.c backupfile.c mkpath.c
+SRCS= patch.c pch.c inp.c util.c backupfile.c mkpath.c ed.c
.include
Index: ed.c
===
RCS file: ed.c
diff -N ed.c
--- /dev/null 1 Jan 1970 00:00:00 -
+++ ed.c13 Oct 2015 21:31:52 -
@@ -0,0 +1,342 @@
+/* $OpenBSD$ */
+
+/*
+ * Copyright (c) 2015 Tobias Stoeckmann
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include
+#include
+
+#include
+#include
+#include
+#include
+
+#include "common.h"
+#include "util.h"
+#include "pch.h"
+#include "inp.h"
+
+/* states of finite state machine */
+#define FSM_CMD1
+#define FSM_A 2
+#define FSM_C 3
+#define FSM_D 4
+#define FSM_I 5
+#define FSM_S 6
+
+#define SRC_INP1 /* line's origin is input file */
+#define SRC_PCH2 /* line's origin is patch file */
+
+#define S_PATTERN "/.//"
+
+static voidinit_lines(void);
+static voidfree_lines(void);
+static struct ed_line *get_line(LINENUM);
+static struct ed_line *create_line(off_t);
+static int valid_addr(LINENUM, LINENUM);
+static int get_command(void);
+static voidwrite_lines(char *);
+
+LIST_HEAD(ed_head, ed_line) head;
+struct ed_line {
+ LIST_ENTRY(ed_line) entries;
+ int src;
+ unsigned long subst;
+ union {
+ LINENUM lineno;
+ off_t seek;
+ } pos;
+};
+
+static LINENUM first_addr;
+static LINENUM second_addr;
+static LINENUM line_count;
+static struct ed_line *cline; /* current line */
+
+void
+do_ed_script(void)
+{
+ off_t linepos;
+ struct ed_line *nline;
+ LINENUM i, range;
+ int fsm;
+
+ init_lines();
+ cline = NULL;
+ fsm = FSM_CMD;
+
+ for (;;) {
+ linepos = ftello(pfp);
+ if (pgets(buf, sizeof buf, pfp) == NULL)
+ break;
+ p_input_line++;
+
+ if (fsm == FSM_CMD) {
+ if ((fsm = get_command()) == -1)
+ break;
+
+ switch (fsm) {
+ case FSM_C:
+ case FSM_D:
+ /* delete lines in specified range */
+ if (second_addr == -1)
+ range = 1;
+ else
+ range = second_addr - first_addr + 1;
+ for (i = 0; i < range; i++) {
+ nline = LIST_NEXT(cline, entries);
+ LIST_REMOVE(cline, entries);
+
sdiff repledge after fork/execvp and drop proc/exec
More restrictive pledge(s) can be done if tmpdir is equal or subordinate to
_PATH_TMP by using the "tmppath" request instead of the "wpath cpath" duo.
For now just do the obvious repledge after the fork/execvp and drop proc
and exec.
Rob
Index: sdiff.c
===
RCS file: /cvs/src/usr.bin/sdiff/sdiff.c,v
retrieving revision 1.33
diff -u -p -r1.33 sdiff.c
--- sdiff.c 10 Oct 2015 19:03:08 - 1.33
+++ sdiff.c 13 Oct 2015 21:16:47 -
@@ -314,6 +314,9 @@ main(int argc, char **argv)
err(2, "could not fork");
}
+ if (pledge("stdio rpath wpath cpath", NULL) == -1)
+ err(1, "pledge");
+
/* parent */
/* We don't write to the pipe. */
close(fd[1]);
mg(1) dired sort
This diff allows dired mode to sort directory contents alphabetically
or by date order. It uses the same key as emacs: 's', though the
extended command name is 'dired-sort'. Comments/ok?
-lum
Index: dired.c
===
RCS file: /cvs/src/usr.bin/mg/dired.c,v
retrieving revision 1.78
diff -u -p -u -p -r1.78 dired.c
--- dired.c 12 Oct 2015 19:08:39 - 1.78
+++ dired.c 13 Oct 2015 19:57:48 -
@@ -51,15 +51,20 @@ static int d_forwline(int, int);
static int d_backline(int, int);
static int d_killbuffer_cmd(int, int);
static int d_refreshbuffer(int, int);
+static int d_sort(int, int);
static void reaper(int);
static struct buffer *refreshbuffer(struct buffer *);
static int createlist(struct buffer *);
static void redelete(struct buffer *);
static char *findfname(struct line *, char *);
+static void finddotlsal(struct buffer *);
extern struct keymap_s helpmap, cXmap, metamap;
-const char DDELCHAR = 'D';
+const char DDELCHAR = 'D';
+const int LSALT = 4; /* 4 = -alt, 3 = -al) */
+charlsargs[5] = "-al"; /* see LSALT above */
+int lsarg_toggle = 0; /* 's' has implications for dot */
/*
* Structure which holds a linked list of file names marked for
@@ -134,7 +139,7 @@ static PF diredn[] = {
d_backline, /* p */
d_killbuffer_cmd, /* q */
d_rename, /* r */
- rescan, /* s */
+ d_sort, /* s */
rescan, /* t */
d_undel,/* u */
rescan, /* v */
@@ -212,6 +217,7 @@ dired_init(void)
funmap_add(d_rename, "dired-do-rename");
funmap_add(d_backpage, "dired-scroll-down");
funmap_add(d_forwpage, "dired-scroll-up");
+ funmap_add(d_sort, "dired-sort");
funmap_add(d_undel, "dired-unmark");
funmap_add(d_killbuffer_cmd, "quit-window");
maps_add((KEYMAP *)&diredmap, "dired");
@@ -750,7 +756,16 @@ refreshbuffer(struct buffer *bp)
if (ddel)
redelete(bp);
- /* find dot line */
+ /* If required, find suitable dot line if toggling sort order. */
+ if (lsarg_toggle && (strlen(lsargs) == LSALT)) {
+ tmp_w_dotline = 2; /* 2nd line for -alt */
+ lsarg_toggle = 0;
+ } else if (lsarg_toggle) {
+ finddotlsal(bp);/* or after ".." if -al */
+ lsarg_toggle = 0;
+ curbp = bp;
+ return (bp);
+ }
bp->b_dotp = bfirstlp(bp);
if (tmp_w_dotline > bp->b_lines)
tmp_w_dotline = bp->b_lines - 1;
@@ -848,7 +863,6 @@ struct buffer *
dired_(char *dname)
{
struct buffer *bp;
- int i;
size_t len;
if ((dname = adjustname(dname, TRUE)) == NULL) {
@@ -881,26 +895,11 @@ dired_(char *dname)
bp = bfind(dname, TRUE);
bp->b_flag |= BFREADONLY | BFIGNDIRTY;
- if ((d_exec(2, bp, NULL, "ls", "-al", dname, NULL)) != TRUE)
+ if ((d_exec(2, bp, NULL, "ls", lsargs, dname, NULL)) != TRUE)
return (NULL);
- /* Find the line with ".." on it. */
- bp->b_dotp = bfirstlp(bp);
- bp->b_dotline = 1;
- for (i = 0; i < bp->b_lines; i++) {
- bp->b_dotp = lforw(bp->b_dotp);
- bp->b_dotline++;
- if (d_warpdot(bp->b_dotp, &bp->b_doto) == FALSE)
- continue;
- if (strcmp(ltext(bp->b_dotp) + bp->b_doto, "..") == 0)
- break;
- }
+ finddotlsal(bp);
- /* We want dot on the entry right after "..", if possible. */
- if (++i < bp->b_lines - 2) {
- bp->b_dotp = lforw(bp->b_dotp);
- bp->b_dotline++;
- }
d_warpdot(bp->b_dotp, &bp->b_doto);
(void)strlcpy(bp->b_fname, dname, sizeof(bp->b_fname));
@@ -1035,4 +1034,46 @@ findfname(struct line *lp, char *fn)
return NULL;
fn = &lp->l_text[start];
return fn;
+}
+
+static int
+d_sort(int f, int n)
+{
+ struct buffer *bp;
+
+ if (strlen(lsargs) == LSALT)
+ (void)snprintf(lsargs, sizeof(lsargs), "-al");
+ else
+ (void)snprintf(lsargs, sizeof(lsargs), "-alt");
+
+ lsarg_toggle = 1;
+
+ if ((bp = refreshbuffer(curbp)) == NULL)
+ return (FALSE);
+ return (showbuffer(bp, curwp, WFFULL | WFMODE));
+}
+
+static void
+finddotlsal(struct buffer *bp)
+{
+ int i;
+
+ bp->b_dotp = bfirstlp(bp);
+ bp->b_dotline = 1;
+ for (i = 0; i < bp->b_lines; i++) {
+ bp->b_dotp = lforw(bp->b_dotp);
+ bp->b_dotline++;
+ if (d_warpdot(bp->b_dotp, &bp->b_doto) == FALSE)
+
Document lidsuspend in sysctl(8)
Index: sbin/sysctl/sysctl.8 === RCS file: /cvs/src/sbin/sysctl/sysctl.8,v retrieving revision 1.187 diff -u -p -r1.187 sysctl.8 --- sbin/sysctl/sysctl.83 Oct 2015 09:17:13 - 1.187 +++ sbin/sysctl/sysctl.813 Oct 2015 19:09:03 - @@ -374,6 +374,7 @@ and a few require a kernel compiled with .It machdep.xcrypt Ta integer Ta no .It machdep.allowaperture Ta integer Ta yes .It machdep.led_blink Ta integer Ta yes +.It machdep.lidsuspend Ta integer Ta yes .It machdep.ceccerrs Ta integer Ta no .It machdep.cecclast Ta quad Ta no .It ddb.radix Ta integer Ta yes
use machdep.lidsuspend=2 for hibernate
This diff has been sitting in my patches folder for weeks now - posting
it now for discussion. What it does is to extend the machdep.lidsuspend
sysctl to also cover hibernate. I think it might be interesting for
users of a laptop/netbook where suspend tends to drain the battery, like
my MSI Wind U100 does. Unfortunately The U100 doesn't even report lid
status, but that's another story...
The machdep.lidsuspend sysctl is currently used like this: The value
zero does nothing and every other value causes the system to suspend
when the lid is being closed. I propose slightly different semantics:
The value zero does nothing (as before), the value one causes the
system to suspend (as before) and the value two initiates hibernation.
All other values do not cause any effect.
cheers,
natano
(tested on my Thinkpad T520 / amd64)
Index: etc/etc.amd64/sysctl.conf
===
RCS file: /cvs/src/etc/etc.amd64/sysctl.conf,v
retrieving revision 1.6
diff -u -p -u -r1.6 sysctl.conf
--- etc/etc.amd64/sysctl.conf 16 Jun 2015 20:30:24 - 1.6
+++ etc/etc.amd64/sysctl.conf 12 Oct 2015 20:44:39 -
@@ -1,3 +1,4 @@
#machdep.allowaperture=2 # See xf86(4)
#machdep.kbdreset=1# permit console CTRL-ALT-DEL to do a nice halt
#machdep.lidsuspend=0 # do not suspend laptop upon lid closing
+#machdep.lidsuspend=2 # put system into hibernation upon lid closing
Index: etc/etc.i386/sysctl.conf
===
RCS file: /cvs/src/etc/etc.i386/sysctl.conf,v
retrieving revision 1.18
diff -u -p -u -r1.18 sysctl.conf
--- etc/etc.i386/sysctl.conf16 Jun 2015 20:30:24 - 1.18
+++ etc/etc.i386/sysctl.conf12 Oct 2015 20:44:39 -
@@ -2,6 +2,7 @@
#machdep.apmhalt=1 # 1=powerdown hack, try if halt -p doesn't work
#machdep.kbdreset=1# permit console CTRL-ALT-DEL to do a nice halt
#machdep.lidsuspend=0 # do not suspend laptop upon lid closing
+#machdep.lidsuspend=2 # put system into hibernation upon lid closing
#machdep.userldt=1 # allow userland programs to play with ldt,
# required by some ports
#kern.emul.linux=1 # enable running Linux binaries
Index: sys/dev/acpi/acpibtn.c
===
RCS file: /cvs/src/sys/dev/acpi/acpibtn.c,v
retrieving revision 1.41
diff -u -p -u -r1.41 acpibtn.c
--- sys/dev/acpi/acpibtn.c 27 Jan 2015 19:40:14 - 1.41
+++ sys/dev/acpi/acpibtn.c 12 Oct 2015 20:44:49 -
@@ -16,11 +16,16 @@
*/
#include
+#include
#include
#include
#include
#include
+#ifdef HIBERNATE
+#include
+#endif
+
#include
#include
@@ -225,10 +230,28 @@ acpibtn_notify(struct aml_node *node, in
"_LID", 0, NULL, &lid))
return (0);
sc->sc_sens.value = lid;
- if (lid_suspend == 0)
+
+ if (lid != 0)
break;
- if (lid == 0)
+
+ switch (lid_suspend) {
+ case 1:
goto sleep;
+#ifdef HIBERNATE
+ case 2:
+ if (get_hibernate_io_function(swdevt[0].sw_dev) == NULL)
+ return (EOPNOTSUPP);
+
+ /* Request to go to hibernation */
+ if (acpi_record_event(sc->sc_acpi,
APM_USER_HIBERNATE_REQ))
+ acpi_addtask(sc->sc_acpi, acpi_sleep_task,
+ sc->sc_acpi, ACPI_STATE_S4);
+ break;
+#endif /* HIBERNATE */
+ case 0:
+ default:
+ break;
+ }
#endif /* SMALL_KERNEL */
break;
case ACPIBTN_SLEEP:
Re: patch: native ed support
On Mon, Oct 12, 2015 at 12:01:49PM -0600, Todd C. Miller wrote:
> I think this is the correct approach. I've only taken a brief look
> so far but I think you should make write_lines() static in its
> declaration to match the prototype.
Thanks for pointing this out. Updated diff below:
- Make write_lines static
- Fixed typo in comment (finit_e_ state machine)
- Check for our new /.// substitution expression
- Be more restrictive about command parsing, so we do not
allow commands like "10insert" instead of "10i".
- The variable p in get_command was not initialized when calling
isidigit, my bad...
Tobias
Index: Makefile
===
RCS file: /cvs/src/usr.bin/patch/Makefile,v
retrieving revision 1.4
diff -u -p -r1.4 Makefile
--- Makefile16 May 2005 15:22:46 - 1.4
+++ Makefile13 Oct 2015 16:33:09 -
@@ -1,6 +1,6 @@
# $OpenBSD: Makefile,v 1.4 2005/05/16 15:22:46 espie Exp $
PROG= patch
-SRCS= patch.c pch.c inp.c util.c backupfile.c mkpath.c
+SRCS= patch.c pch.c inp.c util.c backupfile.c mkpath.c ed.c
.include
Index: ed.c
===
RCS file: ed.c
diff -N ed.c
--- /dev/null 1 Jan 1970 00:00:00 -
+++ ed.c13 Oct 2015 16:33:09 -
@@ -0,0 +1,340 @@
+/* $OpenBSD$ */
+
+/*
+ * Copyright (c) 2015 Tobias Stoeckmann
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include
+#include
+
+#include
+#include
+#include
+#include
+
+#include "common.h"
+#include "util.h"
+#include "pch.h"
+#include "inp.h"
+
+/* states of finite state machine */
+#define FSM_CMD1
+#define FSM_A 2
+#define FSM_C 3
+#define FSM_D 4
+#define FSM_I 5
+#define FSM_S 6
+
+#define SRC_INP1 /* line's origin is input file */
+#define SRC_PCH2 /* line's origin is patch file */
+
+static voidinit_lines(void);
+static voidfree_lines(void);
+static struct ed_line *get_line(LINENUM);
+static struct ed_line *create_line(off_t);
+static int valid_addr(LINENUM, LINENUM);
+static int get_command(void);
+static voidwrite_lines(char *);
+
+LIST_HEAD(ed_head, ed_line) head;
+struct ed_line {
+ LIST_ENTRY(ed_line) entries;
+ int src;
+ unsigned long subst;
+ union {
+ LINENUM lineno;
+ off_t seek;
+ } pos;
+};
+
+static LINENUM first_addr;
+static LINENUM second_addr;
+static LINENUM line_count;
+static struct ed_line *cline; /* current line */
+
+void
+do_ed_script(void)
+{
+ off_t linepos;
+ struct ed_line *nline;
+ LINENUM i, range;
+ int fsm;
+
+ init_lines();
+ cline = NULL;
+ fsm = FSM_CMD;
+
+ for (;;) {
+ linepos = ftello(pfp);
+ if (pgets(buf, sizeof buf, pfp) == NULL)
+ break;
+ p_input_line++;
+
+ if (fsm == FSM_CMD) {
+ if ((fsm = get_command()) == -1)
+ break;
+
+ switch (fsm) {
+ case FSM_C:
+ case FSM_D:
+ /* delete lines in specified range */
+ if (second_addr == -1)
+ range = 1;
+ else
+ range = second_addr - first_addr + 1;
+ for (i = 0; i < range; i++) {
+ nline = LIST_NEXT(cline, entries);
+ LIST_REMOVE(cline, entries);
+ free(cline);
+ cline = nline;
+ line_count--;
+ }
+ fsm = (fsm == FSM_C) ? FSM_I : FSM_CMD;
+ break;
+ case FSM_S:
+ cline->subst++;
+ fsm = FSM_CMD;
+
lmw/stmw and corrupted frame on G5
On my G5s it is impossible to "c[ontinue]" execution after breaking into ddb(4). Doing so always result in a: panic: trap 9300 at 101000 (ddb_trap+0x40) lr 0x1b Note that 0x9300 == 0x8000 | 0x1300, so it seems that EXC_BPT that is set to enter ddb(8) has not been cleared (or better say the previous value has not been properly restored). I tracked down the problem to the "stmw" instruction in ddb_trap as adding an "isync" right after this instruction "fixes" the problem: @@ -1264,6 +1264,7 @@ _C_LABEL(ddb_trap): isync GET_CPUINFO(%r3) stmw%r28,CI_DDBSAVE(%r3) + isync I'm far from being a PowerPC expert so I'm really interested in hearing what others think of this issue. Nonetheless the 970FX user manual says about lmw/stmw: "The architecture allows these instructions to be interrupted by certain types of asynchronous interrupts (external interrupts, decrementer interrupts, machine check interrupts, and system reset interrupts). In these cases, for the load multiple instructions, all of the registers that were to be updated will have an undefined value, and the instruction must be completely restarted to achieve the full effect (that is, no partial restart capability is supported). For the store multiple instructions, some of the storage locations referenced by the instruction may have been updated. However, to guarantee full completion of the store multiple instruction, they must also be completely restarted." But given the fact that these are microcoded instructions, apparently slower and obviously non-safe, I'd head towards replacing them with multiple stw/lwz. Diff below also fixes the issue for me. If this is a sensible approach I'll try to get rid of all lmw/stmw. I'll obviously keep the socppc version in sync. Comments? Ok? Index: macppc/locore.S === RCS file: /cvs/src/sys/arch/macppc/macppc/locore.S,v retrieving revision 1.50 diff -u -p -r1.50 locore.S --- macppc/locore.S 29 Jul 2015 18:52:44 - 1.50 +++ macppc/locore.S 9 Oct 2015 16:41:16 - @@ -692,7 +692,10 @@ nop32_7s: mtmsrd %r1 nop32_7e: GET_CPUINFO(%r1) - stmw%r28,CI_DDBSAVE(%r1)/* free r28-r31 */ + stw %r28,(CI_DDBSAVE+0)(%r1) /* free r28 */ + stw %r29,(CI_DDBSAVE+4)(%r1) /* free r29 */ + stw %r30,(CI_DDBSAVE+8)(%r1) /* free r30 */ + stw %r31,(CI_DDBSAVE+12)(%r1) /* free r31 */ mflr%r28/* save LR */ mfcr%r29/* save CR */ GET_CPUINFO(%r30) @@ -1263,7 +1266,10 @@ _C_LABEL(ddb_trap): mtmsr %r3 /* disable interrupts */ isync GET_CPUINFO(%r3) - stmw%r28,CI_DDBSAVE(%r3) + stw %r28,(CI_DDBSAVE+0)(%r3) /* save r28 */ + stw %r29,(CI_DDBSAVE+4)(%r3) /* save r29 */ + stw %r30,(CI_DDBSAVE+8)(%r3) /* save r30 */ + stw %r31,(CI_DDBSAVE+12)(%r3) /* save r31 */ /* * If we are already running in interrupt context, the CPU
Re: Char cleanup in lex(1)
Michael McConville wrote:
> Theo de Raadt wrote:
> > > When scanning for is*() function uses with signed chars, I found that
> > > lex(1) uses an unecessary #define for unsigned char. The below diff
> > > removes it and fixes an undefined is*() usage or two as well.
> >
> > Our tree uses lex pretty much as-is from upstream, so this refactoring
> > isn't the way to go.
> >
> > If you find an actual bug however.. fix them, using the established
> > practice.
>
> Here are just the bug fixes:
Forgot to use Char. New diff:
Index: misc.c
===
RCS file: /cvs/src/usr.bin/lex/misc.c,v
retrieving revision 1.13
diff -u -p -r1.13 misc.c
--- misc.c 27 Oct 2013 18:31:24 - 1.13
+++ misc.c 13 Oct 2015 14:58:40 -
@@ -109,7 +109,7 @@ char *str;
{
while ( *str )
{
- if ( ! isascii( (Char) *str ) || ! islower( *str ) )
+ if ( ! isascii( (Char) *str ) || ! islower( (Char) *str ) )
return 0;
++str;
}
@@ -125,7 +125,7 @@ char *str;
{
while ( *str )
{
- if ( ! isascii( (Char) *str ) || ! isupper( *str ) )
+ if ( ! isascii( (Char) *str ) || ! isupper( (Char) *str ) )
return 0;
++str;
}
@@ -590,7 +590,7 @@ Char array[];
int sptr = 2;
while ( isascii( array[sptr] ) &&
- isxdigit( (char) array[sptr] ) )
+ isxdigit( array[sptr] ) )
/* Don't increment inside loop control
* because if isdigit() is a macro it might
* expand into multiple increments ...
Re: Char cleanup in lex(1)
Theo de Raadt wrote:
> > When scanning for is*() function uses with signed chars, I found that
> > lex(1) uses an unecessary #define for unsigned char. The below diff
> > removes it and fixes an undefined is*() usage or two as well.
>
> Our tree uses lex pretty much as-is from upstream, so this refactoring
> isn't the way to go.
>
> If you find an actual bug however.. fix them, using the established
> practice.
Here are just the bug fixes:
Index: misc.c
===
RCS file: /cvs/src/usr.bin/lex/misc.c,v
retrieving revision 1.13
diff -u -p -r1.13 misc.c
--- misc.c 27 Oct 2013 18:31:24 - 1.13
+++ misc.c 13 Oct 2015 14:30:30 -
@@ -109,7 +109,7 @@ char *str;
{
while ( *str )
{
- if ( ! isascii( (Char) *str ) || ! islower( *str ) )
+ if ( ! isascii( (Char) *str ) || ! islower( (unsigned char)
*str ) )
return 0;
++str;
}
@@ -125,7 +125,7 @@ char *str;
{
while ( *str )
{
- if ( ! isascii( (Char) *str ) || ! isupper( *str ) )
+ if ( ! isascii( (Char) *str ) || ! isupper( (unsigned char)
*str ) )
return 0;
++str;
}
@@ -590,7 +590,7 @@ Char array[];
int sptr = 2;
while ( isascii( array[sptr] ) &&
- isxdigit( (char) array[sptr] ) )
+ isxdigit( array[sptr] ) )
/* Don't increment inside loop control
* because if isdigit() is a macro it might
* expand into multiple increments ...
Re: ifdef DIAGNOSTIC in azalia.c
Alexey Suslikov gmail.com> writes:
>
> Alexey Suslikov gmail.com> writes:
>
> > If there is a need to debug something in azalia.c, defining DIAGNOSTIC
> > is overkill so replace two instances of DIAGNOSTIC with AZALIA_DEBUG
> > (DPRINTF->printf suggested by ratchov ).
> >
> > Also, entirely remove 3rd instance of DIAGNOSTIC. Normally it is not
> > compiled and, ratchov thinks, related to deleted code.
> >
> > Okays? Comments?
>
> Is there any interest in this? Alexandre, did I correctly understood an
> output of our discussion?
ping
re-sending in case diff got missed.
--- azalia.c.orig Wed Sep 23 16:10:19 2015
+++ azalia.cWed Sep 23 16:11:47 2015
@@ -1170,9 +1170,9 @@
uint32_t verb;
uint16_t corbwp;
-#ifdef DIAGNOSTIC
+#ifdef AZALIA_DEBUG
if ((AZ_READ_1(az, CORBCTL) & HDA_CORBCTL_CORBRUN) == 0) {
- DPRINTF(("%s: CORB is not running.\n", XNAME(az)));
+ printf(("%s: CORB is not running.\n", XNAME(az)));
return(-1);
}
#endif
@@ -1196,9 +1196,9 @@
int i;
uint16_t wp;
-#ifdef DIAGNOSTIC
+#ifdef AZALIA_DEBUG
if ((AZ_READ_1(az, RIRBCTL) & HDA_RIRBCTL_RIRBDMAEN) == 0) {
- DPRINTF(("%s: RIRB is not running.\n", XNAME(az)));
+ printf(("%s: RIRB is not running.\n", XNAME(az)));
return(-1);
}
#endif
@@ -4054,12 +4054,6 @@
/* number of blocks must be <= HDA_BDL_MAX */
az = v;
size = az->pstream.buffer.size;
-#ifdef DIAGNOSTIC
- if (size <= 0) {
- printf("%s: size is 0", __func__);
- return 256;
- }
-#endif
if (size > HDA_BDL_MAX * blk) {
blk = size / HDA_BDL_MAX;
if (blk & 0x7f)
Re: preparing pfi_kif to MP world
Hello,
> > > Furthermore existing functions pfi_kif_ref()/pfi_kif_unref() are thrown
> > > away
> > > in favor of pfi_kif_take()/pfi_kif_rele(), which follow naming convention
> > > set by refcnt_init(9). Patch also removes kif reference types (enum
> > > pfi_kif_refs).
> > >
> [snip]
> > > @@ -810,3 +762,28 @@ pfi_unmask(void *addr)
> > > return (b);
> > > }
> > >
> > > +struct pfi_kif *
> > > +pfi_kif_take(struct pfi_kif *kif)
> > > +{
> > > + if (kif != pfi_all) {
> > > + PF_REF_TAKE(kif->pfik_refcnt);
> > > + }
>
> No need for curly braces here --^
>
yes, that's true, will fix that...
>
> Even the typedef? (-:
>
sorry I forgot about OpenBSD hates typedefs ;-)
I'll keep in mind to kill all typdefs in my patches next time.
> but I'd prefer the former, e.g. _k instead of _kif_. Double
O.K. I'll stick to _k style.
>
> PFI_KIF_TAKE and PFI_KIF_RELE macros add another level of
> complexity for almost zero gain. PFI_KIF_TAKE is not even
> used. I'd say scrap them.
it's true in case of pfi_kif objects we can scrap them.
on the other hand the macros illustrate the way I'd like to reference counting
for other objects. I'll stick to the pfi_kif just as an illustration.
if I use lowercase function such as:
r->kif = pfi_kif_take(kif);
then I'm sure the kif instance exists, if it does not exist the thing
will blow up with NULL pointer dereference and I know I'm wrong. On the
other hand if I do:
r->kif = PFI_KIF_TAKE(kif);
then it means kif is optional here, but if it exists, then I must have
a reference for it here.
The way we have it in PF on Solaris these days looks as follows:
_take(x)
{
if (x != NULL)
PF_INC_REF(x->refcnt);
return (x);
}
This way we assume 'x' is always optional. And this is sometimes quite
wrong, as we might set mine, we trip over later.
The uppercase macros allows us to easily annotate cases, where the
object is optional and vice versa: the lower case functions annotate
cases, where we assume the object must exist. This could save us few
days
of debugging.
So I'd like to make sure if you hate that approach in general or in the
scope of this particular patch. I admit the pfi_kif is not the right
guy for the uppercase version as the pfi_kif instance always must exist.
I'll remove those macros from proposed patch, but I'd like to know
how do you feel about them for other objects such as tables, anchors,
states,
> I would also say that the PF_REF_* should stay under _KERNEL
> as they simply cannot be used in the userland and moved some-
> where before "extern struct pf_status pf_status" (after the
> prototypes chunk) or after the "extern struct pf_pool_limit"
> right before the "#endif /* _KERNEL */".
>
yes that's very good point. (How I could miss it?)
thanks and
regards
sasha
Re: FreeType-2.6.1 !!header files layout changed again!!
David Coppa: > New freetype version, new header file layout :( :( > [...] > Obviously, I'm expecting some fallouts from a bulk build with this... The only initial fallout from this is devel/xulrunner/24. However, without xulrunner ~750 further packages aren't built, so we need a fix for xulrunner before we can see what the overall breakage is. -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: preparing pfi_kif to MP world
On Tue, Oct 13, 2015 at 20:36 +1000, David Gwynne wrote:
>
> > On 12 Oct 2015, at 12:00 AM, Alexandr Nedvedicky
> > wrote:
> >
> > Hello,
> >
> > patch below introduces struct refcnt to pfi_kif structure. Patch also
> > changes
> > pfi_kif_get() function to always return a reference to pfi_kif instance.
> >
> > Furthermore existing functions pfi_kif_ref()/pfi_kif_unref() are thrown away
> > in favor of pfi_kif_take()/pfi_kif_rele(), which follow naming convention
> > set by refcnt_init(9). Patch also removes kif reference types (enum
> > pfi_kif_refs).
> >
[snip]
> >
> > thanks and
> > regards
> > sasha
> >
> > -8<8<8<---8<
> > Index: if_pfsync.c
> > ===
> > RCS file: /cvs/src/sys/net/if_pfsync.c,v
> > retrieving revision 1.220
> > diff -u -p -r1.220 if_pfsync.c
> > --- if_pfsync.c 11 Sep 2015 08:17:06 - 1.220
> > +++ if_pfsync.c 11 Oct 2015 13:46:11 -
> > @@ -631,6 +631,8 @@ pfsync_state_import(struct pfsync_state
> > }
> > CLR(st->state_flags, PFSTATE_ACK);
> >
> > + pfi_kif_rele(kif);
> > +
> > return (0);
> >
> > cleanup:
> > @@ -650,6 +652,9 @@ pfsync_state_import(struct pfsync_state
> > pool_put(&pf_state_scrub_pl, st->src.scrub);
> > pool_put(&pf_state_pl, st);
> > }
> > +
> > + pfi_kif_rele(kif);
> > +
> > return (error);
> > }
> >
> > @@ -759,6 +764,7 @@ pfsync_in_clr(caddr_t buf, int len, int
> > struct pf_state *st, *nexts;
> > struct pf_state_key *sk, *nextsk;
> > struct pf_state_item *si;
> > + struct pfi_kif *kif;
> > u_int32_t creatorid;
> >
> > for (i = 0; i < count; i++) {
> > @@ -775,7 +781,11 @@ pfsync_in_clr(caddr_t buf, int len, int
> > }
> > }
> > } else {
> > - if (pfi_kif_get(clr->ifname) == NULL)
> > + /*
> > +* Do we realize here the pfi_kif_get() actually
> > +* creates kif for name if it does not exist?
> > +*/
> > + if ((kif = pfi_kif_get(clr->ifname)) == NULL)
> > continue;
> >
> > /* XXX correct? */
> > @@ -791,6 +801,8 @@ pfsync_in_clr(caddr_t buf, int len, int
> > }
> > }
> > }
> > +
> > + pfi_kif_rele(kif);
> > }
> > }
> >
> > Index: pf.c
> > ===
> > RCS file: /cvs/src/sys/net/pf.c,v
> > retrieving revision 1.946
> > diff -u -p -r1.946 pf.c
> > --- pf.c8 Oct 2015 11:36:51 - 1.946
> > +++ pf.c11 Oct 2015 13:46:14 -
> > @@ -944,7 +944,7 @@ pf_state_insert(struct pfi_kif *kif, str
> > TAILQ_INSERT_TAIL(&state_list, s, entry_list);
> > pf_status.fcounters[FCNT_STATE_INSERT]++;
> > pf_status.states++;
> > - pfi_kif_ref(kif, PFI_KIF_REF_STATE);
> > + pfi_kif_take(kif);
> > #if NPFSYNC > 0
> > pfsync_insert_state(s);
> > #endif /* NPFSYNC > 0 */
> > @@ -1315,7 +1315,7 @@ pf_free_state(struct pf_state *cur)
> > pool_put(&pf_rule_item_pl, ri);
> > }
> > pf_normalize_tcp_cleanup(cur);
> > - pfi_kif_unref(cur->kif, PFI_KIF_REF_STATE);
> > + pfi_kif_rele(cur->kif);
> > TAILQ_REMOVE(&state_list, cur, entry_list);
> > if (cur->tag)
> > pf_tag_unref(cur->tag);
> > Index: pf_if.c
> > ===
> > RCS file: /cvs/src/sys/net/pf_if.c,v
> > retrieving revision 1.80
> > diff -u -p -r1.80 pf_if.c
> > --- pf_if.c 4 Sep 2015 21:40:25 - 1.80
> > +++ pf_if.c 11 Oct 2015 13:46:14 -
> > @@ -107,7 +107,7 @@ pfi_kif_get(const char *kif_name)
> > bzero(&s, sizeof(s));
> > strlcpy(s.pfik_name, kif_name, sizeof(s.pfik_name));
> > if ((kif = RB_FIND(pfi_ifhead, &pfi_ifs, (struct pfi_kif *)&s)) != NULL)
> > - return (kif);
> > + return (pfi_kif_take(kif));
> >
> > /* create new one */
> > if ((kif = malloc(sizeof(*kif), PFI_MTYPE, M_NOWAIT|M_ZERO)) == NULL)
> > @@ -116,6 +116,7 @@ pfi_kif_get(const char *kif_name)
> > strlcpy(kif->pfik_name, kif_name, sizeof(kif->pfik_name));
> > kif->pfik_tzero = time_second;
> > TAILQ_INIT(&kif->pfik_dynaddrs);
> > + PF_REF_INIT(kif->pfik_refcnt);
> >
> > if (!strcmp(kif->pfik_name, "any")) {
> > /* both so it works in the ioctl and the regular case */
> > @@ -124,72 +125,21 @@ pfi_kif_get(const char *kif_name)
> > }
> >
> > RB_INSERT(pfi_ifhead, &pfi_ifs, kif);
> > + /*
> > +* there is no pfi_kif_remove(), hence caller shares reference with
> > +* pfi_ifs look up table.
> > +*/
> > return (kif);
> > }
> >
> > -void
> > -pfi_kif_ref(struct pfi_kif *kif, enum pfi_kif_refs wha
Re: preparing pfi_kif to MP world
On Tue, Oct 13, 2015 at 20:36 +1000, David Gwynne wrote:
>
> > On 12 Oct 2015, at 12:00 AM, Alexandr Nedvedicky
> > wrote:
> >
> > Hello,
> >
> > patch below introduces struct refcnt to pfi_kif structure. Patch also
> > changes
> > pfi_kif_get() function to always return a reference to pfi_kif instance.
> >
> > Furthermore existing functions pfi_kif_ref()/pfi_kif_unref() are thrown away
> > in favor of pfi_kif_take()/pfi_kif_rele(), which follow naming convention
> > set by refcnt_init(9). Patch also removes kif reference types (enum
> > pfi_kif_refs).
> >
> > Patch also updates if_pfsync.c file. I'm bit puzzled with test as follows
> > in pfsync_in_clr():
> >
> > 770 for (i = 0; i < count; i++) {
> > 771 clr = (struct pfsync_clr *)buf + len * i;
> > 772 creatorid = clr->creatorid;
> > 773
> > 774 if (clr->ifname[0] == '\0') {
> > ...
> > 783 } else {
> > 784 if (pfi_kif_get(clr->ifname) == NULL)
> > 789 continue;
> > 790
> >
> > I could not make any sense of line 784. Are we just making sure particular
> > kif
> > object exists for clr->ifname? Note pfi_kif_get() creates kif for us if it
> > does not exist. If we want to query for kif existence only patch offers
> > pfi_kif_check() function. I need some advice here if we should keep, what's
> > currently in patch or switch to pfi_kif_check().
>
> my name is probably against that code.
>
> i didn't realise that pfi_kif_get creates a kif on demand. however,
> it also uses malloc(, , M_NOWAIT) to try and allocate it, which can
> fail and cause pfi_kif_get to return NULL.
>
Revision 1.102 used to get the kif and check the state against it:
if (si->s->creatorid == creatorid &&
si->s->kif == kif) {
Then in the rev 1.103 you have removed the check. The question is
why did you do that? (-:
Re: patch for two nits around pf_insert_src_node() et. al.
On 12/10/15(Mon) 22:29, Alexandr Nedvedicky wrote:
> Hello,
>
> Richard Procter came back to me in private email with one more nit to fix:
>
> we can get rid of
>
> if (sn->rule.ptr != NULL)
> test condition in pfioctl() function as well.
>
> The relevant snippet looks as follows:
>
> 2188 p = psn->psn_src_nodes;
> 2189 RB_FOREACH(n, pf_src_tree, &tree_src_tracking) {
>
> 2198 pstore->kif = NULL;
> 2199 if (n->rule.ptr != NULL)
> 2200 pstore->rule.nr = n->rule.ptr->nr;
>
> need one more OK for Richard's suggestion. Updated patch is below.
> Complete email from Richard follows the patch.
ok mpi@
>
> thanks and
> regards
> sasha
>
> 8<---8<---8<--8<
>
> Index: pf.c
> ===
> RCS file: /cvs/src/sys/net/pf.c,v
> retrieving revision 1.946
> diff -u -p -r1.946 pf.c
> --- pf.c 8 Oct 2015 11:36:51 - 1.946
> +++ pf.c 12 Oct 2015 20:20:17 -
> @@ -501,7 +501,7 @@ pf_src_connlimit(struct pf_state **state
> int
> pf_insert_src_node(struct pf_src_node **sn, struct pf_rule *rule,
> enum pf_sn_types type, sa_family_t af, struct pf_addr *src,
> -struct pf_addr *raddr, int global)
> +struct pf_addr *raddr)
> {
> struct pf_src_node k;
>
> @@ -509,10 +509,7 @@ pf_insert_src_node(struct pf_src_node **
> k.af = af;
> k.type = type;
> PF_ACPY(&k.addr, src, af);
> - if (global)
> - k.rule.ptr = NULL;
> - else
> - k.rule.ptr = rule;
> + k.rule.ptr = rule;
> pf_status.scounters[SCNT_SRC_NODE_SEARCH]++;
> *sn = RB_FIND(pf_src_tree, &tree_src_tracking, &k);
> }
> @@ -531,10 +528,7 @@ pf_insert_src_node(struct pf_src_node **
>
> (*sn)->type = type;
> (*sn)->af = af;
> - if (global)
> - (*sn)->rule.ptr = NULL;
> - else
> - (*sn)->rule.ptr = rule;
> + (*sn)->rule.ptr = rule;
> PF_ACPY(&(*sn)->addr, src, af);
> if (raddr)
> PF_ACPY(&(*sn)->raddr, raddr, af);
> @@ -550,8 +544,7 @@ pf_insert_src_node(struct pf_src_node **
> return (-1);
> }
> (*sn)->creation = time_uptime;
> - if ((*sn)->rule.ptr != NULL)
> - (*sn)->rule.ptr->src_nodes++;
> + (*sn)->rule.ptr->src_nodes++;
> pf_status.scounters[SCNT_SRC_NODE_INSERT]++;
> pf_status.src_nodes++;
> } else {
> @@ -570,16 +563,14 @@ pf_remove_src_node(struct pf_src_node *s
> if (sn->states > 0 || sn->expire > time_uptime)
> return;
>
> - if (sn->rule.ptr != NULL) {
> - sn->rule.ptr->src_nodes--;
> - if (sn->rule.ptr->states_cur == 0 &&
> - sn->rule.ptr->src_nodes == 0)
> - pf_rm_rule(NULL, sn->rule.ptr);
> - RB_REMOVE(pf_src_tree, &tree_src_tracking, sn);
> - pf_status.scounters[SCNT_SRC_NODE_REMOVALS]++;
> - pf_status.src_nodes--;
> - pool_put(&pf_src_tree_pl, sn);
> - }
> + sn->rule.ptr->src_nodes--;
> + if (sn->rule.ptr->states_cur == 0 &&
> + sn->rule.ptr->src_nodes == 0)
> + pf_rm_rule(NULL, sn->rule.ptr);
> + RB_REMOVE(pf_src_tree, &tree_src_tracking, sn);
> + pf_status.scounters[SCNT_SRC_NODE_REMOVALS]++;
> + pf_status.src_nodes--;
> + pool_put(&pf_src_tree_pl, sn);
> }
>
> struct pf_src_node *
> @@ -3381,7 +3372,7 @@ pf_test_rule(struct pf_pdesc *pd, struct
>
> if (r->rule_flag & PFRULE_SRCTRACK &&
> pf_insert_src_node(&sns[PF_SN_NONE], r, PF_SN_NONE, pd->af,
> - pd->src, NULL, 0) != 0) {
> + pd->src, NULL) != 0) {
> REASON_SET(&reason, PFRES_SRCLIMIT);
> goto cleanup;
> }
> Index: pf_ioctl.c
> ===
> RCS file: /cvs/src/sys/net/pf_ioctl.c,v
> retrieving revision 1.290
> diff -u -p -r1.290 pf_ioctl.c
> --- pf_ioctl.c4 Sep 2015 21:40:25 - 1.290
> +++ pf_ioctl.c12 Oct 2015 20:20:20 -
> @@ -2175,8 +2175,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
> bzero(&pstore->entry, sizeof(pstore->entry));
> pstore->rule.ptr = NULL;
> pstore->kif = NULL;
> - if (n->rule.ptr != NULL)
> - pstore->rule.nr = n->rule.ptr->nr;
> + pstore->rule.nr = n->rule.ptr->nr;
> pstore->creation = secs - pstore->creation;
>
Re: preparing pfi_kif to MP world
> On 12 Oct 2015, at 12:00 AM, Alexandr Nedvedicky
> wrote:
>
> Hello,
>
> patch below introduces struct refcnt to pfi_kif structure. Patch also changes
> pfi_kif_get() function to always return a reference to pfi_kif instance.
>
> Furthermore existing functions pfi_kif_ref()/pfi_kif_unref() are thrown away
> in favor of pfi_kif_take()/pfi_kif_rele(), which follow naming convention
> set by refcnt_init(9). Patch also removes kif reference types (enum
> pfi_kif_refs).
>
> Patch also updates if_pfsync.c file. I'm bit puzzled with test as follows
> in pfsync_in_clr():
>
> 770 for (i = 0; i < count; i++) {
> 771 clr = (struct pfsync_clr *)buf + len * i;
> 772 creatorid = clr->creatorid;
> 773
> 774 if (clr->ifname[0] == '\0') {
> ...
> 783 } else {
> 784 if (pfi_kif_get(clr->ifname) == NULL)
> 789 continue;
> 790
>
> I could not make any sense of line 784. Are we just making sure particular kif
> object exists for clr->ifname? Note pfi_kif_get() creates kif for us if it
> does not exist. If we want to query for kif existence only patch offers
> pfi_kif_check() function. I need some advice here if we should keep, what's
> currently in patch or switch to pfi_kif_check().
my name is probably against that code.
i didn't realise that pfi_kif_get creates a kif on demand. however, it also
uses malloc(, , M_NOWAIT) to try and allocate it, which can fail and cause
pfi_kif_get to return NULL.
>
> thanks and
> regards
> sasha
>
> -8<8<8<---8<
> Index: if_pfsync.c
> ===
> RCS file: /cvs/src/sys/net/if_pfsync.c,v
> retrieving revision 1.220
> diff -u -p -r1.220 if_pfsync.c
> --- if_pfsync.c 11 Sep 2015 08:17:06 - 1.220
> +++ if_pfsync.c 11 Oct 2015 13:46:11 -
> @@ -631,6 +631,8 @@ pfsync_state_import(struct pfsync_state
> }
> CLR(st->state_flags, PFSTATE_ACK);
>
> + pfi_kif_rele(kif);
> +
> return (0);
>
> cleanup:
> @@ -650,6 +652,9 @@ pfsync_state_import(struct pfsync_state
> pool_put(&pf_state_scrub_pl, st->src.scrub);
> pool_put(&pf_state_pl, st);
> }
> +
> + pfi_kif_rele(kif);
> +
> return (error);
> }
>
> @@ -759,6 +764,7 @@ pfsync_in_clr(caddr_t buf, int len, int
> struct pf_state *st, *nexts;
> struct pf_state_key *sk, *nextsk;
> struct pf_state_item *si;
> + struct pfi_kif *kif;
> u_int32_t creatorid;
>
> for (i = 0; i < count; i++) {
> @@ -775,7 +781,11 @@ pfsync_in_clr(caddr_t buf, int len, int
> }
> }
> } else {
> - if (pfi_kif_get(clr->ifname) == NULL)
> + /*
> + * Do we realize here the pfi_kif_get() actually
> + * creates kif for name if it does not exist?
> + */
> + if ((kif = pfi_kif_get(clr->ifname)) == NULL)
> continue;
>
> /* XXX correct? */
> @@ -791,6 +801,8 @@ pfsync_in_clr(caddr_t buf, int len, int
> }
> }
> }
> +
> + pfi_kif_rele(kif);
> }
> }
>
> Index: pf.c
> ===
> RCS file: /cvs/src/sys/net/pf.c,v
> retrieving revision 1.946
> diff -u -p -r1.946 pf.c
> --- pf.c 8 Oct 2015 11:36:51 - 1.946
> +++ pf.c 11 Oct 2015 13:46:14 -
> @@ -944,7 +944,7 @@ pf_state_insert(struct pfi_kif *kif, str
> TAILQ_INSERT_TAIL(&state_list, s, entry_list);
> pf_status.fcounters[FCNT_STATE_INSERT]++;
> pf_status.states++;
> - pfi_kif_ref(kif, PFI_KIF_REF_STATE);
> + pfi_kif_take(kif);
> #if NPFSYNC > 0
> pfsync_insert_state(s);
> #endif/* NPFSYNC > 0 */
> @@ -1315,7 +1315,7 @@ pf_free_state(struct pf_state *cur)
> pool_put(&pf_rule_item_pl, ri);
> }
> pf_normalize_tcp_cleanup(cur);
> - pfi_kif_unref(cur->kif, PFI_KIF_REF_STATE);
> + pfi_kif_rele(cur->kif);
> TAILQ_REMOVE(&state_list, cur, entry_list);
> if (cur->tag)
> pf_tag_unref(cur->tag);
> Index: pf_if.c
> ===
> RCS file: /cvs/src/sys/net/pf_if.c,v
> retrieving revision 1.80
> diff -u -p -r1.80 pf_if.c
> --- pf_if.c 4 Sep 2015 21:40:25 - 1.80
> +++ pf_if.c 11 Oct 2015 13:46:14 -
> @@ -107,7 +107,7 @@ pfi_kif_get(const char *kif_name)
> bzero(&s, sizeof(s));
> strlcpy(s.pfik_name, kif_name, sizeof(s.pfik_name));
> if ((kif = RB_FIND(pfi_ifhead, &pfi_ifs, (struct pfi_kif *)&s)) != NULL)
> - return (ki
Re: patch for two nits around pf_insert_src_node() et. al.
On Mon, Oct 12, 2015 at 22:29 +0200, Alexandr Nedvedicky wrote:
> Hello,
>
> Richard Procter came back to me in private email with one more nit to fix:
>
> we can get rid of
>
> if (sn->rule.ptr != NULL)
> test condition in pfioctl() function as well.
>
> The relevant snippet looks as follows:
>
> 2188 p = psn->psn_src_nodes;
> 2189 RB_FOREACH(n, pf_src_tree, &tree_src_tracking) {
>
> 2198 pstore->kif = NULL;
> 2199 if (n->rule.ptr != NULL)
> 2200 pstore->rule.nr = n->rule.ptr->nr;
>
> need one more OK for Richard's suggestion. Updated patch is below.
> Complete email from Richard follows the patch.
>
Sure, good catch, Richard!
> thanks and
> regards
> sasha
>
> 8<---8<---8<--8<
>
> Index: pf.c
> ===
> RCS file: /cvs/src/sys/net/pf.c,v
> retrieving revision 1.946
> diff -u -p -r1.946 pf.c
> --- pf.c 8 Oct 2015 11:36:51 - 1.946
> +++ pf.c 12 Oct 2015 20:20:17 -
> @@ -501,7 +501,7 @@ pf_src_connlimit(struct pf_state **state
> int
> pf_insert_src_node(struct pf_src_node **sn, struct pf_rule *rule,
> enum pf_sn_types type, sa_family_t af, struct pf_addr *src,
> -struct pf_addr *raddr, int global)
> +struct pf_addr *raddr)
> {
> struct pf_src_node k;
>
> @@ -509,10 +509,7 @@ pf_insert_src_node(struct pf_src_node **
> k.af = af;
> k.type = type;
> PF_ACPY(&k.addr, src, af);
> - if (global)
> - k.rule.ptr = NULL;
> - else
> - k.rule.ptr = rule;
> + k.rule.ptr = rule;
> pf_status.scounters[SCNT_SRC_NODE_SEARCH]++;
> *sn = RB_FIND(pf_src_tree, &tree_src_tracking, &k);
> }
> @@ -531,10 +528,7 @@ pf_insert_src_node(struct pf_src_node **
>
> (*sn)->type = type;
> (*sn)->af = af;
> - if (global)
> - (*sn)->rule.ptr = NULL;
> - else
> - (*sn)->rule.ptr = rule;
> + (*sn)->rule.ptr = rule;
> PF_ACPY(&(*sn)->addr, src, af);
> if (raddr)
> PF_ACPY(&(*sn)->raddr, raddr, af);
> @@ -550,8 +544,7 @@ pf_insert_src_node(struct pf_src_node **
> return (-1);
> }
> (*sn)->creation = time_uptime;
> - if ((*sn)->rule.ptr != NULL)
> - (*sn)->rule.ptr->src_nodes++;
> + (*sn)->rule.ptr->src_nodes++;
> pf_status.scounters[SCNT_SRC_NODE_INSERT]++;
> pf_status.src_nodes++;
> } else {
> @@ -570,16 +563,14 @@ pf_remove_src_node(struct pf_src_node *s
> if (sn->states > 0 || sn->expire > time_uptime)
> return;
>
> - if (sn->rule.ptr != NULL) {
> - sn->rule.ptr->src_nodes--;
> - if (sn->rule.ptr->states_cur == 0 &&
> - sn->rule.ptr->src_nodes == 0)
> - pf_rm_rule(NULL, sn->rule.ptr);
> - RB_REMOVE(pf_src_tree, &tree_src_tracking, sn);
> - pf_status.scounters[SCNT_SRC_NODE_REMOVALS]++;
> - pf_status.src_nodes--;
> - pool_put(&pf_src_tree_pl, sn);
> - }
> + sn->rule.ptr->src_nodes--;
> + if (sn->rule.ptr->states_cur == 0 &&
> + sn->rule.ptr->src_nodes == 0)
> + pf_rm_rule(NULL, sn->rule.ptr);
> + RB_REMOVE(pf_src_tree, &tree_src_tracking, sn);
> + pf_status.scounters[SCNT_SRC_NODE_REMOVALS]++;
> + pf_status.src_nodes--;
> + pool_put(&pf_src_tree_pl, sn);
> }
>
> struct pf_src_node *
> @@ -3381,7 +3372,7 @@ pf_test_rule(struct pf_pdesc *pd, struct
>
> if (r->rule_flag & PFRULE_SRCTRACK &&
> pf_insert_src_node(&sns[PF_SN_NONE], r, PF_SN_NONE, pd->af,
> - pd->src, NULL, 0) != 0) {
> + pd->src, NULL) != 0) {
> REASON_SET(&reason, PFRES_SRCLIMIT);
> goto cleanup;
> }
> Index: pf_ioctl.c
> ===
> RCS file: /cvs/src/sys/net/pf_ioctl.c,v
> retrieving revision 1.290
> diff -u -p -r1.290 pf_ioctl.c
> --- pf_ioctl.c4 Sep 2015 21:40:25 - 1.290
> +++ pf_ioctl.c12 Oct 2015 20:20:20 -
> @@ -2175,8 +2175,7 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
> bzero(&pstore->entry, sizeof(pstore->entry));
> pstore->rule.ptr = NULL;
> pstore->kif = NULL;
> - if (n->rule.ptr != NULL)
> - pstore->rule.nr = n->rule.ptr->nr;
> + pstore->rule.nr = n->rule.ptr->nr;
> pstore->creation = s
Re: vattr_null() doesn't initialize va_filerev
On Thu, 8 Oct 2015, Martin Natano wrote:
> Filesystem implementations depend on vattr_null() to initialize the
> fields in struct vattr, which is true for all the fields except
> va_filerev. It therefore is not set to VNOVAL as expected by the file
> system, but contains whatever was there on the stack. This causes
> VOP_GETATTR() on cd9660 and msdosfs vnodes to yield garbage for
> va_filerev.
Thanks for the report. Fixing that made me think more closely about
vattr_null() and ended up totally rewriting it.
To back up...
Beware code that chains variable assignments to a potentially negative
value, such as:
foo = bar = baz = -1;
If foo, bar, and baz have different sizes, then the result may depend on
the order! To demonstrate:
#include
int main(void)
{
unsigned long long llu;
unsigned u;
u = llu = -1; printf("u\t%#x\nllu\t%#llx\n", u, llu);
llu = u = -1; printf("u\t%#x\nllu\t%#llx\n", u, llu);
return 0;
}
That program outputs:
u 0x
llu 0x
u 0x
llu 0x
showing that the order of assignment matters. What's happening is that
the value of an assignment is of the type of the lvalue being assigned to,
so
llu = u = -1;
is the same as
llu = (unsigned)(u = -1);
Since (unsigned)-1 == 2^31-1 is already in the range of llu, there's no
expansion to 2^63-1. (I haven't verified, but assignment to an unsigned
type to a smaller, signed type may count as signed overflow and
technically be undefined behavior. If so and your compiler optimized on
that basis, beware!)
vattr_null() in the kernel was balanced on a knife's edge, switching among
*six* types in the third statement here:
/* XXX These next two used to be one line, but for a GCC bug. */
vap->va_size = VNOVAL;
vap->va_bytes = VNOVAL;
vap->va_mode = vap->va_nlink = vap->va_uid = vap->va_gid =
vap->va_fsid = vap->va_fileid =
vap->va_blocksize = vap->va_rdev =
vap->va_atime.tv_sec = vap->va_atime.tv_nsec =
vap->va_mtime.tv_sec = vap->va_mtime.tv_nsec =
vap->va_ctime.tv_sec = vap->va_ctime.tv_nsec =
vap->va_flags = vap->va_gen = VNOVAL;
That has all of int, u_int, long, u_long, quad_t, and u_quad_t, and only
works because it has a signed type on the right when assigning to a larger
type. It breaks if, for example, you swap va_nlink and va_fsid, or if you
swap the tv_sec and tv_nsec order, or if you put va_size or va_bytes on
the front of the third statement:
vap->va_size = vap->va_bytes = vap->va_mode = ...
I suspect the "GCC bug" comment was because the author (pre-rev 1.1)
didn't understand the C promotion rules.
So I've changed it to stop being fancy and just do the assignments one by
one. This also results in the assignments being done in memory order:
vap->va_type = VNON;
/*
* Don't get fancy: u_quad_t = u_int = VNOVAL leaves the u_quad_t
* with 2^31-1 instead of 2^64-1. Just write'm out and let
* the compiler do its job.
*/
vap->va_mode = VNOVAL;
vap->va_nlink = VNOVAL;
vap->va_uid = VNOVAL;
vap->va_gid = VNOVAL;
vap->va_fsid = VNOVAL;
...etc
For this sort of init/reinit function, we may update that to use a
static const variable with the desired values and just assign from that,
but that'll just be gravy compared to making to code robust to changes
to typedefs.
Philip Guenther
a tale of software maintenance: OpenSSL and EVP_CHECK_DES_KEY
In case you need an OpenSSL anecdote to scare your co-workers with... Many of you may remember from your crypto class in college that DES has 16 'weak' keys that have group-like properties; check wikipedia for a longer explanation. These are not generally considered a problem: in any sane situation, keys for DES are generated with a CSPRNG (cryptographically secure random number generator). Since there are 2^56 possible keys, the odds of hitting one of these is 1 in 2^52. That's "both you and your computer were--independently--struck by lightening this year" territory. So, the *serious* recommendation by the cryptographic community is to ignore the possibility of getting a weak key: don't check for them. If you get one either a) your random number generator is bad, like *Debian* bad, and you're *totally screwed* already: checking for weak DES keys is putting new vinyl on the Titanic's deck's chairs, OR b) wow, you're unlucky! Sorry about the lightening; you should buy a lottery ticket! ...but don't worry, the attacker was just going to brute force your DES keys anyway! You're more likely to get the check wrong than to ever hit one of them. Huh, that's a funny way to phrase it... So OpenSSL has _optional_ code to reject attempts to use weak DES keys. It, sanely, is *not* enabled by default; if you want it you have to compile with -DEVP_CHECK_DES_KEY. Last Thursday it was reported to the openssl-dev mailing list by Ben Kaduk that there was a defect in this optional code: it had a syntax error and didn't even compile. It had a typo of "!!" instead of "||": if (DES_set_key_checked(&deskey[0], &data(ctx)->ks1) !! DES_set_key_checked(&deskey[1], &data(ctx)->ks2)) ... This syntax error was present in the _original_ commit: the code in the #ifdefs had _never_ been compiled. ... ... This code was commited in 2004. ... ... (stop screaming and catch your breath) ... The LibreSSL response? The #ifdefs and code in them have been deleted. The OpenSSL response? The code... that in 11 years had never been used... for a deprecated cipher... was *fixed* on Saturday, retaining the #ifdefs
