When looking at the recent changes in this file I noticed that the
presence of both ber_read() and ber_readbuf() was due to an incomplete
cleanup from my part.
revision 1.32
date: 2018/02/08 18:02:06; author: jca; state: Exp; lines: +6 -22;
commitid: YNQMco5lCS8YEifQ;
Kill ber.c support for direct fd read/writes
This mechanism is already unused and annotated with lots of XXX's, no
need to keep it around. ok claudio@
I think the recent changes would have been easier if the code had been
further simplified. Here's a shot at it:
- stop looping in ber_read(), there is no fd read to handle any more so
calling ber_readbuf() once is enough
- amend the condition which decides whether to return -1/ECANCELED:
- it makes little sense to pass a buffer length size of zero to
ber_read(), but then we should probably return 0 and not -1/ECANCELED
- I don't think we should perform partial reads: either the caller
function has the data it asked for, or nothing. Allowing partial
reads means that we're putting the burden of handling an incomplete
buffer on the caller.
- inline what remains of ber_readbuf() in ber_read()
regress tests for ldapd and snmpd pass, it would be cool to have test
reports from people who actually use those programs (ldap, ldapd, snmpd
and ypldap).
Looking for reviews and oks. The diff is not that long but I can split
it in smaller parts if it helps.
Index: usr.bin/ldap/ber.c
===================================================================
RCS file: /d/cvs/src/usr.bin/ldap/ber.c,v
retrieving revision 1.11
diff -u -p -r1.11 ber.c
--- usr.bin/ldap/ber.c 4 Jul 2018 15:21:24 -0000 1.11
+++ usr.bin/ldap/ber.c 6 Jul 2018 11:50:24 -0000
@@ -31,8 +31,6 @@
#include "ber.h"
-#define MINIMUM(a, b) (((a) < (b)) ? (a) : (b))
-
#define BER_TYPE_CONSTRUCTED 0x20 /* otherwise primitive */
#define BER_TYPE_SINGLE_MAX 30
#define BER_TAG_MASK 0x1f
@@ -48,7 +46,6 @@ static ssize_t get_id(struct ber *b, uns
int *cstruct);
static ssize_t get_len(struct ber *b, ssize_t *len);
static ssize_t ber_read_element(struct ber *ber, struct ber_element *elm);
-static ssize_t ber_readbuf(struct ber *b, void *buf, size_t nbytes);
static ssize_t ber_getc(struct ber *b, u_char *c);
static ssize_t ber_read(struct ber *ber, void *buf, size_t len);
@@ -1208,28 +1205,6 @@ ber_read_element(struct ber *ber, struct
return totlen;
}
-static ssize_t
-ber_readbuf(struct ber *b, void *buf, size_t nbytes)
-{
- size_t sz, len;
-
- if (b->br_rbuf == NULL)
- return -1;
-
- sz = b->br_rend - b->br_rptr;
- len = MINIMUM(nbytes, sz);
- if (len == 0) {
- errno = ECANCELED;
- return (-1); /* end of buffer and parser wants more data */
- }
-
- bcopy(b->br_rptr, buf, len);
- b->br_rptr += len;
- b->br_offs += len;
-
- return (len);
-}
-
void
ber_set_readbuf(struct ber *b, void *buf, size_t len)
{
@@ -1269,23 +1244,28 @@ ber_free(struct ber *b)
static ssize_t
ber_getc(struct ber *b, u_char *c)
{
- return ber_readbuf(b, c, 1);
+ return ber_read(b, c, 1);
}
static ssize_t
ber_read(struct ber *ber, void *buf, size_t len)
{
- u_char *b = buf;
- ssize_t r, remain = len;
+ size_t sz;
- while (remain > 0) {
- r = ber_readbuf(ber, b, remain);
- if (r == -1)
- return -1;
- b += r;
- remain -= r;
+ if (ber->br_rbuf == NULL)
+ return -1;
+
+ sz = ber->br_rend - ber->br_rptr;
+ if (len > sz) {
+ errno = ECANCELED;
+ return -1; /* parser wants more data than available */
}
- return (b - (u_char *)buf);
+
+ bcopy(ber->br_rptr, buf, len);
+ ber->br_rptr += len;
+ ber->br_offs += len;
+
+ return len;
}
int
Index: usr.sbin/ldapd/ber.c
===================================================================
RCS file: /d/cvs/src/usr.sbin/ldapd/ber.c,v
retrieving revision 1.21
diff -u -p -r1.21 ber.c
--- usr.sbin/ldapd/ber.c 4 Jul 2018 15:21:24 -0000 1.21
+++ usr.sbin/ldapd/ber.c 6 Jul 2018 11:47:55 -0000
@@ -31,8 +31,6 @@
#include "ber.h"
-#define MINIMUM(a, b) (((a) < (b)) ? (a) : (b))
-
#define BER_TYPE_CONSTRUCTED 0x20 /* otherwise primitive */
#define BER_TYPE_SINGLE_MAX 30
#define BER_TAG_MASK 0x1f
@@ -48,7 +46,6 @@ static ssize_t get_id(struct ber *b, uns
int *cstruct);
static ssize_t get_len(struct ber *b, ssize_t *len);
static ssize_t ber_read_element(struct ber *ber, struct ber_element *elm);
-static ssize_t ber_readbuf(struct ber *b, void *buf, size_t nbytes);
static ssize_t ber_getc(struct ber *b, u_char *c);
static ssize_t ber_read(struct ber *ber, void *buf, size_t len);
@@ -1208,28 +1205,6 @@ ber_read_element(struct ber *ber, struct
return totlen;
}
-static ssize_t
-ber_readbuf(struct ber *b, void *buf, size_t nbytes)
-{
- size_t sz, len;
-
- if (b->br_rbuf == NULL)
- return -1;
-
- sz = b->br_rend - b->br_rptr;
- len = MINIMUM(nbytes, sz);
- if (len == 0) {
- errno = ECANCELED;
- return (-1); /* end of buffer and parser wants more data */
- }
-
- bcopy(b->br_rptr, buf, len);
- b->br_rptr += len;
- b->br_offs += len;
-
- return (len);
-}
-
void
ber_set_readbuf(struct ber *b, void *buf, size_t len)
{
@@ -1269,23 +1244,28 @@ ber_free(struct ber *b)
static ssize_t
ber_getc(struct ber *b, u_char *c)
{
- return ber_readbuf(b, c, 1);
+ return ber_read(b, c, 1);
}
static ssize_t
ber_read(struct ber *ber, void *buf, size_t len)
{
- u_char *b = buf;
- ssize_t r, remain = len;
+ size_t sz;
- while (remain > 0) {
- r = ber_readbuf(ber, b, remain);
- if (r == -1)
- return -1;
- b += r;
- remain -= r;
+ if (ber->br_rbuf == NULL)
+ return -1;
+
+ sz = ber->br_rend - ber->br_rptr;
+ if (len > sz) {
+ errno = ECANCELED;
+ return -1; /* parser wants more data than available */
}
- return (b - (u_char *)buf);
+
+ bcopy(ber->br_rptr, buf, len);
+ ber->br_rptr += len;
+ ber->br_offs += len;
+
+ return len;
}
int
Index: usr.sbin/snmpd/ber.c
===================================================================
RCS file: /d/cvs/src/usr.sbin/snmpd/ber.c,v
retrieving revision 1.40
diff -u -p -r1.40 ber.c
--- usr.sbin/snmpd/ber.c 4 Jul 2018 15:21:24 -0000 1.40
+++ usr.sbin/snmpd/ber.c 6 Jul 2018 11:50:18 -0000
@@ -31,8 +31,6 @@
#include "ber.h"
-#define MINIMUM(a, b) (((a) < (b)) ? (a) : (b))
-
#define BER_TYPE_CONSTRUCTED 0x20 /* otherwise primitive */
#define BER_TYPE_SINGLE_MAX 30
#define BER_TAG_MASK 0x1f
@@ -48,7 +46,6 @@ static ssize_t get_id(struct ber *b, uns
int *cstruct);
static ssize_t get_len(struct ber *b, ssize_t *len);
static ssize_t ber_read_element(struct ber *ber, struct ber_element *elm);
-static ssize_t ber_readbuf(struct ber *b, void *buf, size_t nbytes);
static ssize_t ber_getc(struct ber *b, u_char *c);
static ssize_t ber_read(struct ber *ber, void *buf, size_t len);
@@ -1208,28 +1205,6 @@ ber_read_element(struct ber *ber, struct
return totlen;
}
-static ssize_t
-ber_readbuf(struct ber *b, void *buf, size_t nbytes)
-{
- size_t sz, len;
-
- if (b->br_rbuf == NULL)
- return -1;
-
- sz = b->br_rend - b->br_rptr;
- len = MINIMUM(nbytes, sz);
- if (len == 0) {
- errno = ECANCELED;
- return (-1); /* end of buffer and parser wants more data */
- }
-
- bcopy(b->br_rptr, buf, len);
- b->br_rptr += len;
- b->br_offs += len;
-
- return (len);
-}
-
void
ber_set_readbuf(struct ber *b, void *buf, size_t len)
{
@@ -1269,23 +1244,28 @@ ber_free(struct ber *b)
static ssize_t
ber_getc(struct ber *b, u_char *c)
{
- return ber_readbuf(b, c, 1);
+ return ber_read(b, c, 1);
}
static ssize_t
ber_read(struct ber *ber, void *buf, size_t len)
{
- u_char *b = buf;
- ssize_t r, remain = len;
+ size_t sz;
- while (remain > 0) {
- r = ber_readbuf(ber, b, remain);
- if (r == -1)
- return -1;
- b += r;
- remain -= r;
+ if (ber->br_rbuf == NULL)
+ return -1;
+
+ sz = ber->br_rend - ber->br_rptr;
+ if (len > sz) {
+ errno = ECANCELED;
+ return -1; /* parser wants more data than available */
}
- return (b - (u_char *)buf);
+
+ bcopy(ber->br_rptr, buf, len);
+ ber->br_rptr += len;
+ ber->br_offs += len;
+
+ return len;
}
int
Index: usr.sbin/ypldap/ber.c
===================================================================
RCS file: /d/cvs/src/usr.sbin/ypldap/ber.c,v
retrieving revision 1.23
diff -u -p -r1.23 ber.c
--- usr.sbin/ypldap/ber.c 4 Jul 2018 15:21:24 -0000 1.23
+++ usr.sbin/ypldap/ber.c 8 Jul 2018 13:34:13 -0000
@@ -31,8 +31,6 @@
#include "ber.h"
-#define MINIMUM(a, b) (((a) < (b)) ? (a) : (b))
-
#define BER_TYPE_CONSTRUCTED 0x20 /* otherwise primitive */
#define BER_TYPE_SINGLE_MAX 30
#define BER_TAG_MASK 0x1f
@@ -48,7 +46,6 @@ static ssize_t get_id(struct ber *b, uns
int *cstruct);
static ssize_t get_len(struct ber *b, ssize_t *len);
static ssize_t ber_read_element(struct ber *ber, struct ber_element *elm);
-static ssize_t ber_readbuf(struct ber *b, void *buf, size_t nbytes);
static ssize_t ber_getc(struct ber *b, u_char *c);
static ssize_t ber_read(struct ber *ber, void *buf, size_t len);
@@ -1208,28 +1205,6 @@ ber_read_element(struct ber *ber, struct
return totlen;
}
-static ssize_t
-ber_readbuf(struct ber *b, void *buf, size_t nbytes)
-{
- size_t sz, len;
-
- if (b->br_rbuf == NULL)
- return -1;
-
- sz = b->br_rend - b->br_rptr;
- len = MINIMUM(nbytes, sz);
- if (len == 0) {
- errno = ECANCELED;
- return (-1); /* end of buffer and parser wants more data */
- }
-
- bcopy(b->br_rptr, buf, len);
- b->br_rptr += len;
- b->br_offs += len;
-
- return (len);
-}
-
void
ber_set_readbuf(struct ber *b, void *buf, size_t len)
{
@@ -1269,23 +1244,28 @@ ber_free(struct ber *b)
static ssize_t
ber_getc(struct ber *b, u_char *c)
{
- return ber_readbuf(b, c, 1);
+ return ber_read(b, c, 1);
}
static ssize_t
ber_read(struct ber *ber, void *buf, size_t len)
{
- u_char *b = buf;
- ssize_t r, remain = len;
+ size_t sz;
- while (remain > 0) {
- r = ber_readbuf(ber, b, remain);
- if (r == -1)
- return -1;
- b += r;
- remain -= r;
+ if (ber->br_rbuf == NULL)
+ return -1;
+
+ sz = ber->br_rend - ber->br_rptr;
+ if (len > sz) {
+ errno = ECANCELED;
+ return -1; /* parser wants more data than available */
}
- return (b - (u_char *)buf);
+
+ bcopy(ber->br_rptr, buf, len);
+ ber->br_rptr += len;
+ ber->br_offs += len;
+
+ return len;
}
int
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
