On Mon, Feb 05, 2018 at 06:52:39PM +0100, Jeremie Courreges-Anglas wrote:
>
> Hi,
>
> while reviewing an snmpd diff, I noticed that the fd in struct ber was
> always set to -1; and indeed snmpd, ldapd and ypldap only pass buffers
> to the ber API. So this diff removes support for direct read/writes
> from file descriptors and kills the related XXX.
>
> ok?
Sure, kill it with fire. Happy to see this go.
>
> Index: usr.sbin/ldapd/ber.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ldapd/ber.c,v
> retrieving revision 1.12
> diff -u -p -r1.12 ber.c
> --- usr.sbin/ldapd/ber.c 11 Feb 2017 20:40:03 -0000 1.12
> +++ usr.sbin/ldapd/ber.c 5 Feb 2018 17:51:37 -0000
> @@ -783,10 +783,6 @@ ber_write_elements(struct ber *ber, stru
> if (ber_dump_element(ber, root) == -1)
> return -1;
>
> - /* XXX this should be moved to a different function */
> - if (ber->fd != -1)
> - return write(ber->fd, ber->br_wbuf, len);
> -
> return (len);
> }
>
> @@ -1095,9 +1091,9 @@ ber_read_element(struct ber *ber, struct
> DPRINTF("ber read element size %zd\n", len);
> totlen += r + len;
>
> - /* If using an external buffer and the total size of the element
> - * is larger then the external buffer don't bother to continue. */
> - if (ber->fd == -1 && len > ber->br_rend - ber->br_rptr) {
> + /* If the total size of the element is larger than the buffer
> + * don't bother to continue. */
> + if (len > ber->br_rend - ber->br_rptr) {
> errno = ECANCELED;
> return -1;
> }
> @@ -1243,17 +1239,7 @@ ber_free(struct ber *b)
> static ssize_t
> ber_getc(struct ber *b, u_char *c)
> {
> - ssize_t r;
> - /*
> - * XXX calling read here is wrong in many ways. The most obvious one
> - * being that we will block till data arrives.
> - * But for now it is _good enough_ *gulp*
> - */
> - if (b->fd == -1)
> - r = ber_readbuf(b, c, 1);
> - else
> - r = read(b->fd, c, 1);
> - return r;
> + return ber_readbuf(b, c, 1);
> }
>
> static ssize_t
> @@ -1262,22 +1248,10 @@ ber_read(struct ber *ber, void *buf, siz
> u_char *b = buf;
> ssize_t r, remain = len;
>
> - /*
> - * XXX calling read here is wrong in many ways. The most obvious one
> - * being that we will block till data arrives.
> - * But for now it is _good enough_ *gulp*
> - */
> -
> while (remain > 0) {
> - if (ber->fd == -1)
> - r = ber_readbuf(ber, b, remain);
> - else
> - r = read(ber->fd, b, remain);
> - if (r == -1) {
> - if (errno == EINTR || errno == EAGAIN)
> - continue;
> + r = ber_readbuf(ber, b, remain);
> + if (r == -1)
> return -1;
> - }
> if (r == 0)
> return (b - (u_char *)buf);
> b += r;
> Index: usr.sbin/ldapd/ber.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ldapd/ber.h,v
> retrieving revision 1.2
> diff -u -p -r1.2 ber.h
> --- usr.sbin/ldapd/ber.h 11 Feb 2017 20:40:03 -0000 1.2
> +++ usr.sbin/ldapd/ber.h 5 Feb 2018 17:51:37 -0000
> @@ -35,7 +35,6 @@ struct ber_element {
> };
>
> struct ber {
> - int fd;
> u_char *br_wbuf;
> u_char *br_wptr;
> u_char *br_wend;
> Index: usr.sbin/ldapd/conn.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ldapd/conn.c,v
> retrieving revision 1.14
> diff -u -p -r1.14 conn.c
> --- usr.sbin/ldapd/conn.c 20 Jan 2017 11:55:08 -0000 1.14
> +++ usr.sbin/ldapd/conn.c 5 Feb 2018 17:51:37 -0000
> @@ -296,7 +296,6 @@ conn_accept(int fd, short event, void *d
> log_warn("malloc");
> goto giveup;
> }
> - conn->ber.fd = -1;
> ber_set_application(&conn->ber, ldap_application);
> conn->fd = afd;
> conn->listener = l;
> Index: usr.sbin/ldapd/util.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ldapd/util.c,v
> retrieving revision 1.8
> diff -u -p -r1.8 util.c
> --- usr.sbin/ldapd/util.c 20 Jan 2017 11:55:08 -0000 1.8
> +++ usr.sbin/ldapd/util.c 5 Feb 2018 17:51:37 -0000
> @@ -113,7 +113,6 @@ ber2db(struct ber_element *root, struct
> memset(val, 0, sizeof(*val));
>
> memset(&ber, 0, sizeof(ber));
> - ber.fd = -1;
> ber_write_elements(&ber, root);
>
> if ((len = ber_get_writebuf(&ber, &buf)) == -1)
> @@ -168,7 +167,6 @@ db2ber(struct btval *val, int compressio
> assert(val != NULL);
>
> memset(&ber, 0, sizeof(ber));
> - ber.fd = -1;
>
> if (compression_level > 0) {
> if (val->size < sizeof(uint32_t))
> Index: usr.sbin/snmpctl/snmpclient.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/snmpctl/snmpclient.c,v
> retrieving revision 1.14
> diff -u -p -r1.14 snmpclient.c
> --- usr.sbin/snmpctl/snmpclient.c 10 Aug 2017 16:03:10 -0000 1.14
> +++ usr.sbin/snmpctl/snmpclient.c 5 Feb 2018 17:51:37 -0000
> @@ -423,7 +423,6 @@ snmpc_sendreq(struct snmpc *sc, u_long t
> #endif
>
> bzero(&ber, sizeof(ber));
> - ber.fd = -1;
>
> len = ber_write_elements(&ber, root);
> if (ber_get_writebuf(&ber, (void *)&ptr) < 1)
> @@ -459,7 +458,6 @@ snmpc_recvresp(int s, int msgver, u_int3
> return (-1);
>
> bzero(&ber, sizeof(ber));
> - ber.fd = -1;
> ber_set_application(&ber, smi_application);
> ber_set_readbuf(&ber, buf, rlen);
>
> Index: usr.sbin/snmpd/ber.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/snmpd/ber.c,v
> retrieving revision 1.31
> diff -u -p -r1.31 ber.c
> --- usr.sbin/snmpd/ber.c 5 Mar 2016 03:31:36 -0000 1.31
> +++ usr.sbin/snmpd/ber.c 5 Feb 2018 17:51:37 -0000
> @@ -796,10 +796,6 @@ ber_write_elements(struct ber *ber, stru
> if (ber_dump_element(ber, root) == -1)
> return -1;
>
> - /* XXX this should be moved to a different function */
> - if (ber->fd != -1)
> - return write(ber->fd, ber->br_wbuf, len);
> -
> return (len);
> }
>
> @@ -1104,9 +1100,9 @@ ber_read_element(struct ber *ber, struct
> DPRINTF("ber read element size %zd\n", len);
> totlen += r + len;
>
> - /* If using an external buffer and the total size of the element
> - * is larger then the external buffer don't bother to continue. */
> - if (ber->fd == -1 && len > ber->br_rend - ber->br_rptr) {
> + /* If the total size of the element is larger than the buffer
> + * don't bother to continue. */
> + if (len > ber->br_rend - ber->br_rptr) {
> errno = ECANCELED;
> return -1;
> }
> @@ -1271,22 +1267,10 @@ ber_read(struct ber *ber, void *buf, siz
> u_char *b = buf;
> ssize_t r, remain = len;
>
> - /*
> - * XXX calling read here is wrong in many ways. The most obvious one
> - * being that we will block till data arrives.
> - * But for now it is _good enough_ *gulp*
> - */
> -
> while (remain > 0) {
> - if (ber->fd == -1)
> - r = ber_readbuf(ber, b, remain);
> - else
> - r = read(ber->fd, b, remain);
> - if (r == -1) {
> - if (errno == EINTR || errno == EAGAIN)
> - continue;
> + r = ber_readbuf(ber, b, remain);
> + if (r == -1)
> return -1;
> - }
> if (r == 0)
> return (b - (u_char *)buf);
> b += r;
> Index: usr.sbin/snmpd/ber.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/snmpd/ber.h,v
> retrieving revision 1.9
> diff -u -p -r1.9 ber.h
> --- usr.sbin/snmpd/ber.h 1 Oct 2013 12:41:47 -0000 1.9
> +++ usr.sbin/snmpd/ber.h 5 Feb 2018 17:51:37 -0000
> @@ -41,7 +41,6 @@ struct ber_element {
> };
>
> struct ber {
> - int fd;
> off_t br_offs;
> u_char *br_wbuf;
> u_char *br_wptr;
> Index: usr.sbin/snmpd/snmpe.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/snmpd/snmpe.c,v
> retrieving revision 1.50
> diff -u -p -r1.50 snmpe.c
> --- usr.sbin/snmpd/snmpe.c 12 Aug 2017 16:31:09 -0000 1.50
> +++ usr.sbin/snmpd/snmpe.c 5 Feb 2018 17:51:37 -0000
> @@ -514,7 +514,6 @@ snmpe_recvmsg(int fd, short sig, void *a
> msg->sm_datalen = (size_t)len;
>
> bzero(&msg->sm_ber, sizeof(msg->sm_ber));
> - msg->sm_ber.fd = -1;
> ber_set_application(&msg->sm_ber, smi_application);
> ber_set_readbuf(&msg->sm_ber, msg->sm_data, msg->sm_datalen);
>
> Index: usr.sbin/snmpd/trap.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/snmpd/trap.c,v
> retrieving revision 1.30
> diff -u -p -r1.30 trap.c
> --- usr.sbin/snmpd/trap.c 20 Jan 2018 18:49:12 -0000 1.30
> +++ usr.sbin/snmpd/trap.c 5 Feb 2018 17:51:37 -0000
> @@ -183,7 +183,6 @@ trap_send(struct ber_oid *oid, struct be
> ber_link_elements(c, elm);
>
> bzero(&ber, sizeof(ber));
> - ber.fd = -1;
>
> TAILQ_FOREACH(tr, &snmpd_env->sc_trapreceivers, entry) {
> if (tr->sa_oid != NULL && tr->sa_oid->bo_n) {
> Index: usr.sbin/snmpd/traphandler.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/snmpd/traphandler.c,v
> retrieving revision 1.10
> diff -u -p -r1.10 traphandler.c
> --- usr.sbin/snmpd/traphandler.c 5 Jan 2018 08:13:32 -0000 1.10
> +++ usr.sbin/snmpd/traphandler.c 5 Feb 2018 17:51:37 -0000
> @@ -218,7 +218,6 @@ traphandler_parse(char *buf, size_t n, s
> u_int vers, gtype, etype;
>
> bzero(&ber, sizeof(ber));
> - ber.fd = -1;
> ber_set_application(&ber, smi_application);
> ber_set_readbuf(&ber, buf, n);
>
> Index: usr.sbin/snmpd/usm.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/snmpd/usm.c,v
> retrieving revision 1.11
> diff -u -p -r1.11 usm.c
> --- usr.sbin/snmpd/usm.c 28 Oct 2016 08:01:53 -0000 1.11
> +++ usr.sbin/snmpd/usm.c 5 Feb 2018 17:51:37 -0000
> @@ -229,7 +229,6 @@ usm_decode(struct snmp_message *msg, str
> goto done;
> }
>
> - ber.fd = -1;
> ber_set_readbuf(&ber, usmparams, len);
> usm = ber_read_elements(&ber, NULL);
> if (usm == NULL) {
> @@ -341,7 +340,6 @@ usm_encode(struct snmp_message *msg, str
>
> msg->sm_digest_offs = 0;
> bzero(&ber, sizeof(ber));
> - ber.fd = -1;
>
> usm = ber_add_sequence(NULL);
>
> @@ -424,7 +422,6 @@ usm_encrypt(struct snmp_message *msg, st
> return pdu;
>
> bzero(&ber, sizeof(ber));
> - ber.fd = -1;
>
> #ifdef DEBUG
> fprintf(stderr, "encrypted PDU:\n");
> @@ -544,7 +541,6 @@ usm_decrypt(struct snmp_message *msg, st
> return NULL;
>
> bzero(&ber, sizeof(ber));
> - ber.fd = -1;
> ber_set_readbuf(&ber, buf, scoped_pdu_len);
> scoped_pdu = ber_read_elements(&ber, NULL);
>
> Index: usr.sbin/ypldap/aldap.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ypldap/aldap.c,v
> retrieving revision 1.38
> diff -u -p -r1.38 aldap.c
> --- usr.sbin/ypldap/aldap.c 21 Dec 2017 05:09:56 -0000 1.38
> +++ usr.sbin/ypldap/aldap.c 5 Feb 2018 17:51:37 -0000
> @@ -91,7 +91,6 @@ aldap_init(int fd)
> return NULL;
> a->buf = evbuffer_new();
> a->fd = fd;
> - a->ber.fd = -1;
> ber_set_application(&a->ber, aldap_application);
>
> return a;
> @@ -318,7 +317,6 @@ aldap_create_page_control(struct ber_ele
> struct ber_element *ber = NULL;
>
> c.br_wbuf = NULL;
> - c.fd = -1;
>
> ber = ber_add_sequence(NULL);
>
> @@ -462,7 +460,6 @@ aldap_parse_page_control(struct ber_elem
> struct aldap_page_control *page;
>
> b.br_wbuf = NULL;
> - b.fd = -1;
> ber_scanf_elements(control, "ss", &oid, &encoded);
> ber_set_readbuf(&b, encoded, control->be_next->be_len);
> elm = ber_read_elements(&b, NULL);
> Index: usr.sbin/ypldap/ber.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ypldap/ber.c,v
> retrieving revision 1.12
> diff -u -p -r1.12 ber.c
> --- usr.sbin/ypldap/ber.c 6 Apr 2016 13:04:10 -0000 1.12
> +++ usr.sbin/ypldap/ber.c 5 Feb 2018 17:51:37 -0000
> @@ -783,10 +783,6 @@ ber_write_elements(struct ber *ber, stru
> if (ber_dump_element(ber, root) == -1)
> return -1;
>
> - /* XXX this should be moved to a different function */
> - if (ber->fd != -1)
> - return write(ber->fd, ber->br_wbuf, len);
> -
> return (len);
> }
>
> @@ -1082,9 +1078,9 @@ ber_read_element(struct ber *ber, struct
> DPRINTF("ber read element size %zd\n", len);
> totlen += r + len;
>
> - /* If using an external buffer and the total size of the element
> - * is larger then the external buffer don't bother to continue. */
> - if (ber->fd == -1 && len > ber->br_rend - ber->br_rptr) {
> + /* If the total size of the element is larger than external
> + * buffer don't bother to continue. */
> + if (len > ber->br_rend - ber->br_rptr) {
> errno = ECANCELED;
> return -1;
> }
> @@ -1230,17 +1226,7 @@ ber_free(struct ber *b)
> static ssize_t
> ber_getc(struct ber *b, u_char *c)
> {
> - ssize_t r;
> - /*
> - * XXX calling read here is wrong in many ways. The most obvious one
> - * being that we will block till data arrives.
> - * But for now it is _good enough_ *gulp*
> - */
> - if (b->fd == -1)
> - r = ber_readbuf(b, c, 1);
> - else
> - r = read(b->fd, c, 1);
> - return r;
> + return ber_readbuf(b, c, 1);
> }
>
> static ssize_t
> @@ -1249,22 +1235,10 @@ ber_read(struct ber *ber, void *buf, siz
> u_char *b = buf;
> ssize_t r, remain = len;
>
> - /*
> - * XXX calling read here is wrong in many ways. The most obvious one
> - * being that we will block till data arrives.
> - * But for now it is _good enough_ *gulp*
> - */
> -
> while (remain > 0) {
> - if (ber->fd == -1)
> - r = ber_readbuf(ber, b, remain);
> - else
> - r = read(ber->fd, b, remain);
> - if (r == -1) {
> - if (errno == EINTR || errno == EAGAIN)
> - continue;
> + r = ber_readbuf(ber, b, remain);
> + if (r == -1)
> return -1;
> - }
> if (r == 0)
> return (b - (u_char *)buf);
> b += r;
> Index: usr.sbin/ypldap/ber.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ypldap/ber.h,v
> retrieving revision 1.2
> diff -u -p -r1.2 ber.h
> --- usr.sbin/ypldap/ber.h 29 Dec 2008 15:48:13 -0000 1.2
> +++ usr.sbin/ypldap/ber.h 5 Feb 2018 17:51:37 -0000
> @@ -35,7 +35,6 @@ struct ber_element {
> };
>
> struct ber {
> - int fd;
> u_char *br_wbuf;
> u_char *br_wptr;
> u_char *br_wend;
>
> --
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
--
:wq Claudio
