Re: [PATCH] column(1): -r to right justify

2018-07-08 Thread Ingo Schwarze
Hi Job,

Job Snijders wrote on Wed, Jul 04, 2018 at 02:09:56PM +0200:

> Following some back and forth on how disklabel output should be
> formatted, I proposed to Kenneth to extend the column(1) utility.
> All that was missing is the ability to right justify.

I'm not totally sure we should add the feature, but maybe you are
right that it's useful enough, and column(1) still remains simple
enough.  We have to pay attention not to turn it into a pig
resembling rs(1), though.

> Patch courtesy of Kenneth R Westerback. OK?

No.  I see at least two aspects of the patch that are clearly
not yet OK, see inline.


> Index: column.1
> ===
> RCS file: /cvs/src/usr.bin/column/column.1,v
> retrieving revision 1.18
> diff -u -p -r1.18 column.1
> --- column.1  24 Oct 2016 13:53:05 -  1.18
> +++ column.1  4 Jul 2018 10:27:54 -
> @@ -40,6 +40,7 @@
>  .Nm column
>  .Op Fl tx
>  .Op Fl c Ar columns
> +.Op Fl r Op Ar list

This is unacceptable.  A few programs exist that take options with
optional arguments, but we certainly do not want to add new ones
because their syntax and semantics is inherently ambiguous and error
prone.  POSIX also discourages it in general.

An option can either take no argument or require an argument,
so make a decision in that respect.  If you opt for requiring
an argument, then you can use a cut(1)-like syntax:

  cut -b 2-3# bytes 2 and 3
  cut -b 2- # all but the first byte
  cut -b -2 # the first two bytes
  cut -b 1- # all bytes, not cutting away anything
  cut -b -2,4-  # all but the third byte

[...]
> Index: column.c
> ===
> RCS file: /cvs/src/usr.bin/column/column.c,v
> retrieving revision 1.26
> diff -u -p -r1.26 column.c
> --- column.c  22 Jun 2018 12:27:00 -  1.26
> +++ column.c  4 Jul 2018 10:28:00 -
[...]
> + rflag = 0; tflag = xflag = 0;

That looks inconsistent.  s/0; //

[...]
> @@ -207,18 +213,69 @@ print(void)
>   puts(table[row]->content);
>  }
>  
> +int
> +rightjustify(const char *rcols, const int col)
> +{
> + const char *errstr;
> + char c, *num, *temp;
> + long long ch, rangestart;
> + unsigned int i;
> +
> + if (rcols == NULL)
> + return 1;
> +
> + temp = strdup(rcols);
> + num = temp;
> + rangestart = -1;
> +
> + c = 0;
> + for (i = 0; i <= strlen(rcols); i++) {
> + ch = temp[i];
> + if (ch == ',' || ch == '-')
> + temp[i] = '\0';
> + if (temp[i] != '\0')
> + continue;
> +
> + c = strtonum(num, 1, INT_MAX, );

This is not acceptable either.

Your algorithm is of the order

  O(rows) * O(columns) * O(strlen(rcols))

and then calling strdup(3) on the quadratic level and strtonum(3)
inside the cubic loop.  Even though performance optimization is
often over-valued, that is not an excuse for selecting a basic
algorithm with an excessively high algorithmic order.

You really have to do argument parsing once before entering the
quadratic main loop rather than redoing it from scratch for each
and every table cell.

[...]
>  void
> -maketbl(void)
> +maketbl(const int rflag, const char *rcols)
>  {
>   struct field **row;
>   int col;
>  
>   for (row = table; entries--; ++row) {
> - for (col = 0; (*row)[col + 1].content != NULL; ++col)
[...]
> + for (col = 0; (*row)[col].content != NULL; ++col) {

Here, you are losing the feature that column(1) refrains from
printing trailing blanks.

I'm putting the audit on hold at this points because fixing the
issues found so far is going to change the patch substantially.

Yours,
  Ingo



Re: if_enc.c: "rdomain" is more descriptive than "id"

2018-07-08 Thread Sebastian Benoit
Jeremie Courreges-Anglas(j...@wxcvbn.org) on 2018.07.08 15:26:33 +0200:
> 
> Maybe not very important, but "id" sounds a tad too generic to me, and
> it looks like we're really dealing with an rdomain here, so here's a diff.
> 
> Thoughts?

yes please, if only for making searching easier

> ok?

yes reads ok

> 
> 
> Index: net/if_enc.c
> ===
> RCS file: /cvs/src/sys/net/if_enc.c,v
> retrieving revision 1.72
> diff -u -p -r1.72 if_enc.c
> --- net/if_enc.c  8 Jul 2018 13:04:04 -   1.72
> +++ net/if_enc.c  8 Jul 2018 13:20:13 -
> @@ -36,7 +36,7 @@
>  #endif
>  
>  struct ifnet **enc_ifps; /* rdomain-mapped enc ifs */
> -u_int  enc_max_id;
> +u_int  enc_max_rdomain;
>  struct ifnet **enc_allifps;  /* unit-mapped enc ifs */
>  u_int  enc_max_unit;
>  #define ENC_MAX_UNITS  4096  /* XXX n per rdomain */
> @@ -210,7 +210,7 @@ enc_ioctl(struct ifnet *ifp, u_long cmd,
>  }
>  
>  struct ifnet *
> -enc_getif(u_int id, u_int unit)
> +enc_getif(u_int rdomain, u_int unit)
>  {
>   struct ifnet*ifp;
>  
> @@ -221,7 +221,7 @@ enc_getif(u_int id, u_int unit)
>   if (unit > enc_max_unit)
>   return (NULL);
>   ifp = enc_allifps[unit];
> - if (ifp == NULL || ifp->if_rdomain != id)
> + if (ifp == NULL || ifp->if_rdomain != rdomain)
>   return (NULL);
>   return (ifp);
>   }
> @@ -229,20 +229,20 @@ enc_getif(u_int id, u_int unit)
>   /* Otherwise return the default enc interface for this rdomain */
>   if (enc_ifps == NULL)
>   return (NULL);
> - else if (id > RT_TABLEID_MAX)
> + else if (rdomain > RT_TABLEID_MAX)
>   return (NULL);
> - else if (id > enc_max_id)
> + else if (rdomain > enc_max_rdomain)
>   return (NULL);
> - return (enc_ifps[id]);
> + return (enc_ifps[rdomain]);
>  }
>  
>  struct ifaddr *
> -enc_getifa(u_int id, u_int unit)
> +enc_getifa(u_int rdomain, u_int unit)
>  {
>   struct ifnet*ifp;
>   struct enc_softc*sc;
>  
> - ifp = enc_getif(id, unit);
> + ifp = enc_getif(rdomain, unit);
>   if (ifp == NULL)
>   return (NULL);
>  
> @@ -250,7 +250,7 @@ enc_getifa(u_int id, u_int unit)
>   return (>sc_ifa);
>  }
>  int
> -enc_setif(struct ifnet *ifp, u_int id)
> +enc_setif(struct ifnet *ifp, u_int rdomain)
>  {
>   struct ifnet**new;
>   size_t   newlen;
> @@ -265,28 +265,28 @@ enc_setif(struct ifnet *ifp, u_int id)
>* for this rdomain, so only the first enc interface that
>* was added for this rdomain becomes the default.
>*/
> - if (enc_getif(id, 0) != NULL)
> + if (enc_getif(rdomain, 0) != NULL)
>   return (0);
>  
> - if (id > RT_TABLEID_MAX)
> + if (rdomain > RT_TABLEID_MAX)
>   return (EINVAL);
>  
> - if (enc_ifps == NULL || id > enc_max_id) {
> - if ((new = mallocarray(id + 1, sizeof(struct ifnet *),
> + if (enc_ifps == NULL || rdomain > enc_max_rdomain) {
> + if ((new = mallocarray(rdomain + 1, sizeof(struct ifnet *),
>   M_DEVBUF, M_NOWAIT|M_ZERO)) == NULL)
>   return (ENOBUFS);
> - newlen = sizeof(struct ifnet *) * (id + 1);
> + newlen = sizeof(struct ifnet *) * (rdomain + 1);
>  
>   if (enc_ifps != NULL) {
>   memcpy(new, enc_ifps,
> - sizeof(struct ifnet *) * (enc_max_id + 1));
> + sizeof(struct ifnet *) * (enc_max_rdomain + 1));
>   free(enc_ifps, M_DEVBUF, 0);
>   }
>   enc_ifps = new;
> - enc_max_id = id;
> + enc_max_rdomain = rdomain;
>   }
>  
> - enc_ifps[id] = ifp;
> + enc_ifps[rdomain] = ifp;
>  
>   /* Indicate that this interface is the rdomain default */
>   ifp->if_link_state = LINK_STATE_UP;
> @@ -297,14 +297,14 @@ enc_setif(struct ifnet *ifp, u_int id)
>  void
>  enc_unsetif(struct ifnet *ifp)
>  {
> - u_intid = ifp->if_rdomain, i;
> + u_intrdomain = ifp->if_rdomain, i;
>   struct ifnet*oifp, *nifp;
>  
> - if ((oifp = enc_getif(id, 0)) == NULL || oifp != ifp)
> + if ((oifp = enc_getif(rdomain, 0)) == NULL || oifp != ifp)
>   return;
>  
>   /* Clear slot for this rdomain */
> - enc_ifps[id] = NULL;
> + enc_ifps[rdomain] = NULL;
>   ifp->if_link_state = LINK_STATE_UNKNOWN;
>  
>   /*
> @@ -314,10 +314,10 @@ enc_unsetif(struct ifnet *ifp)
>   for (i = 0; i < (enc_max_unit + 1); i++) {
>   nifp = enc_allifps[i];
>  
> - if (nifp == NULL || nifp == ifp 

Re: xenodm: source Xsetup before initializing the greeter

2018-07-08 Thread Jeremie Courreges-Anglas
On Sun, Jul 08 2018, Matthieu Herrb  wrote:
> Hi,
>
> prompted by a question by weerd@ who wanted to be able to rotate the
> screen of his gpd win *before* the Xenodm greeter is displayed, I came
> up with this (simple) patch that sources the Xsetup_0 script before
> initializing the greeter widget.
>
> I can't think of any down sides of doing this, and it may even help
> with some further pledges or privilege separation for the greeter.
>
> ok ?

fwiw no regression in my simple setup (xsession that starts both
one-shot and long-running X programs).

> Index: greeter/greet.c
> ===
> RCS file: /cvs/OpenBSD/xenocara/app/xenodm/greeter/greet.c,v
> retrieving revision 1.5
> diff -u -r1.5 greet.c
> --- greeter/greet.c   6 May 2018 15:25:27 -   1.5
> +++ greeter/greet.c   30 Jun 2018 08:51:44 -
> @@ -301,13 +301,13 @@
>  Arg  arglist[2];
>  Display*dpy;
>  
> -dpy = InitGreet (d);
>  /*
>   * Run the setup script - note this usually will not work when
>   * the server is grabbed, so we don't even bother trying.
>   */
>  if (!d->grabServer)
>   SetupDisplay (d);
> +dpy = InitGreet (d);
>  if (!dpy) {
>   LogError ("Cannot reopen display %s for greet window\n", d->name);
>   exit (RESERVER_DISPLAY);


-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE



Re: Fix a kernelpanic when playing with rdomain(4) and enc(4)

2018-07-08 Thread Jeremie Courreges-Anglas
On Sun, Jun 24 2018, Denis Fondras  wrote:
> When removing enc(4) interface from rdomain, the kernel panics randomly
> (memcpy() seems to copy outside of the mallocarray() boundaries) with 
> something
> like :
>
> Data modified on freelist: word -35183699295756 of object 0x8059da80 
> size 0x8 previous type free (invalid addr 0x7b44962aa448c22a)
> kernel: protection fault trap, code=0
> Stopped at  malloc+0x4d3:   movq0x8(%r14),%rbx

[...]

> Here is a fix :
>
> Index: if_enc.c
> ===
> RCS file: /cvs/src/sys/net/if_enc.c,v
> retrieving revision 1.70
> diff -u -p -r1.70 if_enc.c
> --- if_enc.c  16 Oct 2017 08:22:25 -  1.70
> +++ if_enc.c  24 Jun 2018 17:15:32 -
> @@ -271,7 +271,7 @@ enc_setif(struct ifnet *ifp, u_int id)
>   if (id > RT_TABLEID_MAX)
>   return (EINVAL);
>  
> - if (id == 0 || id > enc_max_id) {
> + if (enc_ifps == NULL || id > enc_max_id) {
>   if ((new = mallocarray(id + 1, sizeof(struct ifnet *),
>   M_DEVBUF, M_NOWAIT|M_ZERO)) == NULL)
>   return (ENOBUFS);

This pattern is also used in enc_clone_create(), even if right now we
can't call this function twice for enc0 I think the code should be made
consistent.  ok?


Index: net/if_enc.c
===
--- net/if_enc.c.orig
+++ net/if_enc.c
@@ -120,7 +120,7 @@ enc_clone_create(struct if_clone *ifc, i
return (error);
}
 
-   if (unit == 0 || unit > enc_max_unit) {
+   if (enc_ifps == NULL || unit > enc_max_unit) {
if ((new = mallocarray(unit + 1, sizeof(struct ifnet *),
M_DEVBUF, M_NOWAIT|M_ZERO)) == NULL) {
NET_UNLOCK();


-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE



Re: systat(1) iostat vs. iostat(1)

2018-07-08 Thread Sebastian Benoit
yes, its bytes.

ok benno@

Jason McIntyre(j...@kerhand.co.uk) on 2018.07.07 17:20:19 +0100:
> On Sat, Jul 07, 2018 at 08:28:52AM +0200, Marcus MERIGHI wrote:
> > Hello, 
> > 
> > when I run 'iostat(1) sd1 1' I get the transfer speed in MB/s. Running
> > 'systat(1) iostat 1' in parallel I get the transfer speed in bytes, as
> > calculations show. Therefore adjust the manual of systat(1): 
> > kilobytes -> bytes.
> > 
> > Marcus
> > 
> 
> someone (obsd dev) commit this or give me an ok, please.
> jmc
> 
> > Index: systat.1
> > ===
> > RCS file: /cvs/src/usr.bin/systat/systat.1,v
> > retrieving revision 1.106
> > diff -u -p -u -r1.106 systat.1
> > --- systat.130 May 2018 13:53:09 -  1.106
> > +++ systat.17 Jul 2018 06:20:03 -
> > @@ -293,7 +293,7 @@ this is the default.
> >  .It Ic iostat
> >  Display statistics about disk throughput.
> >  Statistics
> > -on disk throughput show, for each drive, data transferred in kilobytes,
> > +on disk throughput show, for each drive, data transferred in bytes,
> >  number of disk transactions performed, and time spent in disk accesses
> >  (in fractions of a second).
> >  .It Ic malloc
> > 
> 



Re: Fix a kernelpanic when playing with rdomain(4) and enc(4)

2018-07-08 Thread Jeremie Courreges-Anglas
On Sun, Jul 08 2018, Jeremie Courreges-Anglas  wrote:
> On Sun, Jun 24 2018, Denis Fondras  wrote:
>> When removing enc(4) interface from rdomain, the kernel panics randomly
>> (memcpy() seems to copy outside of the mallocarray() boundaries) with 
>> something
>> like :
>>
>> Data modified on freelist: word -35183699295756 of object 0x8059da80 
>> size 0x8 previous type free (invalid addr 0x7b44962aa448c22a)
>> kernel: protection fault trap, code=0
>> Stopped at  malloc+0x4d3:   movq0x8(%r14),%rbx
>
> [...]
>
>> Here is a fix :
>>
>> Index: if_enc.c
>> ===
>> RCS file: /cvs/src/sys/net/if_enc.c,v
>> retrieving revision 1.70
>> diff -u -p -r1.70 if_enc.c
>> --- if_enc.c 16 Oct 2017 08:22:25 -  1.70
>> +++ if_enc.c 24 Jun 2018 17:15:32 -
>> @@ -271,7 +271,7 @@ enc_setif(struct ifnet *ifp, u_int id)
>>  if (id > RT_TABLEID_MAX)
>>  return (EINVAL);
>>  
>> -if (id == 0 || id > enc_max_id) {
>> +if (enc_ifps == NULL || id > enc_max_id) {
>>  if ((new = mallocarray(id + 1, sizeof(struct ifnet *),
>>  M_DEVBUF, M_NOWAIT|M_ZERO)) == NULL)
>>  return (ENOBUFS);
>
> This pattern is also used in enc_clone_create(), even if right now we
> can't call this function twice for enc0 I think the code should be made
> consistent.  ok?

Better send the correct diff, thanks Denis for the heads-up.


Index: net/if_enc.c
===
--- net/if_enc.c.orig
+++ net/if_enc.c
@@ -120,7 +120,7 @@ enc_clone_create(struct if_clone *ifc, i
return (error);
}
 
-   if (unit == 0 || unit > enc_max_unit) {
+   if (enc_allifps == NULL || unit > enc_max_unit) {
if ((new = mallocarray(unit + 1, sizeof(struct ifnet *),
M_DEVBUF, M_NOWAIT|M_ZERO)) == NULL) {
NET_UNLOCK();


-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE



Re: Fix a kernelpanic when playing with rdomain(4) and enc(4)

2018-07-08 Thread Denis Fondras
OK denis@

On Sun, Jul 08, 2018 at 02:04:56PM +0200, Jeremie Courreges-Anglas wrote:
> On Sun, Jul 08 2018, Jeremie Courreges-Anglas  wrote:
> > On Sun, Jun 24 2018, Denis Fondras  wrote:
> >> When removing enc(4) interface from rdomain, the kernel panics randomly
> >> (memcpy() seems to copy outside of the mallocarray() boundaries) with 
> >> something
> >> like :
> >>
> >> Data modified on freelist: word -35183699295756 of object 
> >> 0x8059da80 size 0x8 previous type free (invalid addr 
> >> 0x7b44962aa448c22a)
> >> kernel: protection fault trap, code=0
> >> Stopped at  malloc+0x4d3:   movq0x8(%r14),%rbx
> >
> > [...]
> >
> >> Here is a fix :
> >>
> >> Index: if_enc.c
> >> ===
> >> RCS file: /cvs/src/sys/net/if_enc.c,v
> >> retrieving revision 1.70
> >> diff -u -p -r1.70 if_enc.c
> >> --- if_enc.c   16 Oct 2017 08:22:25 -  1.70
> >> +++ if_enc.c   24 Jun 2018 17:15:32 -
> >> @@ -271,7 +271,7 @@ enc_setif(struct ifnet *ifp, u_int id)
> >>if (id > RT_TABLEID_MAX)
> >>return (EINVAL);
> >>  
> >> -  if (id == 0 || id > enc_max_id) {
> >> +  if (enc_ifps == NULL || id > enc_max_id) {
> >>if ((new = mallocarray(id + 1, sizeof(struct ifnet *),
> >>M_DEVBUF, M_NOWAIT|M_ZERO)) == NULL)
> >>return (ENOBUFS);
> >
> > This pattern is also used in enc_clone_create(), even if right now we
> > can't call this function twice for enc0 I think the code should be made
> > consistent.  ok?
> 
> Better send the correct diff, thanks Denis for the heads-up.
> 
> 
> Index: net/if_enc.c
> ===
> --- net/if_enc.c.orig
> +++ net/if_enc.c
> @@ -120,7 +120,7 @@ enc_clone_create(struct if_clone *ifc, i
>   return (error);
>   }
>  
> - if (unit == 0 || unit > enc_max_unit) {
> + if (enc_allifps == NULL || unit > enc_max_unit) {
>   if ((new = mallocarray(unit + 1, sizeof(struct ifnet *),
>   M_DEVBUF, M_NOWAIT|M_ZERO)) == NULL) {
>   NET_UNLOCK();
> 
> 
> -- 
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE
> 



Re: systat(1) iostat vs. iostat(1)

2018-07-08 Thread Jason McIntyre
On Sat, Jul 07, 2018 at 08:28:52AM +0200, Marcus MERIGHI wrote:
> Hello, 
> 
> when I run 'iostat(1) sd1 1' I get the transfer speed in MB/s. Running
> 'systat(1) iostat 1' in parallel I get the transfer speed in bytes, as
> calculations show. Therefore adjust the manual of systat(1): 
> kilobytes -> bytes.
> 
> Marcus
> 

fixed, thanks.
jmc

> Index: systat.1
> ===
> RCS file: /cvs/src/usr.bin/systat/systat.1,v
> retrieving revision 1.106
> diff -u -p -u -r1.106 systat.1
> --- systat.1  30 May 2018 13:53:09 -  1.106
> +++ systat.1  7 Jul 2018 06:20:03 -
> @@ -293,7 +293,7 @@ this is the default.
>  .It Ic iostat
>  Display statistics about disk throughput.
>  Statistics
> -on disk throughput show, for each drive, data transferred in kilobytes,
> +on disk throughput show, for each drive, data transferred in bytes,
>  number of disk transactions performed, and time spent in disk accesses
>  (in fractions of a second).
>  .It Ic malloc
> 



if_enc.c: "rdomain" is more descriptive than "id"

2018-07-08 Thread Jeremie Courreges-Anglas


Maybe not very important, but "id" sounds a tad too generic to me, and
it looks like we're really dealing with an rdomain here, so here's a diff.

Thoughts?  ok?


Index: net/if_enc.c
===
RCS file: /cvs/src/sys/net/if_enc.c,v
retrieving revision 1.72
diff -u -p -r1.72 if_enc.c
--- net/if_enc.c8 Jul 2018 13:04:04 -   1.72
+++ net/if_enc.c8 Jul 2018 13:20:13 -
@@ -36,7 +36,7 @@
 #endif
 
 struct ifnet   **enc_ifps; /* rdomain-mapped enc ifs */
-u_intenc_max_id;
+u_intenc_max_rdomain;
 struct ifnet   **enc_allifps;  /* unit-mapped enc ifs */
 u_intenc_max_unit;
 #define ENC_MAX_UNITS4096  /* XXX n per rdomain */
@@ -210,7 +210,7 @@ enc_ioctl(struct ifnet *ifp, u_long cmd,
 }
 
 struct ifnet *
-enc_getif(u_int id, u_int unit)
+enc_getif(u_int rdomain, u_int unit)
 {
struct ifnet*ifp;
 
@@ -221,7 +221,7 @@ enc_getif(u_int id, u_int unit)
if (unit > enc_max_unit)
return (NULL);
ifp = enc_allifps[unit];
-   if (ifp == NULL || ifp->if_rdomain != id)
+   if (ifp == NULL || ifp->if_rdomain != rdomain)
return (NULL);
return (ifp);
}
@@ -229,20 +229,20 @@ enc_getif(u_int id, u_int unit)
/* Otherwise return the default enc interface for this rdomain */
if (enc_ifps == NULL)
return (NULL);
-   else if (id > RT_TABLEID_MAX)
+   else if (rdomain > RT_TABLEID_MAX)
return (NULL);
-   else if (id > enc_max_id)
+   else if (rdomain > enc_max_rdomain)
return (NULL);
-   return (enc_ifps[id]);
+   return (enc_ifps[rdomain]);
 }
 
 struct ifaddr *
-enc_getifa(u_int id, u_int unit)
+enc_getifa(u_int rdomain, u_int unit)
 {
struct ifnet*ifp;
struct enc_softc*sc;
 
-   ifp = enc_getif(id, unit);
+   ifp = enc_getif(rdomain, unit);
if (ifp == NULL)
return (NULL);
 
@@ -250,7 +250,7 @@ enc_getifa(u_int id, u_int unit)
return (>sc_ifa);
 }
 int
-enc_setif(struct ifnet *ifp, u_int id)
+enc_setif(struct ifnet *ifp, u_int rdomain)
 {
struct ifnet**new;
size_t   newlen;
@@ -265,28 +265,28 @@ enc_setif(struct ifnet *ifp, u_int id)
 * for this rdomain, so only the first enc interface that
 * was added for this rdomain becomes the default.
 */
-   if (enc_getif(id, 0) != NULL)
+   if (enc_getif(rdomain, 0) != NULL)
return (0);
 
-   if (id > RT_TABLEID_MAX)
+   if (rdomain > RT_TABLEID_MAX)
return (EINVAL);
 
-   if (enc_ifps == NULL || id > enc_max_id) {
-   if ((new = mallocarray(id + 1, sizeof(struct ifnet *),
+   if (enc_ifps == NULL || rdomain > enc_max_rdomain) {
+   if ((new = mallocarray(rdomain + 1, sizeof(struct ifnet *),
M_DEVBUF, M_NOWAIT|M_ZERO)) == NULL)
return (ENOBUFS);
-   newlen = sizeof(struct ifnet *) * (id + 1);
+   newlen = sizeof(struct ifnet *) * (rdomain + 1);
 
if (enc_ifps != NULL) {
memcpy(new, enc_ifps,
-   sizeof(struct ifnet *) * (enc_max_id + 1));
+   sizeof(struct ifnet *) * (enc_max_rdomain + 1));
free(enc_ifps, M_DEVBUF, 0);
}
enc_ifps = new;
-   enc_max_id = id;
+   enc_max_rdomain = rdomain;
}
 
-   enc_ifps[id] = ifp;
+   enc_ifps[rdomain] = ifp;
 
/* Indicate that this interface is the rdomain default */
ifp->if_link_state = LINK_STATE_UP;
@@ -297,14 +297,14 @@ enc_setif(struct ifnet *ifp, u_int id)
 void
 enc_unsetif(struct ifnet *ifp)
 {
-   u_intid = ifp->if_rdomain, i;
+   u_intrdomain = ifp->if_rdomain, i;
struct ifnet*oifp, *nifp;
 
-   if ((oifp = enc_getif(id, 0)) == NULL || oifp != ifp)
+   if ((oifp = enc_getif(rdomain, 0)) == NULL || oifp != ifp)
return;
 
/* Clear slot for this rdomain */
-   enc_ifps[id] = NULL;
+   enc_ifps[rdomain] = NULL;
ifp->if_link_state = LINK_STATE_UNKNOWN;
 
/*
@@ -314,10 +314,10 @@ enc_unsetif(struct ifnet *ifp)
for (i = 0; i < (enc_max_unit + 1); i++) {
nifp = enc_allifps[i];
 
-   if (nifp == NULL || nifp == ifp || nifp->if_rdomain != id)
+   if (nifp == NULL || nifp == ifp || nifp->if_rdomain != rdomain)
continue;
 
-   enc_ifps[id] = nifp;
+   enc_ifps[rdomain] = nifp;
nifp->if_link_state = LINK_STATE_UP;
   

xenodm: source Xsetup before initializing the greeter

2018-07-08 Thread Matthieu Herrb
Hi,

prompted by a question by weerd@ who wanted to be able to rotate the
screen of his gpd win *before* the Xenodm greeter is displayed, I came
up with this (simple) patch that sources the Xsetup_0 script before
initializing the greeter widget.

I can't think of any down sides of doing this, and it may even help
with some further pledges or privilege separation for the greeter.

ok ?

Index: greeter/greet.c
===
RCS file: /cvs/OpenBSD/xenocara/app/xenodm/greeter/greet.c,v
retrieving revision 1.5
diff -u -r1.5 greet.c
--- greeter/greet.c 6 May 2018 15:25:27 -   1.5
+++ greeter/greet.c 30 Jun 2018 08:51:44 -
@@ -301,13 +301,13 @@
 Argarglist[2];
 Display*dpy;
 
-dpy = InitGreet (d);
 /*
  * Run the setup script - note this usually will not work when
  * the server is grabbed, so we don't even bother trying.
  */
 if (!d->grabServer)
SetupDisplay (d);
+dpy = InitGreet (d);
 if (!dpy) {
LogError ("Cannot reopen display %s for greet window\n", d->name);
exit (RESERVER_DISPLAY);

-- 
Matthieu Herrb



netstart(8): check for root privileges

2018-07-08 Thread Theo Buehler
When running netstart as regular user, the output looks something like
this:

$ sh /etc/netstart iwn0
/etc/netstart[226]: /etc/soii.key: cannot open $(<) input
sysctl: net.inet6.ip6.soiikey: Operation not permitted
ifconfig: SIOCS80211NWID: Operation not permitted
ifconfig: SIOCS80211WPAPSK: Operation not permitted
etc...

There was a privilege check present for a while (added by jasper in
r1.170), but backed out by rpe in r1.181, as it turned out to break
diskless setups, as /usr/bin/id might not be present during early boot.
Diff below adds it back while checking first that id(1) is there.

Index: netstart
===
RCS file: /var/cvs/src/etc/netstart,v
retrieving revision 1.198
diff -u -p -r1.198 netstart
--- netstart28 Apr 2018 22:38:32 -  1.198
+++ netstart8 Jul 2018 16:48:01 -
@@ -201,6 +201,13 @@ defaultroute() {
done
 }
 
+# Make sure the invoking user has the right privileges.  Check for presence of
+# id(1) to avoid problems with diskless setups.
+if [[ -x /usr/bin/id ]] && (($(id -u) != 0)); then
+   echo "${0##*/}: need root privileges"
+   exit 1
+fi
+
 # Get network related vars from rc.conf using the parsing routine from rc.subr.
 FUNCS_ONLY=1 . /etc/rc.d/rc.subr
 _rc_parse_conf



Re: [PATCH] column(1): -r to right justify

2018-07-08 Thread Ingo Schwarze
An afterthought...

Ingo Schwarze wrote on Sun, Jul 08, 2018 at 08:22:34PM +0200:

> If you opt for requiring an argument,
> then you can use a cut(1)-like syntax:

Alternatively, you can use a syntax similar to tbl(7) "Layout" lines:

  rrll

which allows later extensions like

  r|rclnl

That's much more powerful, not harder to understand for the user,
and probably also easier to implement.

Yours,
  Ingo



disable seemingly unsupported MSI for AMD 17h/Raven Ridge HD Audio in azalia.c

2018-07-08 Thread Thomas Frohwein
Hi,

It appears that HD Audio from AMD's generation Ryzen can't handle MSI.
This leads to the bug that I reported here:

https://marc.info/?l=openbsd-bugs=151648196215922=2

Disabling MSI resolves the problem on my current system which is a Raven
Ridge APU. I don't have the Summit Ridge hardware anymore to test that
it is also resolved there, but the line is included in the diff
(PCI_PRODUCT_AMD_AMD64_17_HDA). It seems likely that this diff will also
fix HD Audio on Summit Ridge. However, testing would be welcome by
anyone who has a first-gen Ryzen.

I was slightly confused by the fact that so far it seems I've been the
only one who reported this. Even searching online for such an issue in
other OS didn't yield anything. However, the issue was there between 2
different Ryzen CPUs, 3 different motherboards, and at least 2 separate
OpenBSD -current installations. And it was never there on any
Intel-based setup, with otherwise same hardware and OpenBSD install.
While there have been several reports of people using Ryzen with
OpenBSD, they may not have used audio (that's my explanation for this
at the moment).

This diff was collaborative work with brynet@.

ok?

Index: azalia.c
===
RCS file: /cvs/src/sys/dev/pci/azalia.c,v
retrieving revision 1.244
diff -u -p -r1.244 azalia.c
--- azalia.c22 Apr 2018 10:02:13 -  1.244
+++ azalia.c7 Jul 2018 18:26:20 -
@@ -517,6 +517,15 @@ azalia_pci_attach(struct device *parent,
azalia_pci_write(sc->pc, sc->tag, ICH_PCI_MMC, reg);
}
 
+   /* disable MSI for AMD Summit Ridge/Raven Ridge HD Audio */
+   if (PCI_VENDOR(sc->pciid) == PCI_VENDOR_AMD) {
+   switch (PCI_PRODUCT(sc->pciid)) {
+   case PCI_PRODUCT_AMD_AMD64_17_HDA:
+   case PCI_PRODUCT_AMD_RAVENRIDGE_HDA:
+   pa->pa_flags &= ~PCI_FLAGS_MSI_ENABLED;
+   }
+   }
+
/* interrupt */
if (pci_intr_map_msi(pa, ) && pci_intr_map(pa, )) {
printf(": can't map interrupt\n");



Re: usbdevs & hub ports status

2018-07-08 Thread Martin Pieuchot
On 04/07/18(Wed) 17:00, Martin Pieuchot wrote:
> Diff below adds support for printing USB ports status.  It includes an
> ABI change as we currently do not export port status/change to userland.
> 
> I'd really like to export the current cached value to userland via the
> USB_DEVICEINFO ioctl(2) to reduce the numbers of I/O.
> 
> Note that this diff also fixes some link-status defines in usb.h
> 
> Since it includes an ABI break I cranked devel/libusb1.
> 
>   $ usbdevs -dvv 
>   Controller /dev/usb0:
>   addr 01: 8086: Intel, xHCI root hub
>super speed, self powered, config 1, rev 1.00
>driver: uhub0
>port 01: .02a0 power Rx.detect
>port 02: .02a0 power Rx.detect
>port 03: .02a0 power Rx.detect
>port 04: .0503 connect enabled recovery
>port 05: .02a0 power Rx.detect
>   ...
>   addr 02: 04f2:b45d Chicony Electronics Co.,Ltd., Integrated Camera
>high speed, power 500 mA, config 1, rev 0.29, iSerialNumber 0x0001
>driver: uvideo0
> 
> ok?

Anyone?  Comments?  Oks?
 
> Index: src/usr.sbin/usbdevs/usbdevs.c
> ===
> RCS file: /cvs/src/usr.sbin/usbdevs/usbdevs.c,v
> retrieving revision 1.27
> diff -u -p -r1.27 usbdevs.c
> --- src/usr.sbin/usbdevs/usbdevs.c3 Jul 2018 13:21:31 -   1.27
> +++ src/usr.sbin/usbdevs/usbdevs.c4 Jul 2018 14:46:08 -
> @@ -54,7 +54,7 @@ int verbose = 0;
>  int showdevs = 0;
>  
>  void usage(void);
> -void usbdev(int f, uint8_t, int rec);
> +void usbdev(int f, uint8_t);
>  void usbdump(int f);
>  void dumpone(char *name, int f, int addr);
>  int main(int, char **);
> @@ -69,13 +69,13 @@ usage(void)
>  }
>  
>  char done[USB_MAX_DEVICES];
> -int indent;
>  
>  void
> -usbdev(int f, uint8_t addr, int rec)
> +usbdev(int f, uint8_t addr)
>  {
>   struct usb_device_info di;
> - int e, p, i, s, nports;
> + int e, i, port, nports;
> + uint16_t change, status;
>  
>   di.udi_addr = addr;
>   e = ioctl(f, USB_DEVICEINFO, );
> @@ -89,6 +89,7 @@ usbdev(int f, uint8_t addr, int rec)
>   done[addr] = 1;
>   printf("%04x:%04x %s, %s", di.udi_vendorNo, di.udi_productNo,
>   di.udi_vendor, di.udi_product);
> +
>   if (verbose) {
>   printf("\n\t ");
>   switch (di.udi_speed) {
> @@ -122,48 +123,96 @@ usbdev(int f, uint8_t addr, int rec)
>   printf(", iSerialNumber %s", di.udi_serial);
>   }
>   printf("\n");
> +
>   if (showdevs) {
>   for (i = 0; i < USB_MAX_DEVNAMES; i++)
>   if (di.udi_devnames[i][0])
> - printf("%*s  %s\n", indent, "",
> - di.udi_devnames[i]);
> + printf("\t driver: %s\n", di.udi_devnames[i]);
>   }
> - if (!rec)
> - return;
>  
> - nports = MINIMUM(di.udi_nports, nitems(di.udi_ports));
>   if (verbose > 1) {
> - for (p = 0; p < nports; p++) {
> - s = di.udi_ports[p];
> - printf("\t port %02u:", p+1);
> - if (s < USB_MAX_DEVICES)
> - printf(" addr %02u\n", s);
> - else {
> - printf(" %s\n",
> - s == USB_PORT_ENABLED ? "enabled" :
> - s == USB_PORT_SUSPENDED ? "suspended" :
> - s == USB_PORT_POWERED ? "powered" :
> - s == USB_PORT_DISABLED ? "disabled" :
> - "???");
> + nports = MINIMUM(di.udi_nports, nitems(di.udi_ports));
> + for (port = 0; port < nports; port++) {
> + status = di.udi_ports[port] & 0x;
> + change = di.udi_ports[port] >> 16;
> + printf("\t port %02u: %04x.%04x", port+1, change,
> + status);
> +
> + if (status & UPS_CURRENT_CONNECT_STATUS)
> + printf(" connect");
> +
> + if (status & UPS_PORT_ENABLED)
> + printf(" enabled");
> +
> + if (status & UPS_SUSPEND)
> + printf(" supsend");
> +
> + if (status & UPS_OVERCURRENT_INDICATOR)
> + printf(" overcurrent");
> +
> + if (di.udi_speed < USB_SPEED_SUPER) {
> + if (status & UPS_PORT_L1)
> + printf(" l1");
> +
> + if (status & UPS_PORT_POWER)
> + printf(" power");
> + } else {
> + if (status & UPS_PORT_POWER_SS)
> + printf(" power");
> +

Patch for column number in doas error messages

2018-07-08 Thread Phil Eaton
Hey,

Doas currently tells you the line but not the column for syntax errors. In
the case of a missing newline at the end of a line I was confused. So I
added the column number to the message as well.

Also, is there any interest in relaxing the grammar so a trailing rule
without a newline is ok?

Let me know what you think.

diff --git parse.y parse.y
index fde406bcf5a..f98deb81706 100644
--- parse.y
+++ parse.y
@@ -195,7 +195,7 @@ yyerror(const char *fmt, ...)
  va_start(va, fmt);
  vfprintf(stderr, fmt, va);
  va_end(va);
- fprintf(stderr, " at line %d\n", yylval.lineno + 1);
+ fprintf(stderr, " at line %d, column %d\n", yylval.lineno + 1,
yylval.colno);
  parse_errors++;
 }



-- 
Phil Eaton


Re: disable seemingly unsupported MSI for AMD 17h/Raven Ridge HD Audio in azalia.c

2018-07-08 Thread Jonathan Gray
On Sun, Jul 08, 2018 at 12:15:39PM -0700, Thomas Frohwein wrote:
> Hi,
> 
> It appears that HD Audio from AMD's generation Ryzen can't handle MSI.
> This leads to the bug that I reported here:
> 
> https://marc.info/?l=openbsd-bugs=151648196215922=2
> 
> Disabling MSI resolves the problem on my current system which is a Raven
> Ridge APU. I don't have the Summit Ridge hardware anymore to test that
> it is also resolved there, but the line is included in the diff
> (PCI_PRODUCT_AMD_AMD64_17_HDA). It seems likely that this diff will also
> fix HD Audio on Summit Ridge. However, testing would be welcome by
> anyone who has a first-gen Ryzen.
> 
> I was slightly confused by the fact that so far it seems I've been the
> only one who reported this. Even searching online for such an issue in
> other OS didn't yield anything. However, the issue was there between 2
> different Ryzen CPUs, 3 different motherboards, and at least 2 separate
> OpenBSD -current installations. And it was never there on any
> Intel-based setup, with otherwise same hardware and OpenBSD install.
> While there have been several reports of people using Ryzen with
> OpenBSD, they may not have used audio (that's my explanation for this
> at the moment).
> 
> This diff was collaborative work with brynet@.
> 
> ok?

Does following the HUDSON2_HDA case and enabling pcie snooping
instead change anything?

> 
> Index: azalia.c
> ===
> RCS file: /cvs/src/sys/dev/pci/azalia.c,v
> retrieving revision 1.244
> diff -u -p -r1.244 azalia.c
> --- azalia.c  22 Apr 2018 10:02:13 -  1.244
> +++ azalia.c  7 Jul 2018 18:26:20 -
> @@ -517,6 +517,15 @@ azalia_pci_attach(struct device *parent,
>   azalia_pci_write(sc->pc, sc->tag, ICH_PCI_MMC, reg);
>   }
>  
> + /* disable MSI for AMD Summit Ridge/Raven Ridge HD Audio */
> + if (PCI_VENDOR(sc->pciid) == PCI_VENDOR_AMD) {
> + switch (PCI_PRODUCT(sc->pciid)) {
> + case PCI_PRODUCT_AMD_AMD64_17_HDA:
> + case PCI_PRODUCT_AMD_RAVENRIDGE_HDA:
> + pa->pa_flags &= ~PCI_FLAGS_MSI_ENABLED;
> + }
> + }
> +
>   /* interrupt */
>   if (pci_intr_map_msi(pa, ) && pci_intr_map(pa, )) {
>   printf(": can't map interrupt\n");
> 



broadcom netxtreme-c/e driver

2018-07-08 Thread Jonathan Matthew
This started out as a port of freebsd's bnxt driver, but along the way I
had to rewrite the interesting bits.

NetXtreme-C/E (BCM573xx and BCM574xx) is a different line of chips to
NetXtreme II 10G (BCM577xx, which mje was working on), and luckily for me it's
much easier to talk to.  The reason these ones are interesting to me is that
they're the only 10G mezzanine option for Dell's Epyc based servers.

I've got it to the point where I could commit to cvs over it, so I'd like to
get it into the tree.  Lots of stuff is still missing - media types, multicast,
jumbos, any of the vast array of offloads, and it's not particularly fast yet,
only slightly over a gigabit in iperf/tcpbench type tests.  Lots of work to do
still.

The current code is below, only I'm not including bnxtreg.h as it's fairly
large (>30k lines) and it came unmodified from freebsd:
https://svnweb.freebsd.org/base/head/sys/dev/bnxt/hsi_struct_def.h?revision=323233=markup

ok?


/*  $OpenBSD$   */
/*-
 * Broadcom NetXtreme-C/E network driver.
 *
 * Copyright (c) 2016 Broadcom, All Rights Reserved.
 * The term Broadcom refers to Broadcom Limited and/or its subsidiaries
 *
 * Redistribution and use in source and binary forms, with or without
 * modification, are permitted provided that the following conditions
 * are met:
 * 1. Redistributions of source code must retain the above copyright
 *notice, this list of conditions and the following disclaimer.
 * 2. Redistributions in binary form must reproduce the above copyright
 *notice, this list of conditions and the following disclaimer in the
 *documentation and/or other materials provided with the distribution.
 *
 * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS'
 * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
 * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
 * ARE DISCLAIMED.  IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS
 * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
 * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
 * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
 * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
 * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
 * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
 * THE POSSIBILITY OF SUCH DAMAGE.
 */

/*
 * Copyright (c) 2018 Jonathan Matthew 
 *
 * 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 "bpfilter.h"

#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 

#include 

#include 
#include 
#include 

#define __FBSDID(x)
#include 

#include 
#include 

#if NBPFILTER > 0
#include 
#endif

#include 
#include 

#define BNXT_HWRM_BAR   0x10
#define BNXT_DOORBELL_BAR   0x18

#define BNXT_RX_RING_ID 0
#define BNXT_AG_RING_ID 1
#define BNXT_TX_RING_ID 3

#define BNXT_MAX_QUEUE  8
#define BNXT_MAX_MTU9000

#define BNXT_MAX_TX_SEGS32  /* a bit much? */

#define BNXT_HWRM_SHORT_REQ_LEN sizeof(struct hwrm_short_input)

#define BNXT_HWRM_LOCK_INIT(_sc, _name) \
mtx_init_flags(>sc_lock, IPL_NET, _name, 0)
#define BNXT_HWRM_LOCK(_sc) mtx_enter(&_sc->sc_lock)
#define BNXT_HWRM_UNLOCK(_sc)   mtx_leave(&_sc->sc_lock)
#define BNXT_HWRM_LOCK_DESTROY(_sc) /* nothing */
#define BNXT_HWRM_LOCK_ASSERT(_sc)  MUTEX_ASSERT_LOCKED(&_sc->sc_lock)

#define BNXT_FLAG_VF0x0001
#define BNXT_FLAG_NPAR  0x0002
#define BNXT_FLAG_WOL_CAP   0x0004
#define BNXT_FLAG_SHORT_CMD 0x0008

#define NEXT_CP_CONS_V(_ring, _cons, _v_bit)\
do {\
if (++(_cons) == (_ring)->ring_size)\
((_cons) = 0, (_v_bit) = !_v_bit);  \
} while (0);

struct bnxt_cos_queue {
uint8_t id;
uint8_t profile;
};

struct bnxt_ring {
uint64_tpaddr;
uint64_tdoorbell;
caddr_t vaddr;
uint32_tring_size;
uint16_tid;
uint16_t

ber.c: simplify ber_read()

2018-07-08 Thread Jeremie Courreges-Anglas


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 -   1.11
+++ usr.bin/ldap/ber.c  6 Jul 2018 11:50:24 -
@@ -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_MAX30
 #define BER_TAG_MASK   0x1f
@@ -48,7 +46,6 @@ static ssize_tget_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.c4 Jul 2018 15:21:24 -   1.21
+++ usr.sbin/ldapd/ber.c6 Jul 2018 11:47:55 -
@@ -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_MAX30
 #define BER_TAG_MASK   0x1f
@@ -48,7 +46,6 @@ static ssize_tget_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;
 }