Re: [PATCH] src/usr.bin/ftp/fetch.c: free ressl_config
On Thu, Sep 11, 2014 at 11:05:02PM -0500, Kent R. Spillner wrote:
While reviewing tedu@'s libressl config cleanup diffs I noticed we're
not explicitly freeing ressl_config in ftp(1).
...
Index: fetch.c
===
...
if (ssl != NULL) {
ressl_close(ssl);
+ ressl_config_free(ressl_config);
Hmm this doesn't look right to me. ressl_config is not allocated the
same way as the other variables. The other variables such as ssl,
sslhost, etc are local to that function and allocated there.
ressl_config is a global and only allocated in main.c. It is allocated
once and configured using getopt/getsubopt().
If you free it, you would need to reallocate it each time or set it to
NULL so libressl will revert back and discard the user's changes.
I think either way is wrong. The SSL configuration should be retained
even if a single url_get() fails (which isn't fatal).
The above code is from url_get() which is called in a loop inside
auto_fetch(). I think the above change would use the getsubopt()
configuration for the first HTTPS URL argument and then try to use freed
memory for the others.
Re: Switch ppb(4) from workq to task
On Mon, Sep 8, 2014 at 12:23 PM, sven falempin sven.falem...@gmail.com wrote:
On Mon, Sep 8, 2014 at 12:17 PM, Miod Vallat m...@online.fr wrote:
My question is more about the driver itself, it did not change a lot
since 1994 , while netbsd apparently move to a complete different
driver :
http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/dev/ppbus/?only_with_tag=HEAD
Are those driving the same chipset family ?
any info on that ?
ppbus in NetBSD is a port of FreeBSD's ppbus which allows access to
non-printer devices connected to parallel ports, such as zip drives. It
has no relation with PCI-PCI bridges.
indeed, these one looks better :
http://cvsweb.netbsd.org/bsdweb.cgi/~checkout~/src/sys/dev/pci/ppb.c?rev=1.52content-type=text/plainonly_with_tag=HEAD
--
Dear tech readers,
I add logs into ppb driver to look for a relation between the driver
and the disconnection.
( I run openBSD patched branch 5.5, the main board is an APU from pcengines. )
Let me explain what happen as i have some updates:
On a pci express i plug a pcie to pci bridge, on this bridge i plug
two network card , so far so good.
one card run the vr driver the other one the rl driver.
After more than 24 hours and less 36 hours ( i am still refining this
) both network cards stop replying to icmp.
dmesg said once:
rl0: watchdog timeout
and
vr1: restarting (yes the network card got more than on Ethernet, i use
the second for testing)
so i suspected the bridge, added some log to see if it was related to
some kind of event.
Now i only see vr1: restarting but behavior is the same, on a plus
side i figure i am losing the Ethernet negotiation and each time i
enter
ifconfig vr1 the dmesg add vr1: restarting
I would like to get to the bottom of this, but i need guidance,
(for example is it possible to syslog from kernel, because those
printf has no time information)
meanwhile i will of course reproduce with the last snapshots
Best Regards,
Index: ppb.c
===
RCS file: /cvs/src/sys/dev/pci/ppb.c,v
retrieving revision 1.57
diff -u -p -r1.57 ppb.c
--- ppb.c 29 Jan 2014 18:30:39 - 1.57
+++ ppb.c 12 Sep 2014 13:49:57 -
@@ -328,6 +328,7 @@ ppbdetach(struct device *self, int flags
struct ppb_softc *sc = (struct ppb_softc *)self;
char *name;
int rv;
+printf([XXX] %s: ppbdetach %08X, sc-sc_dev.dv_xname, flags);
if (sc-sc_intrhand)
pci_intr_disestablish(sc-sc_pc, sc-sc_intrhand);
@@ -366,6 +367,7 @@ ppbactivate(struct device *self, int act
switch (act) {
case DVACT_SUSPEND:
+printf([XXX] %s: ppb activate suspend, sc-sc_dev.dv_xname);
rv = config_activate_children(self, act);
/* Save registers that may get lost. */
@@ -394,6 +396,7 @@ ppbactivate(struct device *self, int act
}
break;
case DVACT_RESUME:
+printf([XXX] %s: ppb activate resume %s,
sc-sc_dev.dv_xname, (pci_dopm) ? true : false);
if (pci_dopm) {
/* Restore power. */
pci_set_powerstate(pc, tag, sc-sc_pmcsr_state);
@@ -461,6 +464,7 @@ ppbactivate(struct device *self, int act
rv = config_activate_children(self, act);
break;
case DVACT_POWERDOWN:
+printf([XXX] %s: ppb activate powerdown %s,
sc-sc_dev.dv_xname, (pci_dopm) ? true : false);
rv = config_activate_children(self, act);
if (pci_dopm) {
@@ -474,6 +478,7 @@ ppbactivate(struct device *self, int act
}
break;
default:
+printf([XXX] %s: ppb activate default %d:%s,
sc-sc_dev.dv_xname, act, (pci_dopm) ? true : false);
rv = config_activate_children(self, act);
break;
}
@@ -645,8 +650,12 @@ ppb_intr(void *arg)
* sure we will not try to forcibly detach re(4) when it isn't
* ready to deal with that.
*/
- if (cold)
+ if (cold) {
+printf (%s: the cold case,sc-sc_dev.dv_xname);
return (0);
+} else {
+printf (%s: the Hot case, sc-sc_dev.dv_xname);
+}
reg = pci_conf_read(sc-sc_pc, sc-sc_tag,
sc-sc_cap_off + PCI_PCIE_SLCSR);
@@ -682,6 +691,7 @@ ppb_hotplug_insert(void *arg1, void *arg
return;
/* XXX Powerup the card. */
+printf (%s: the powerup case,sc-sc_dev.dv_xname);
/* XXX Turn on LEDs. */
@@ -702,7 +712,9 @@ ppb_hotplug_fixup(struct pci_attach_args
{
pcireg_t bhlcr;
+
bhlcr = pci_conf_read(pa-pa_pc, pa-pa_tag, PCI_BHLC_REG);
+printf ([XXX] : the fixup case %d, PCI_HDRTYPE_TYPE(bhlcr));
switch (PCI_HDRTYPE_TYPE(bhlcr)) {
case 0:
return ppb_hotplug_fixup_type0(pa-pa_pc,
@@ -789,9 +801,11 @@
Re: [PATCH] src/usr.bin/ftp/fetch.c: free ressl_config
On Fri, Sep 12, 2014 at 12:38:07AM -0700, Doug Hogan wrote:
Hmm this doesn't look right to me. ressl_config is not allocated the
same way as the other variables. The other variables such as ssl,
sslhost, etc are local to that function and allocated there.
ressl_config is a global and only allocated in main.c. It is allocated
once and configured using getopt/getsubopt().
If you free it, you would need to reallocate it each time or set it to
NULL so libressl will revert back and discard the user's changes.
I think either way is wrong. The SSL configuration should be retained
even if a single url_get() fails (which isn't fatal).
The above code is from url_get() which is called in a loop inside
auto_fetch(). I think the above change would use the getsubopt()
configuration for the first HTTPS URL argument and then try to use freed
memory for the others.
Of course, you're right. Is there any reason why ressl_config must be
global? It's only used in url_get. It should be harmless to create a
new one each time as long as we always free it.
Here's a diff for that:
Index: fetch.c
===
RCS file: /work/cvsroot/src/usr.bin/ftp/fetch.c,v
retrieving revision 1.129
diff -p -u -r1.129 fetch.c
--- fetch.c 25 Aug 2014 15:36:00 - 1.129
+++ fetch.c 12 Sep 2014 14:23:51 -
@@ -192,6 +192,7 @@ url_get(const char *origline, const char
char *locbase, *full_host = NULL;
const char *scheme;
int ishttpurl = 0, ishttpsurl = 0;
+ struct ressl_config *ressl_config = NULL;
#endif /* !SMALL */
struct ressl *ssl = NULL;
int status;
@@ -605,6 +606,26 @@ again:
fprintf(ttyout, failed to create SSL client\n);
goto cleanup_url_get;
}
+ if ((ressl_config = ressl_config_new()) == NULL) {
+ fprintf(ttyout, failed to create ReSSL config\n);
+ goto cleanup_url_get;
+ }
+
+ if (ssl_cafile != NULL)
+ ressl_config_set_ca_file(ressl_config, ssl_cafile);
+ if (ssl_capath != NULL)
+ ressl_config_set_ca_path(ressl_config, ssl_capath);
+ if (ssl_ciphers != NULL)
+ ressl_config_set_ciphers(ressl_config, ssl_ciphers);
+
+ if (ssl_verify)
+ ressl_config_verify(ressl_config);
+ else
+ ressl_config_insecure_no_verify(ressl_config);
+
+ if (ssl_verify_depth)
+ ressl_config_set_verify_depth(ressl_config,
ssl_verify_depth);
+
if (ressl_configure(ssl, ressl_config) != 0) {
fprintf(ttyout, SSL configuration failure: %s\n,
ressl_error(ssl));
@@ -978,6 +999,7 @@ cleanup_url_get:
#ifndef SMALL
if (ssl != NULL) {
ressl_close(ssl);
+ ressl_config_free(ressl_config);
ressl_free(ssl);
}
free(full_host);
Index: ftp_var.h
===
RCS file: /work/cvsroot/src/usr.bin/ftp/ftp_var.h,v
retrieving revision 1.35
diff -p -u -r1.35 ftp_var.h
--- ftp_var.h 14 Jul 2014 09:26:27 - 1.35
+++ ftp_var.h 12 Sep 2014 13:19:34 -
@@ -234,5 +234,9 @@ FILE*ttyout;/* stdout or stderr,
depe
extern struct cmd cmdtab[];
#ifndef SMALL
-extern struct ressl_config *ressl_config;
+char *ssl_cafile;
+char *ssl_capath;
+char *ssl_ciphers;
+int ssl_verify;
+int ssl_verify_depth;
#endif /* !SMALL */
Index: main.c
===
RCS file: /work/cvsroot/src/usr.bin/ftp/main.c,v
retrieving revision 1.92
diff -p -u -r1.92 main.c
--- main.c 16 Jul 2014 04:52:43 - 1.92
+++ main.c 12 Sep 2014 13:38:02 -
@@ -76,8 +76,6 @@
#include string.h
#include unistd.h
-#include ressl.h
-
#include cmds.h
#include ftp_var.h
@@ -97,8 +95,6 @@ char * const ssl_verify_opts[] = {
depth,
NULL
};
-
-struct ressl_config *ressl_config;
#endif /* !SMALL */
int family = PF_UNSPEC;
@@ -309,12 +305,6 @@ main(volatile int argc, char *argv[])
case 'S':
#ifndef SMALL
- if (ressl_config == NULL) {
- ressl_config = ressl_config_new();
- if (ressl_config == NULL)
- errx(1, ressl config failed);
- }
-
cp = optarg;
while (*cp) {
char*str;
@@ -322,28 +312,24 @@ main(volatile int argc, char *argv[])
case SSL_CAFILE:
if (str == NULL)
Re: [PATCH] src/usr.bin/ftp/fetch.c: free ressl_config
On Fri, Sep 12, 2014 at 09:39:30AM -0500, Kent R. Spillner wrote: Of course, you're right. Is there any reason why ressl_config must be global? It's only used in url_get. It should be harmless to create a new one each time as long as we always free it. Here's a diff for that: Forgot to add: ftp(1) regress tests pass on amd64-current with this diff.
Re: [PATCH] fix overflow handling in dd(1)
On Thu, 11 Sep 2014 22:03:04 -0700, William Orr wrote: I'm resubmitting this patch since the source tree was locked last time I submitted. Any thoughts? I think we've discussed this one to death already. It looks fine to me. - todd
Re: [PATCH] src/usr.bin/ftp/fetch.c: free ressl_config
On Fri, Sep 12, 2014 at 09:39, Kent R. Spillner wrote: On Fri, Sep 12, 2014 at 12:38:07AM -0700, Doug Hogan wrote: Hmm this doesn't look right to me. ressl_config is not allocated the same way as the other variables. The other variables such as ssl, sslhost, etc are local to that function and allocated there. ressl_config is a global and only allocated in main.c. It is allocated once and configured using getopt/getsubopt(). If you free it, you would need to reallocate it each time or set it to NULL so libressl will revert back and discard the user's changes. I think either way is wrong. The SSL configuration should be retained even if a single url_get() fails (which isn't fatal). The above code is from url_get() which is called in a loop inside auto_fetch(). I think the above change would use the getsubopt() configuration for the first HTTPS URL argument and then try to use freed memory for the others. Of course, you're right. Is there any reason why ressl_config must be global? It's only used in url_get. It should be harmless to create a new one each time as long as we always free it. If the config doesn't change, I don't think this gains us anything. The point of separating the config from the context was exactly so that you could use the same config several times. I think leaking the config is fine as it is.
LibreSSL: Extending EC_KEY or adding GOST_KEY?
Hello,
I'm polishing the GOST implementation for LibreSSL
(https://github.com/libressl-portable/openbsd/pull/6).
Currently there are three instances of ASN methods
and pmethods structures, because there three
different OIDs related to GOST public keys
(-2001, -2012, 256 bit, -2012 512 bit).
I think I can merge all of them. However I need
to store an additional NID (digest NID, which
is a part of public key parameters) together
with a _KEY instance. I see two possibilities.
First is to add a digest_nid field to EC_KEY
structure. Second is to add a
typedef struct {
EC_KEY *ec;
int digest_nid;
} GOST_KEY;
First way is a bit hackish. Second is clean but clumsy.
What would you prefer?
--
With best wishes
Dmitry
Re: improve ressl config setting
On Fri, 12 Sep 2014, Ted Unangst wrote:
On Wed, Sep 10, 2014 at 16:38, Ted Unangst wrote:
On Fri, Aug 15, 2014 at 13:06, Ted Unangst wrote:
Instead, ressl should copy all parameters as necessary and
free them. This does introduce an error case into formerly void
functions, but I think that's ok. The alternative would be to use
fixed arrays inside ressl_config, but that seems much worse.
Here's a complete diff.
Initial comments inline...
(I think we should also zero the key memory if not null, but that's not
included in this change.)
Kent Spillner noticed I hadn't cleaned up the references to default
config in ressl.c. Here's a fixed diff, that also zeroes key memory.
Index: ressl.c
===
RCS file: /cvs/src/lib/libressl/ressl.c,v
retrieving revision 1.12
diff -u -p -r1.12 ressl.c
--- ressl.c 15 Aug 2014 16:55:32 - 1.12
+++ ressl.c 11 Sep 2014 19:18:49 -
@@ -29,7 +29,7 @@
#include ressl.h
#include ressl_internal.h
-extern struct ressl_config ressl_config_default;
+static struct ressl_config *ressl_config_default;
int
ressl_init(void)
@@ -42,6 +42,9 @@ ressl_init(void)
SSL_load_error_strings();
SSL_library_init();
+ if ((ressl_config_default = ressl_config_new()) == NULL)
+ return (-1);
+
ressl_initialised = 1;
return (0);
@@ -78,7 +81,7 @@ ressl_new(void)
if ((ctx = calloc(1, sizeof(*ctx))) == NULL)
return (NULL);
- ctx-config = ressl_config_default;
+ ctx-config = ressl_config_default;
ressl_reset(ctx);
@@ -89,7 +92,7 @@ int
ressl_configure(struct ressl *ctx, struct ressl_config *config)
{
if (config == NULL)
- config = ressl_config_default;
+ config = ressl_config_default;
ctx-config = config;
Index: ressl.h
===
RCS file: /cvs/src/lib/libressl/ressl.h,v
retrieving revision 1.13
diff -u -p -r1.13 ressl.h
--- ressl.h 27 Aug 2014 10:46:53 - 1.13
+++ ressl.h 10 Sep 2014 20:23:46 -
@@ -31,15 +31,15 @@ const char *ressl_error(struct ressl *ct
struct ressl_config *ressl_config_new(void);
void ressl_config_free(struct ressl_config *config);
-void ressl_config_set_ca_file(struct ressl_config *config, char *ca_file);
-void ressl_config_set_ca_path(struct ressl_config *config, char *ca_path);
-void ressl_config_set_cert_file(struct ressl_config *config, char
*cert_file); -void ressl_config_set_cert_mem(struct ressl_config *config,
char *cert, +int ressl_config_set_ca_file(struct ressl_config *config, char
*ca_file); +int ressl_config_set_ca_path(struct ressl_config *config, char
*ca_path); +int ressl_config_set_cert_file(struct ressl_config *config,
char *cert_file); +int ressl_config_set_cert_mem(struct ressl_config
*config, char *cert, size_t len);
-void ressl_config_set_ciphers(struct ressl_config *config, char *ciphers);
+int ressl_config_set_ciphers(struct ressl_config *config, char *ciphers);
int ressl_config_set_ecdhcurve(struct ressl_config *config, const char *);
-void ressl_config_set_key_file(struct ressl_config *config, char
*key_file); -void ressl_config_set_key_mem(struct ressl_config *config,
char *key, +int ressl_config_set_key_file(struct ressl_config *config, char
*key_file); +int ressl_config_set_key_mem(struct ressl_config *config, char
*key, size_t len);
void ressl_config_set_verify_depth(struct ressl_config *config,
int verify_depth);
Index: ressl_config.c
===
RCS file: /cvs/src/lib/libressl/ressl_config.c,v
retrieving revision 1.8
diff -u -p -r1.8 ressl_config.c
--- ressl_config.c27 Aug 2014 10:46:53 - 1.8
+++ ressl_config.c11 Sep 2014 19:14:27 -
@@ -21,27 +21,55 @@
#include ressl.h
#include ressl_internal.h
-/*
- * Default configuration.
- */
-struct ressl_config ressl_config_default = {
- .ca_file = _PATH_SSL_CA_FILE,
- .ca_path = NULL,
- .ciphers = NULL,
- .ecdhcurve = NID_X9_62_prime256v1,
- .verify = 1,
- .verify_depth = 6,
-};
+#define SET_STRING(config, name, val) do { \
+ free(config-name); \
+ config-name = NULL;\
+ if (val != NULL) { \
+ if ((config-name = strdup(val)) == NULL) \
+ return -1; \
+ } \
+} while (0)
+
+#define SET_MEM(config, name, namelen, val, vallen) do { \
+ free(config-name); \
+ config-name = NULL;\
+ if (val != NULL) { \
+ if ((config-name = memdup(val, vallen)) == NULL) \
+ return -1; \
+ config-namelen = vallen; \
+ }
Re: Marking CIRCLEQ_* macros as deprecated in queue(3)
On Mon, 8 Sep 2014, Jona Joachim wrote: On 2014-09-06, Gr?goire Duch?ne gduch...@awhk.org wrote: Hi, Although CIRCLEQ_* macros are deprecated, queue(3) does not say so. perhaps it would be interesting to mention why circle queues got deprecated. Let's go a step further and remove them from the queue(3) manpage, leaving just a blurb about why and recommending conversion to tail queues. ok? Philip Index: queue.3 === RCS file: /cvs/src/share/man/man3/queue.3,v retrieving revision 1.59 diff -u -p -r1.59 queue.3 --- queue.3 14 Aug 2013 06:32:31 - 1.59 +++ queue.3 12 Sep 2014 19:46:19 - @@ -98,27 +98,8 @@ .Nm TAILQ_INSERT_HEAD , .Nm TAILQ_INSERT_TAIL , .Nm TAILQ_REMOVE , -.Nm TAILQ_REPLACE , -.Nm CIRCLEQ_ENTRY , -.Nm CIRCLEQ_HEAD , -.Nm CIRCLEQ_HEAD_INITIALIZER , -.Nm CIRCLEQ_FIRST , -.Nm CIRCLEQ_LAST , -.Nm CIRCLEQ_END , -.Nm CIRCLEQ_NEXT , -.Nm CIRCLEQ_PREV , -.Nm CIRCLEQ_EMPTY , -.Nm CIRCLEQ_FOREACH , -.Nm CIRCLEQ_FOREACH_SAFE , -.Nm CIRCLEQ_FOREACH_REVERSE_SAFE , -.Nm CIRCLEQ_INIT , -.Nm CIRCLEQ_INSERT_AFTER , -.Nm CIRCLEQ_INSERT_BEFORE , -.Nm CIRCLEQ_INSERT_HEAD , -.Nm CIRCLEQ_INSERT_TAIL , -.Nm CIRCLEQ_REMOVE , -.Nm CIRCLEQ_REPLACE -.Nd implementations of singly-linked lists, doubly-linked lists, simple queues, tail queues, and circular queues +.Nm TAILQ_REPLACE +.Nd implementations of singly-linked lists, doubly-linked lists, simple queues, and tail queues .Sh SYNOPSIS .In sys/queue.h .Pp @@ -233,44 +214,10 @@ .Fn TAILQ_REMOVE TAILQ_HEAD *head struct TYPE *elm FIELDNAME .Ft void .Fn TAILQ_REPLACE TAILQ_HEAD *head struct TYPE *elm struct TYPE *elm2 FIELDNAME -.Pp -.Fn CIRCLEQ_ENTRY TYPE -.Fn CIRCLEQ_HEAD HEADNAME TYPE -.Fn CIRCLEQ_HEAD_INITIALIZER CIRCLEQ_HEAD head -.Ft struct TYPE * -.Fn CIRCLEQ_FIRST CIRCLEQ_HEAD *head -.Ft struct TYPE * -.Fn CIRCLEQ_LAST CIRCLEQ_HEAD *head -.Ft struct TYPE * -.Fn CIRCLEQ_END CIRCLEQ_HEAD *head -.Ft struct TYPE * -.Fn CIRCLEQ_NEXT struct TYPE *listelm FIELDNAME -.Ft struct TYPE * -.Fn CIRCLEQ_PREV struct TYPE *listelm FIELDNAME -.Ft int -.Fn CIRCLEQ_EMPTY CIRCLEQ_HEAD *head -.Fn CIRCLEQ_FOREACH VARNAME CIRCLEQ_HEAD *head FIELDNAME -.Fn CIRCLEQ_FOREACH_SAFE VARNAME CIRCLEQ_HEAD *head FIELDNAME TEMP_VARNAME -.Fn CIRCLEQ_FOREACH_REVERSE VARNAME CIRCLEQ_HEAD *head FIELDNAME -.Fn CIRCLEQ_FOREACH_REVERSE_SAFE VARNAME CIRCLEQ_HEAD *head FIELDNAME TEMP_VARNAME -.Ft void -.Fn CIRCLEQ_INIT CIRCLEQ_HEAD *head -.Ft void -.Fn CIRCLEQ_INSERT_AFTER CIRCLEQ_HEAD *head struct TYPE *listelm struct TYPE *elm FIELDNAME -.Ft void -.Fn CIRCLEQ_INSERT_BEFORE CIRCLEQ_HEAD *head struct TYPE *listelm struct TYPE *elm FIELDNAME -.Ft void -.Fn CIRCLEQ_INSERT_HEAD CIRCLEQ_HEAD *head struct TYPE *elm FIELDNAME -.Ft void -.Fn CIRCLEQ_INSERT_TAIL CIRCLEQ_HEAD *head struct TYPE *elm FIELDNAME -.Ft void -.Fn CIRCLEQ_REMOVE CIRCLEQ_HEAD *head struct TYPE *elm FIELDNAME -.Ft void -.Fn CIRCLEQ_REPLACE CIRCLEQ_HEAD *head struct TYPE *elm struct TYPE *elm2 FIELDNAME .Sh DESCRIPTION -These macros define and operate on five types of data structures: -singly-linked lists, simple queues, lists, tail queues, and circular queues. -All five structures support the following functionality: +These macros define and operate on four types of data structures: +singly-linked lists, simple queues, lists, and tail queues. +All four structures support the following functionality: .Pp .Bl -enum -compact -offset indent .It @@ -283,7 +230,7 @@ Removal of an entry from the head of the Forward traversal through the list. .El .Pp -Singly-linked lists are the simplest of the five data structures +Singly-linked lists are the simplest of the four data structures and support only the above functionality. Singly-linked lists are ideal for applications with large datasets and few or no removals, or for implementing a LIFO queue. @@ -310,8 +257,8 @@ than singly-linked lists. Simple queues are ideal for applications with large datasets and few or no removals, or for implementing a FIFO queue. .Pp -All doubly linked types of data structures (lists, tail queues, and circle -queues) additionally allow: +All doubly linked types of data structures (lists and tail queues) +additionally allow: .Pp .Bl -enum -compact -offset indent .It @@ -354,27 +301,10 @@ Code size is about 15% greater and opera than singly-linked lists. .El .Pp -Circular queues add the following functionality: -.Pp -.Bl -enum -compact -offset indent -.It -Entries can be added at the end of a list. -.It -They may be traversed backwards, from tail to head. -.El -.Pp -However: -.Pp -.Bl -enum -compact -offset indent -.It -All list insertions and removals must specify the head of the list. -.It -Each head entry requires two pointers rather than one. -.It -The termination condition for traversal is more complex. -.It -Code size is about 40% greater and operations run about 45% slower than lists. -.El +An additional type of data structure,
Re: Marking CIRCLEQ_* macros as deprecated in queue(3)
No objection here. I'd go a step farther and nuke all the FOO_END macros as well. They add nothing and just make code harder to read. - todd
Re: Marking CIRCLEQ_* macros as deprecated in queue(3)
On Fri, 12 Sep 2014, Todd C. Miller wrote: No objection here. I'd go a step farther and nuke all the FOO_END macros as well. They add nothing and just make code harder to read. Better? Index: queue.3 === RCS file: /cvs/src/share/man/man3/queue.3,v retrieving revision 1.59 diff -u -p -r1.59 queue.3 --- queue.3 14 Aug 2013 06:32:31 - 1.59 +++ queue.3 12 Sep 2014 20:50:09 - @@ -39,7 +39,6 @@ .Nm SLIST_HEAD_INITIALIZER , .Nm SLIST_FIRST , .Nm SLIST_NEXT , -.Nm SLIST_END , .Nm SLIST_EMPTY , .Nm SLIST_FOREACH , .Nm SLIST_FOREACH_SAFE , @@ -54,7 +53,6 @@ .Nm LIST_HEAD_INITIALIZER , .Nm LIST_FIRST , .Nm LIST_NEXT , -.Nm LIST_END , .Nm LIST_EMPTY , .Nm LIST_FOREACH , .Nm LIST_FOREACH_SAFE , @@ -69,7 +67,6 @@ .Nm SIMPLEQ_HEAD_INITIALIZER , .Nm SIMPLEQ_FIRST , .Nm SIMPLEQ_NEXT , -.Nm SIMPLEQ_END , .Nm SIMPLEQ_EMPTY , .Nm SIMPLEQ_FOREACH , .Nm SIMPLEQ_FOREACH_SAFE , @@ -84,7 +81,6 @@ .Nm TAILQ_HEAD_INITIALIZER , .Nm TAILQ_FIRST , .Nm TAILQ_NEXT , -.Nm TAILQ_END , .Nm TAILQ_LAST , .Nm TAILQ_PREV , .Nm TAILQ_EMPTY , @@ -98,27 +94,8 @@ .Nm TAILQ_INSERT_HEAD , .Nm TAILQ_INSERT_TAIL , .Nm TAILQ_REMOVE , -.Nm TAILQ_REPLACE , -.Nm CIRCLEQ_ENTRY , -.Nm CIRCLEQ_HEAD , -.Nm CIRCLEQ_HEAD_INITIALIZER , -.Nm CIRCLEQ_FIRST , -.Nm CIRCLEQ_LAST , -.Nm CIRCLEQ_END , -.Nm CIRCLEQ_NEXT , -.Nm CIRCLEQ_PREV , -.Nm CIRCLEQ_EMPTY , -.Nm CIRCLEQ_FOREACH , -.Nm CIRCLEQ_FOREACH_SAFE , -.Nm CIRCLEQ_FOREACH_REVERSE_SAFE , -.Nm CIRCLEQ_INIT , -.Nm CIRCLEQ_INSERT_AFTER , -.Nm CIRCLEQ_INSERT_BEFORE , -.Nm CIRCLEQ_INSERT_HEAD , -.Nm CIRCLEQ_INSERT_TAIL , -.Nm CIRCLEQ_REMOVE , -.Nm CIRCLEQ_REPLACE -.Nd implementations of singly-linked lists, doubly-linked lists, simple queues, tail queues, and circular queues +.Nm TAILQ_REPLACE +.Nd implementations of singly-linked lists, doubly-linked lists, simple queues, and tail queues .Sh SYNOPSIS .In sys/queue.h .Pp @@ -129,8 +106,6 @@ .Fn SLIST_FIRST SLIST_HEAD *head .Ft struct TYPE * .Fn SLIST_NEXT struct TYPE *listelm FIELDNAME -.Ft struct TYPE * -.Fn SLIST_END SLIST_HEAD *head .Ft int .Fn SLIST_EMPTY SLIST_HEAD *head .Fn SLIST_FOREACH VARNAME SLIST_HEAD *head FIELDNAME @@ -155,8 +130,6 @@ .Fn LIST_FIRST LIST_HEAD *head .Ft struct TYPE * .Fn LIST_NEXT struct TYPE *listelm FIELDNAME -.Ft struct TYPE * -.Fn LIST_END LIST_HEAD *head .Ft int .Fn LIST_EMPTY LIST_HEAD *head .Fn LIST_FOREACH VARNAME LIST_HEAD *head FIELDNAME @@ -181,8 +154,6 @@ .Fn SIMPLEQ_FIRST SIMPLEQ_HEAD *head .Ft struct TYPE * .Fn SIMPLEQ_NEXT struct TYPE *listelm FIELDNAME -.Ft struct TYPE * -.Fn SIMPLEQ_END SIMPLEQ_HEAD *head .Ft int .Fn SIMPLEQ_EMPTY SIMPLEQ_HEAD *head .Fn SIMPLEQ_FOREACH VARNAME SIMPLEQ_HEAD *head FIELDNAME @@ -208,8 +179,6 @@ .Ft struct TYPE * .Fn TAILQ_NEXT struct TYPE *listelm FIELDNAME .Ft struct TYPE * -.Fn TAILQ_END TAILQ_HEAD *head -.Ft struct TYPE * .Fn TAILQ_LAST TAILQ_HEAD *head HEADNAME .Ft struct TYPE * .Fn TAILQ_PREV struct TYPE *listelm HEADNAME FIELDNAME @@ -233,44 +202,10 @@ .Fn TAILQ_REMOVE TAILQ_HEAD *head struct TYPE *elm FIELDNAME .Ft void .Fn TAILQ_REPLACE TAILQ_HEAD *head struct TYPE *elm struct TYPE *elm2 FIELDNAME -.Pp -.Fn CIRCLEQ_ENTRY TYPE -.Fn CIRCLEQ_HEAD HEADNAME TYPE -.Fn CIRCLEQ_HEAD_INITIALIZER CIRCLEQ_HEAD head -.Ft struct TYPE * -.Fn CIRCLEQ_FIRST CIRCLEQ_HEAD *head -.Ft struct TYPE * -.Fn CIRCLEQ_LAST CIRCLEQ_HEAD *head -.Ft struct TYPE * -.Fn CIRCLEQ_END CIRCLEQ_HEAD *head -.Ft struct TYPE * -.Fn CIRCLEQ_NEXT struct TYPE *listelm FIELDNAME -.Ft struct TYPE * -.Fn CIRCLEQ_PREV struct TYPE *listelm FIELDNAME -.Ft int -.Fn CIRCLEQ_EMPTY CIRCLEQ_HEAD *head -.Fn CIRCLEQ_FOREACH VARNAME CIRCLEQ_HEAD *head FIELDNAME -.Fn CIRCLEQ_FOREACH_SAFE VARNAME CIRCLEQ_HEAD *head FIELDNAME TEMP_VARNAME -.Fn CIRCLEQ_FOREACH_REVERSE VARNAME CIRCLEQ_HEAD *head FIELDNAME -.Fn CIRCLEQ_FOREACH_REVERSE_SAFE VARNAME CIRCLEQ_HEAD *head FIELDNAME TEMP_VARNAME -.Ft void -.Fn CIRCLEQ_INIT CIRCLEQ_HEAD *head -.Ft void -.Fn CIRCLEQ_INSERT_AFTER CIRCLEQ_HEAD *head struct TYPE *listelm struct TYPE *elm FIELDNAME -.Ft void -.Fn CIRCLEQ_INSERT_BEFORE CIRCLEQ_HEAD *head struct TYPE *listelm struct TYPE *elm FIELDNAME -.Ft void -.Fn CIRCLEQ_INSERT_HEAD CIRCLEQ_HEAD *head struct TYPE *elm FIELDNAME -.Ft void -.Fn CIRCLEQ_INSERT_TAIL CIRCLEQ_HEAD *head struct TYPE *elm FIELDNAME -.Ft void -.Fn CIRCLEQ_REMOVE CIRCLEQ_HEAD *head struct TYPE *elm FIELDNAME -.Ft void -.Fn CIRCLEQ_REPLACE CIRCLEQ_HEAD *head struct TYPE *elm struct TYPE *elm2 FIELDNAME .Sh DESCRIPTION -These macros define and operate on five types of data structures: -singly-linked lists, simple queues, lists, tail queues, and circular queues. -All five structures support the following functionality: +These macros define and operate on four types of data structures: +singly-linked lists, simple queues, lists, and tail queues. +All four structures support the following functionality: .Pp .Bl -enum -compact
Re: Marking CIRCLEQ_* macros as deprecated in queue(3)
On Fri, 12 Sep 2014 13:50:51 -0700, Philip Guenther wrote: On Fri, 12 Sep 2014, Todd C. Miller wrote: No objection here. I'd go a step farther and nuke all the FOO_END macros as well. They add nothing and just make code harder to read. Better? Looks good. OK millert@ - todd
Re: improve ressl config setting
On Sat, Sep 13, 2014 at 04:13, Joel Sing wrote:
I'm not convinced that we should be doing this with the *_mem() functions,
as
there is a benefit to the client owning this memory. Currently, in httpd the
public/private key is removed as soon as ressl_configure() is called.
Obviously libssl still has a copy squirrelled away in memory, however with
this change we now have two copies floating around.
I guess the other option would be for ressl_configure() to clean up the keys
as soon as it is done processing, however that then means that the caller
would have to call the *_mem() functions again if it was going to call
ressl_configure() again...
I think there are some interface problems here.
Let's look at what httpd is doing.
ressl_config_set_key_mem(srv-srv_ressl_config,
srv-srv_conf.ssl_key, srv-srv_conf.ssl_key_len);
if (ressl_configure(srv-srv_ressl_ctx, srv-srv_ressl_config) != 0) {
log_warn(%s: failed to configure SSL - %s, __func__,
ressl_error(srv-srv_ressl_ctx));
return (-1);
}
/* We're now done with the public/private key... */
explicit_bzero(srv-srv_conf.ssl_cert, srv-srv_conf.ssl_cert_len);
explicit_bzero(srv-srv_conf.ssl_key, srv-srv_conf.ssl_key_len);
free(srv-srv_conf.ssl_cert);
free(srv-srv_conf.ssl_key);
srv-srv_conf.ssl_cert = NULL;
srv-srv_conf.ssl_key = NULL;
This is dangerous. ressl_config now has a dangling pointer to the
freed ssl_key. The config object's lifetime extends beyond that of
this function. Not a problem in this instance, but it's easy to
imagine somebody creating a second ressl ctx with the same config and
the resulting fireworks.
If we're going to keep the interface as is, we would need to document
how long the caller needs to keep the key memory alive and for what
operations. I think that's a tangled mess I'd rather avoid by making
everything pass by value.
Right now it's not clear if a config object needs to outlive an
associated context or not. Something else to clarify.
Back to your original point, if the caller desires security, it's easy
to clear the memory by setting the key to NULL. (I need to add another
bzero in that case.)
Re: Marking CIRCLEQ_* macros as deprecated in queue(3)
On Fri, Sep 12, 2014 at 02:23:34PM -0600, Todd C. Miller wrote:
No objection here. I'd go a step farther and nuke all the FOO_END
macros as well. They add nothing and just make code harder to read.
In preparation for that, this replaces all queue_END macro calls.
Since we're not calling CIRCLEQ_* anywhere in the tree, there's no need
to keep the symmetry.
Ok?
Index: bin/md5/md5.c
===
RCS file: /cvs/src/bin/md5/md5.c,v
retrieving revision 1.76
diff -u -p -r1.76 md5.c
--- bin/md5/md5.c 19 Jun 2014 15:30:49 - 1.76
+++ bin/md5/md5.c 13 Sep 2014 01:44:57 -
@@ -263,7 +263,7 @@ main(int argc, char **argv)
strcmp(hf-name, hftmp-name) == 0)
break;
}
- if (hftmp == TAILQ_END(hl))
+ if (hftmp == NULL)
hash_insert(hl, hf, base64);
}
break;
Index: lib/libevent/event-internal.h
===
RCS file: /cvs/src/lib/libevent/event-internal.h,v
retrieving revision 1.6
diff -u -p -r1.6 event-internal.h
--- lib/libevent/event-internal.h 21 Apr 2010 20:02:40 - 1.6
+++ lib/libevent/event-internal.h 13 Sep 2014 01:45:11 -
@@ -78,7 +78,7 @@ struct event_base {
#defineTAILQ_NEXT(elm, field) ((elm)-field.tqe_next)
#define TAILQ_FOREACH(var, head, field)
\
for((var) = TAILQ_FIRST(head); \
- (var) != TAILQ_END(head); \
+ (var) != NULL; \
(var) = TAILQ_NEXT(var, field))
#defineTAILQ_INSERT_BEFORE(listelm, elm, field) do {
\
(elm)-field.tqe_prev = (listelm)-field.tqe_prev; \
Index: sbin/pfctl/parse.y
===
RCS file: /cvs/src/sbin/pfctl/parse.y,v
retrieving revision 1.638
diff -u -p -r1.638 parse.y
--- sbin/pfctl/parse.y 23 Aug 2014 00:11:03 - 1.638
+++ sbin/pfctl/parse.y 13 Sep 2014 01:45:15 -
@@ -1152,8 +1152,8 @@ tabledef : TABLE '' STRING '' table_op
YYERROR;
}
free($3);
- for (ti = SIMPLEQ_FIRST($5.init_nodes);
- ti != SIMPLEQ_END($5.init_nodes); ti = nti) {
+ for (ti = SIMPLEQ_FIRST($5.init_nodes); ti != NULL;
+ ti = nti) {
if (ti-file)
free(ti-file);
for (h = ti-host; h != NULL; h = nh) {
Index: sbin/pfctl/pfctl_optimize.c
===
RCS file: /cvs/src/sbin/pfctl/pfctl_optimize.c,v
retrieving revision 1.33
diff -u -p -r1.33 pfctl_optimize.c
--- sbin/pfctl/pfctl_optimize.c 22 Nov 2013 04:12:48 - 1.33
+++ sbin/pfctl/pfctl_optimize.c 13 Sep 2014 01:45:16 -
@@ -844,7 +844,7 @@ block_feedback(struct pfctl *pf, struct
break;
}
}
- if (por2 == TAILQ_END(block-sb_rules))
+ if (por2 == NULL)
TAILQ_INSERT_TAIL(block-sb_rules, por1, por_entry);
}
Index: sys/arch/alpha/dev/bus_dma.c
===
RCS file: /cvs/src/sys/arch/alpha/dev/bus_dma.c,v
retrieving revision 1.33
diff -u -p -r1.33 bus_dma.c
--- sys/arch/alpha/dev/bus_dma.c12 Jul 2014 18:44:40 - 1.33
+++ sys/arch/alpha/dev/bus_dma.c13 Sep 2014 01:45:16 -
@@ -511,7 +511,7 @@ _bus_dmamem_alloc_range(t, size, alignme
segs[curseg].ds_len = PAGE_SIZE;
m = TAILQ_NEXT(m, pageq);
- for (; m != TAILQ_END(mlist); m = TAILQ_NEXT(m, pageq)) {
+ for (; m != NULL; m = TAILQ_NEXT(m, pageq)) {
curaddr = VM_PAGE_TO_PHYS(m);
#ifdef DIAGNOSTIC
if (curaddr low || curaddr = high) {
Index: sys/arch/arm/arm/bus_dma.c
===
RCS file: /cvs/src/sys/arch/arm/arm/bus_dma.c,v
retrieving revision 1.26
diff -u -p -r1.26 bus_dma.c
--- sys/arch/arm/arm/bus_dma.c 12 Jul 2014 18:44:41 - 1.26
+++ sys/arch/arm/arm/bus_dma.c 13 Sep 2014 01:45:17 -
@@ -1033,7 +1033,7 @@ _bus_dmamem_alloc_range(bus_dma_tag_t t,
#endif /* DEBUG_DMA */
m = TAILQ_NEXT(m, pageq);
- for (; m != TAILQ_END(mlist); m = TAILQ_NEXT(m, pageq)) {
+ for (; m != NULL; m = TAILQ_NEXT(m, pageq)) {
curaddr = VM_PAGE_TO_PHYS(m);
#ifdef DIAGNOSTIC
