move more code under UAUDIO_DEBUG

[Michael W. Bombardieri]

Hello,

In uaudio_add_mixer() the number of input channels (ichs) is
only used in a DPRINTF statement so the code to set its value
can be excluded unless we're building a debug kernel. In my
understanding uaudio_get_cluster_nchan() has no side effects.

- Michael


Index: uaudio.c
===
RCS file: /cvs/src/sys/dev/usb/uaudio.c,v
retrieving revision 1.132
diff -u -p -u -r1.132 uaudio.c
--- uaudio.c8 Aug 2018 14:25:50 -   1.132
+++ uaudio.c20 Aug 2018 09:16:45 -
@@ -750,17 +750,21 @@ uaudio_add_mixer(struct uaudio_softc *sc
 {
const struct usb_audio_mixer_unit *d = iot[id].d.mu;
struct usb_audio_mixer_unit_1 *d1;
-   int c, chs, ichs, ochs, i, o, bno, p, mo, mc, k;
+   int c, chs, ochs, i, o, bno, p, mo, mc, k;
+#ifdef UAUDIO_DEBUG
+   int ichs = 0;
+#endif
uByte *bm;
struct mixerctl mix;
 
DPRINTFN(2,("%s: bUnitId=%d bNrInPins=%d\n", __func__,
d->bUnitId, d->bNrInPins));
 
+#ifdef UAUDIO_DEBUG
/* Compute the number of input channels */
-   ichs = 0;
for (i = 0; i < d->bNrInPins; i++)
ichs += uaudio_get_cluster_nchan(d->baSourceId[i], iot);
+#endif
 
/* and the number of output channels */
d1 = (struct usb_audio_mixer_unit_1 *)>baSourceId[d->bNrInPins];

audio round_buffersize

[Michael W. Bombardieri]

Hello,

Low level audio drivers using the upper layer audio driver's value
for buffer size don't need to declare a round_buffersize method.
The 3rd parameter of the method is the value provided by the
upper layer driver which is used if round_buffersize isn't defined:
https://github.com/openbsd/src/blob/master/sys/dev/audio.c#L203

- Michael


Index: auixp.c
===
RCS file: /cvs/src/sys/dev/pci/auixp.c,v
retrieving revision 1.39
diff -u -p -u -r1.39 auixp.c
--- auixp.c 8 Sep 2017 05:36:52 -   1.39
+++ auixp.c 14 Aug 2018 06:13:43 -
@@ -124,7 +124,6 @@ int auixp_get_port(void *, mixer_ctrl_t 
 intauixp_query_devinfo(void *, mixer_devinfo_t *);
 void * auixp_malloc(void *, int, size_t, int, int);
 void   auixp_free(void *, void *, int);
-size_t auixp_round_buffersize(void *, int, size_t);
 intauixp_get_props(void *);
 intauixp_intr(void *);
 intauixp_allocmem(struct auixp_softc *, size_t, size_t,
@@ -181,7 +180,7 @@ struct audio_hw_if auixp_hw_if = {
auixp_query_devinfo,
auixp_malloc,
auixp_free,
-   auixp_round_buffersize,
+   NULL,   /* round_buffersize */
auixp_get_props,
auixp_trigger_output,
auixp_trigger_input
@@ -452,16 +451,6 @@ auixp_query_devinfo(void *hdl, mixer_dev
co = (struct auixp_codec *) hdl;
return co->codec_if->vtbl->query_devinfo(co->codec_if, di);
 }
-
-
-size_t
-auixp_round_buffersize(void *hdl, int direction, size_t bufsize)
-{
-
-   /* XXX force maximum? i.e. 256 kb? */
-   return bufsize;
-}
-
 
 int
 auixp_get_props(void *hdl)
Index: cs4280.c
===
RCS file: /cvs/src/sys/dev/pci/cs4280.c,v
retrieving revision 1.51
diff -u -p -u -r1.51 cs4280.c
--- cs4280.c26 Dec 2016 17:38:14 -  1.51
+++ cs4280.c14 Aug 2018 06:13:43 -
@@ -206,7 +206,6 @@ int cs4280_mixer_get_port(void *, mixer_
 intcs4280_query_devinfo(void *addr, mixer_devinfo_t *dip);
 void   *cs4280_malloc(void *, int, size_t, int, int);
 void   cs4280_free(void *, void *, int);
-size_t cs4280_round_buffersize(void *, int, size_t);
 intcs4280_get_props(void *);
 intcs4280_trigger_output(void *, void *, void *, int, void (*)(void *),
void *, struct audio_params *);
@@ -253,7 +252,7 @@ struct audio_hw_if cs4280_hw_if = {
cs4280_query_devinfo,
cs4280_malloc,
cs4280_free,
-   cs4280_round_buffersize,
+   NULL,
cs4280_get_props,
cs4280_trigger_output,
cs4280_trigger_input
@@ -1058,16 +1057,6 @@ int
 cs4280_round_blocksize(void *hdl, int blk)
 {
return (blk < CS4280_ICHUNK ? CS4280_ICHUNK : blk & -CS4280_ICHUNK);
-}
-
-size_t
-cs4280_round_buffersize(void *addr, int direction, size_t size)
-{
-   /* although real dma buffer size is 4KB, 
-* let the audio.c driver use a larger buffer.
-* ( suggested by Lennart Augustsson. )
-*/
-   return (size);
 }
 
 int
Index: envy.c
===
RCS file: /cvs/src/sys/dev/pci/envy.c,v
retrieving revision 1.72
diff -u -p -u -r1.72 envy.c
--- envy.c  17 Mar 2018 13:35:12 -  1.72
+++ envy.c  14 Aug 2018 06:13:43 -
@@ -102,7 +102,6 @@ void envy_freem(void *, void *, int);
 int envy_set_params(void *, int, int, struct audio_params *,
 struct audio_params *);
 int envy_round_blocksize(void *, int);
-size_t envy_round_buffersize(void *, int, size_t);
 int envy_trigger_output(void *, void *, void *, int,
 void (*)(void *), void *, struct audio_params *);
 int envy_trigger_input(void *, void *, void *, int,
@@ -196,7 +195,7 @@ struct audio_hw_if envy_hw_if = {
envy_query_devinfo, /* query_devinfo */
envy_allocm,/* malloc */
envy_freem, /* free */
-   envy_round_buffersize,  /* round_buffersize */
+   NULL,   /* round_buffersize */
envy_get_props, /* get_props */
envy_trigger_output,/* trigger_output */
envy_trigger_input  /* trigger_input */
@@ -1885,12 +1884,6 @@ int
 envy_round_blocksize(void *self, int blksz)
 {
return (blksz + 0x1f) & ~0x1f;
-}
-
-size_t
-envy_round_buffersize(void *self, int dir, size_t bufsz)
-{
-   return bufsz;
 }
 
 #ifdef ENVY_DEBUG

uaudio(4) division by zero patch

[Michael W. Bombardieri]

Hello,

I adapted the following patch from netbsd to prevent a division by
zero in the uaudio driver.

 
http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/dev/usb/uaudio.c.diff?r1=1.142=1.143=h

I haven't encountered this problem with my own device but possibly
it can be triggered by some device or by a usb fuzzer.

- Michael


Index: uaudio.c
===
RCS file: /cvs/src/sys/dev/usb/uaudio.c,v
retrieving revision 1.131
diff -u -p -u -r1.131 uaudio.c
--- uaudio.c30 Jul 2018 11:51:42 -  1.131
+++ uaudio.c7 Aug 2018 09:51:47 -
@@ -1069,15 +1069,19 @@ uaudio_add_feature(struct uaudio_softc *
const struct usb_audio_feature_unit *d = iot[id].d.fu;
uByte *ctls = (uByte *)d->bmaControls;
int ctlsize = d->bControlSize;
-   int nchan = (d->bLength - 7) / ctlsize;
u_int fumask, mmask, cmask;
struct mixerctl mix;
-   int chan, ctl, i, unit;
+   int chan, ctl, i, nchan, unit;
const char *mixername;
 
 #define GET(i) (ctls[(i)*ctlsize] | \
(ctlsize > 1 ? ctls[(i)*ctlsize+1] << 8 : 0))
 
+   if (ctlsize == 0) {
+   DPRINTF(("ignoring feature %d: bControlSize == 0\n", id));
+   return;
+   }
+   nchan = (d->bLength - 7) / ctlsize;
mmask = GET(0);
/* Figure out what we can control */
for (cmask = 0, chan = 1; chan < nchan; chan++) {

uaudioctl() shadow variable 'error'

[Michael W. Bombardieri]

Hello,

The function usbioctl() has an error variable and two more which
are declared in nested blocks, i.e. switch cases for USB_REQUEST
and USB_DEVICE_GET_FDESC. The following patch to remove the
nested declarations builds cleanly, and if I'm following things
correctly the function works the same without them. Sorry if I
got it wrong.

- Michael

Index: usb.c
===
RCS file: /cvs/src/sys/dev/usb/usb.c,v
retrieving revision 1.119
diff -u -p -u -r1.119 usb.c
--- usb.c   1 May 2018 18:14:46 -   1.119
+++ usb.c   31 Jul 2018 09:37:21 -
@@ -623,7 +623,6 @@ usbioctl(dev_t devt, u_long cmd, caddr_t
void *ptr = NULL;
int addr = ur->ucr_addr;
usbd_status err;
-   int error = 0;
 
if (!(flag & FWRITE))
return (EBADF);
@@ -777,7 +776,6 @@ usbioctl(dev_t devt, u_long cmd, caddr_t
struct iovec iov;
struct uio uio;
size_t len;
-   int error;
 
if (addr < 1 || addr >= USB_MAX_DEVICES)
return (EINVAL);

remove uaudio_id_name()

[Michael W. Bombardieri]

Hello,

The function uaudio_id_name() was used once for formatting a string
and its return value was used to format a second string. By rolling
the prepended letter "i" into the outer snprintf() the function can
be removed entirely. Does this look correct?

- Michael


Index: uaudio.c
===
RCS file: /cvs/src/sys/dev/usb/uaudio.c,v
retrieving revision 1.130
diff -u -p -u -r1.130 uaudio.c
--- uaudio.c27 Jul 2018 05:48:59 -  1.130
+++ uaudio.c29 Jul 2018 13:22:04 -
@@ -314,8 +314,6 @@ const usb_interface_descriptor_t *uaudio
(const char *, int, int *, int, int);
 
 void   uaudio_mixer_add_ctl(struct uaudio_softc *, struct mixerctl *);
-char   *uaudio_id_name
-   (struct uaudio_softc *, const struct io_terminal *, int);
 uByte  uaudio_get_cluster_nchan
(int, const struct io_terminal *);
 void   uaudio_add_input
@@ -672,14 +670,6 @@ uaudio_mixer_add_ctl(struct uaudio_softc
 #endif
 }
 
-char *
-uaudio_id_name(struct uaudio_softc *sc, const struct io_terminal *iot, int id)
-{
-   static char buf[32];
-   snprintf(buf, sizeof(buf), "i%d", id);
-   return (buf);
-}
-
 uByte
 uaudio_get_cluster_nchan(int id, const struct io_terminal *iot)
 {
@@ -805,9 +795,8 @@ uaudio_add_mixer(struct uaudio_softc *sc
mix.wValue[k++] =
MAKE(p+c+1, o+1);
}
-   snprintf(mix.ctlname, sizeof(mix.ctlname), "mix%d-%s",
-   d->bUnitId, uaudio_id_name(sc, iot,
-   d->baSourceId[i]));
+   snprintf(mix.ctlname, sizeof(mix.ctlname), "mix%d-i%d",
+   d->bUnitId, d->baSourceId[i]);
mix.nchan = chs;
uaudio_mixer_add_ctl(sc, );
} else {

uaudio: simplify free() usage

[Michael W. Bombardieri]

Hello,

A few NULL checks before free() can be removed in uaudio.
Immediately freeing the value returned by uaudio_io_terminaltype()
seems a little strange but I didn't look into that further.

- Michael


Index: uaudio.c
===
RCS file: /cvs/src/sys/dev/usb/uaudio.c,v
retrieving revision 1.129
diff -u -p -u -r1.129 uaudio.c
--- uaudio.c7 Jul 2018 13:03:08 -   1.129
+++ uaudio.c23 Jul 2018 06:29:46 -
@@ -1353,8 +1353,7 @@ uaudio_io_terminaltype(int outtype, stru
it->output = tml;
if (it->inputs != NULL) {
for (i = 0; i < it->inputs_size; i++)
-   if (it->inputs[i] != NULL)
-   free(it->inputs[i], M_TEMP, 0);
+   free(it->inputs[i], M_TEMP, 0);
free(it->inputs, M_TEMP, 0);
}
it->inputs_size = 0;
@@ -1874,8 +1873,7 @@ uaudio_identify_ac(struct uaudio_softc *
continue;
pot = iot[i].d.ot;
tml = uaudio_io_terminaltype(UGETW(pot->wTerminalType), iot, i);
-   if (tml != NULL)
-   free(tml, M_TEMP, 0);
+   free(tml, M_TEMP, 0);
}
 
 #ifdef UAUDIO_DEBUG
@@ -1988,14 +1986,11 @@ uaudio_identify_ac(struct uaudio_softc *
if (iot[i].d.desc == NULL)
continue;
if (iot[i].inputs != NULL) {
-   for (j = 0; j < iot[i].inputs_size; j++) {
-   if (iot[i].inputs[j] != NULL)
-   free(iot[i].inputs[j], M_TEMP, 0);
-   }
+   for (j = 0; j < iot[i].inputs_size; j++)
+   free(iot[i].inputs[j], M_TEMP, 0);
free(iot[i].inputs, M_TEMP, 0);
}
-   if (iot[i].output != NULL)
-   free(iot[i].output, M_TEMP, 0);
+   free(iot[i].output, M_TEMP, 0);
iot[i].d.desc = NULL;
}
free(iot, M_TEMP, 256 * sizeof(struct io_terminal));

xhci typo in comment

[Michael W. Bombardieri]

Hello,

When studying some usb code I noticed a couple of the comments
didn't read well. Does this look better?

- Michael


Index: xhci.c
===
RCS file: /cvs/src/sys/dev/usb/xhci.c,v
retrieving revision 1.86
diff -u -p -u -r1.86 xhci.c
--- xhci.c  13 May 2018 06:58:42 -  1.86
+++ xhci.c  16 Jul 2018 06:02:02 -
@@ -764,7 +764,7 @@ xhci_event_xfer(struct xhci_softc *sc, u
 * If this is not the last TRB of a transfer, we should
 * theoretically clear the IOC at the end of the chain
 * but the HC might have already processed it before we
-* had a change to schedule the softinterrupt.
+* had a chance to schedule the softinterrupt.
 */
xx = (struct xhci_xfer *)xfer;
if (xx->index != trb_idx) {
@@ -1043,7 +1043,7 @@ xhci_pipe_open(struct usbd_pipe *pipe)
 
/*
 * Our USBD Bus Interface is pipe-oriented but for most of the
-* operations we need to access a device context, so keep trace
+* operations we need to access a device context, so keep track
 * of the slot ID in every pipe.
 */
if (slot == 0)
@@ -1327,7 +1327,7 @@ xhci_pipe_init(struct xhci_softc *sc, st
 * be in the ENABLED state.  Issue an "Address Device"
 * with BSR=1 to put the device in the DEFAULT state.
 * We cannot jump directly to the ADDRESSED state with
-* BSR=0 because some Low/Full speed devices wont accept
+* BSR=0 because some Low/Full speed devices won't accept
 * a SET_ADDRESS command before we've read their device
 * descriptor.
 */
@@ -2047,7 +2047,7 @@ xhci_abort_xfer(struct usbd_xfer *xfer, 
 * At this stage the endpoint has been stopped, so update its
 * dequeue pointer past the last TRB of the transfer.
 *
-* Note: This assume that only one transfer per endpoint has
+* Note: This assumes that only one transfer per endpoint has
 *   pending TRBs on the ring.
 */
xhci_cmd_set_tr_deq_async(sc, xp->slot, xp->dci,

Re: eoip(4): MikroTik Ethernet over IP support

[Michael W. Bombardieri]

On Sun, May 27, 2018 at 10:18:30AM +1000, David Gwynne wrote:
> this implements MikroTiks Ethernet over IP protocol support.
> 
> The Mikrotik protocol is basically GRE, so this is implemented as
> eoip(4) as (yet another) part of if_gre. the main differences between
> egre and eoip is that eoip uses gre version 1 (not 0) and 0x6400
> as the protocol identifier (not transparent ethernet as per rfc
> 1701), mandates a key header, but splits the key into 16bit len and
> tunnel id fields (a bit like pptp).
> 
> the keepalive semantics are also different. keepalives are just 0
> length packets, and arent echoed. each side listens for the others
> packet to see if theyre up, but doesn't relay the keepalives. theyre
> more hellos maybe?
> 
> like egre though, it still misaligns the payload, but what doesnt
> these days?
> 
> this also tweaks tcpdump to better support the protocol.

Hello,

In gre_ioctl(), mgre_ioctl(), egre_ioctl() and eoip_ioctl() the
SIOCSIFFLAGS case appears to have code that explicitly sets error=0
but it looks like error is already zero unless I'm reading it wrong.
Also, nvgre_ioctl() is different because it seems to set error=ENETRESET
in that case (i.e. IFF_UP and IFF_RUNNING are both set). Is that
difference intended?

- Michael

Re: switchd: bzero -> memset

[Michael W. Bombardieri]

On Tue, Apr 03, 2018 at 10:20:42PM -0600, Theo de Raadt wrote:
> Michael W. Bombardieri <m...@ii.net> wrote:
> 
> > Hello,
> > 
> > switchd can just use memset instead of mixing memset with bzero.
> > But does the util.c change need to be sync'ed with other tools?
> 
> There are 418 calls to bzero in *bin/*/*c
> 
> bcopy isn't going to be removed from libc, and frankly it is
> diomatically simpler.
> 
> Would you believe in 15 years ago we have found calls where c and len
> were swapped, making it a no-op -- in security sensitive software?
Nice.
> 
> So I don't see the specific value in your proposal.

I agree, bzero is simpler and easier to grep for. But then grep'ing
is more complicated if you mix bzero and memset, so I guess it makes
sense to use one or the other at least within the same file.

switchd: bzero -> memset

[Michael W. Bombardieri]

Hello,

switchd can just use memset instead of mixing memset with bzero.
But does the util.c change need to be sync'ed with other tools?

- Michael


Index: ofp10.c
===
RCS file: /cvs/src/usr.sbin/switchd/ofp10.c,v
retrieving revision 1.19
diff -u -p -u -r1.19 ofp10.c
--- ofp10.c 2 Dec 2016 14:39:46 -   1.19
+++ ofp10.c 4 Apr 2018 04:00:19 -
@@ -342,7 +342,7 @@ ofp10_packet_match(struct packet *pkt, s
 {
struct ether_header *eh = pkt->pkt_eh;
 
-   bzero(m, sizeof(*m));
+   memset(m, 0, sizeof(*m));
m->m_wildcards = htonl(~flags);
 
if ((flags & (OFP10_WILDCARD_DL_SRC|OFP10_WILDCARD_DL_DST)) &&
@@ -377,7 +377,7 @@ ofp10_packet_in(struct switchd *sc, stru
if ((pin = ibuf_getdata(ibuf, sizeof(*pin))) == NULL)
return (-1);
 
-   bzero(, sizeof(pkt));
+   memset(, 0, sizeof(pkt));
len = ntohs(pin->pin_total_len);
srcport = ntohs(pin->pin_port);
 
Index: ofp13.c
===
RCS file: /cvs/src/usr.sbin/switchd/ofp13.c,v
retrieving revision 1.43
diff -u -p -u -r1.43 ofp13.c
--- ofp13.c 17 Jan 2017 09:21:50 -  1.43
+++ ofp13.c 4 Apr 2018 04:00:20 -
@@ -1029,7 +1029,7 @@ ofp13_packet_in(struct switchd *sc, stru
if (pin->pin_reason != OFP_PKTIN_REASON_NO_MATCH)
return (-1);
 
-   bzero(, sizeof(pkt));
+   memset(, 0, sizeof(pkt));
len = ntohs(pin->pin_total_len);
 
/* very basic way of getting the source port */
Index: switchd.c
===
RCS file: /cvs/src/usr.sbin/switchd/switchd.c,v
retrieving revision 1.15
diff -u -p -u -r1.15 switchd.c
--- switchd.c   9 Jan 2017 14:49:22 -   1.15
+++ switchd.c   4 Apr 2018 04:00:20 -
@@ -242,7 +242,7 @@ switchd_socket(struct sockaddr *sock, in
/*
 * Socket options
 */
-   bzero(, sizeof(lng));
+   memset(, 0, sizeof(lng));
if (setsockopt(s, SOL_SOCKET, SO_LINGER, , sizeof(lng)) == -1)
goto bad;
if (reuseport) {
Index: util.c
===
RCS file: /cvs/src/usr.sbin/switchd/util.c,v
retrieving revision 1.6
diff -u -p -u -r1.6 util.c
--- util.c  9 Jan 2017 14:49:22 -   1.6
+++ util.c  4 Apr 2018 04:00:20 -
@@ -196,7 +196,7 @@ prefixlen2mask6(uint8_t prefixlen, uint3
if (prefixlen > 128)
prefixlen = 128;
 
-   bzero(, sizeof(s6));
+   memset(, 0, sizeof(s6));
for (i = 0; i < prefixlen / 8; i++)
s6.s6_addr[i] = 0xff;
i = prefixlen % 8;
@@ -277,7 +277,7 @@ print_map(unsigned int type, struct cons
 
if (idx >= SWITCHD_CYCLE_BUFFERS)
idx = 0;
-   bzero(buf[idx], sizeof(buf[idx]));
+   memset(buf[idx], 0, sizeof(buf[idx]));
 
for (i = 0; map[i].cm_name != NULL; i++) {
if (map[i].cm_type == type) {

azalia(4) debug

[Michael W. Bombardieri]

Hello,

In azalia_set_params_sub() the cmode variable is only used by
DPRINTF() so we can avoid declaring it when debugging is not
enabled.

- Michael


Index: azalia.c
===
RCS file: /cvs/src/sys/dev/pci/azalia.c,v
retrieving revision 1.240
diff -u -p -u -r1.240 azalia.c
--- azalia.c10 Jan 2018 09:00:40 -  1.240
+++ azalia.c21 Mar 2018 04:51:01 -
@@ -3905,14 +3905,11 @@ azalia_match_format(codec_t *codec, int 
 int
 azalia_set_params_sub(codec_t *codec, int mode, audio_params_t *par)
 {
-   char *cmode;
int i, j;
uint ochan, oenc, opre;
-
-   if (mode == AUMODE_PLAY)
-   cmode = "play";
-   else
-   cmode = "record";
+#ifdef AZALIA_DEBUG
+   char *cmode = (mode == AUMODE_PLAY) ? "play" : "record";
+#endif
 
ochan = par->channels;
oenc = par->encoding;

ksh: __func__ in warnings

[Michael W. Bombardieri]

Hello,

Some errors and warnings printed by ksh have the function name
prefixed. __func__ could be used here instead of hard-coding
the name. The names are wrong for tty_init(), j_set_async(),
j_change(), x_file_glob() and c_ulimit() afaics.

- Michael


Index: c_ksh.c
===
RCS file: /cvs/src/bin/ksh/c_ksh.c,v
retrieving revision 1.58
diff -u -p -u -r1.58 c_ksh.c
--- c_ksh.c 16 Jan 2018 22:52:32 -  1.58
+++ c_ksh.c 13 Mar 2018 08:16:37 -
@@ -1273,7 +1273,7 @@ c_getopts(char **wp)
}
 
if (genv->loc->next == NULL) {
-   internal_warningf("c_getopts: no argv");
+   internal_warningf("%s: no argv", __func__);
return 1;
}
/* Which arguments are we parsing... */
Index: c_ulimit.c
===
RCS file: /cvs/src/bin/ksh/c_ulimit.c,v
retrieving revision 1.26
diff -u -p -u -r1.26 c_ulimit.c
--- c_ulimit.c  16 Jan 2018 22:52:32 -  1.26
+++ c_ulimit.c  13 Mar 2018 08:16:37 -
@@ -97,7 +97,7 @@ c_ulimit(char **wp)
for (l = limits; l->name && l->option != optc; l++)
;
if (!l->name) {
-   internal_warningf("ulimit: %c", optc);
+   internal_warningf("%s: %c", __func__, optc);
return 1;
}
if (builtin_opt.optarg) {
Index: edit.c
===
RCS file: /cvs/src/bin/ksh/edit.c,v
retrieving revision 1.63
diff -u -p -u -r1.63 edit.c
--- edit.c  16 Jan 2018 22:52:32 -  1.63
+++ edit.c  13 Mar 2018 08:16:37 -
@@ -372,7 +372,7 @@ x_file_glob(int flags, const char *str, 
source = s;
if (yylex(ONEWORD|UNESCAPE) != LWORD) {
source = sold;
-   internal_warningf("fileglob: substitute error");
+   internal_warningf("%s: substitute error", __func__);
return 0;
}
source = sold;
Index: exec.c
===
RCS file: /cvs/src/bin/ksh/exec.c,v
retrieving revision 1.72
diff -u -p -u -r1.72 exec.c
--- exec.c  16 Jan 2018 22:52:32 -  1.72
+++ exec.c  13 Mar 2018 08:16:37 -
@@ -727,7 +727,7 @@ shcomexec(char **wp)
 
tp = ktsearch(, *wp, hash(*wp));
if (tp == NULL)
-   internal_errorf("shcomexec: %s", *wp);
+   internal_errorf("%s: %s", __func__, *wp);
return call_builtin(tp, wp);
 }
 
@@ -1221,7 +1221,7 @@ herein(const char *content, int sub)
s->start = s->str = content;
source = s;
if (yylex(ONEWORD|HEREDOC) != LWORD)
-   internal_errorf("herein: yylex");
+   internal_errorf("%s: yylex", __func__);
source = osource;
shf_puts(evalstr(yylval.cp, 0), shf);
} else
@@ -1446,5 +1446,5 @@ static void
 dbteste_error(Test_env *te, int offset, const char *msg)
 {
te->flags |= TEF_ERROR;
-   internal_warningf("dbteste_error: %s (offset %d)", msg, offset);
+   internal_warningf("%s: %s (offset %d)", __func__, msg, offset);
 }
Index: jobs.c
===
RCS file: /cvs/src/bin/ksh/jobs.c,v
retrieving revision 1.59
diff -u -p -u -r1.59 jobs.c
--- jobs.c  16 Jan 2018 22:52:32 -  1.59
+++ jobs.c  13 Mar 2018 08:16:38 -
@@ -200,13 +200,13 @@ j_suspend(void)
if (restore_ttypgrp >= 0) {
if (tcsetpgrp(tty_fd, restore_ttypgrp) < 0) {
warningf(false,
-   "j_suspend: tcsetpgrp() failed: %s",
-   strerror(errno));
+   "%s: tcsetpgrp() failed: %s",
+   __func__, strerror(errno));
} else {
if (setpgid(0, restore_ttypgrp) < 0) {
warningf(false,
-   "j_suspend: setpgid() failed: %s",
-   strerror(errno));
+   "%s: setpgid() failed: %s",
+   __func__, strerror(errno));
}
}
}
@@ -225,14 +225,14 @@ j_suspend(void)
if (restore_ttypgrp >= 0) {
if (setpgid(0, kshpid) < 0) {
warningf(false,
-   "j_suspend: setpgid() failed: %s",
-   strerror(errno));
+   "%s: setpgid() 

remove goto in rdioctl()

[Michael W. Bombardieri]

Hello,

Function rdioctl() uses a goto to leave the switch statement
but a break would be enough. There is no other code to jump over
(avoid) as with rdstrategy() which has goto bad and goto done.

- Michael


Index: rd.c
===
RCS file: /cvs/src/sys/dev/rd.c,v
retrieving revision 1.13
diff -u -p -u -r1.13 rd.c
--- rd.c30 Dec 2017 23:08:29 -  1.13
+++ rd.c29 Jan 2018 03:52:56 -
@@ -266,31 +266,31 @@ rdioctl(dev_t dev, u_long cmd, caddr_t d
rdgetdisklabel(dev, sc, lp, 0);
memcpy(sc->sc_dk.dk_label, lp, sizeof(*lp));
free(lp, M_TEMP, sizeof(*lp));
-   goto done;
+   break;
 
case DIOCGPDINFO:
rdgetdisklabel(dev, sc, (struct disklabel *)data, 1);
-   goto done;
+   break;
 
case DIOCGDINFO:
*(struct disklabel *)data = *(sc->sc_dk.dk_label);
-   goto done;
+   break;
 
case DIOCGPART:
((struct partinfo *)data)->disklab = sc->sc_dk.dk_label;
((struct partinfo *)data)->part =
>sc_dk.dk_label->d_partitions[DISKPART(dev)];
-   goto done;
+   break;
 
case DIOCWDINFO:
case DIOCSDINFO:
if ((fflag & FWRITE) == 0) {
error = EBADF;
-   goto done;
+   break;
}
 
if ((error = disk_lock(>sc_dk)) != 0)
-   goto done;
+   break;
 
error = setdisklabel(sc->sc_dk.dk_label,
(struct disklabel *)data, sc->sc_dk.dk_openmask);
@@ -301,10 +301,9 @@ rdioctl(dev_t dev, u_long cmd, caddr_t d
}
 
disk_unlock(>sc_dk);
-   goto done;
+   break;
}
 
- done:
device_unref(>sc_dev);
return (error);
 }

csh: calloc->malloc for new directory pointer

[Michael W. Bombardieri]

Hello,

A fresh new 'struct directory' is allocated at three points in dir.c.
This struct has four members: di_name, di_count, di_next and di_prev.
For each instance all struct members are initialised immediately after
the calloc(), so the new memory doesn't need to be cleared first.

- Michael


Index: dir.c
===
RCS file: /cvs/src/bin/csh/dir.c,v
retrieving revision 1.21
diff -u -p -u -r1.21 dir.c
--- dir.c   26 Dec 2015 13:48:38 -  1.21
+++ dir.c   15 Jan 2018 07:21:18 -
@@ -116,7 +116,7 @@ dinit(Char *hp)
}
 }
 
-dp = xcalloc(1, sizeof(struct directory));
+dp = xmalloc(sizeof(struct directory));
 dp->di_name = Strsave(cp);
 dp->di_count = 0;
 dhead.di_next = dhead.di_prev = dp;
@@ -351,7 +351,7 @@ dochngd(Char **v, struct command *t)
 }
 else
cp = dfollow(*v);
-dp = xcalloc(1, sizeof(struct directory));
+dp = xmalloc(sizeof(struct directory));
 dp->di_name = cp;
 dp->di_count = 0;
 dp->di_next = dcwd->di_next;
@@ -502,7 +502,7 @@ dopushd(Char **v, struct command *t)
Char *ccp;
 
ccp = dfollow(*v);
-   dp = xcalloc(1, sizeof(struct directory));
+   dp = xmalloc(sizeof(struct directory));
dp->di_name = ccp;
dp->di_count = 0;
dp->di_prev = dcwd;

Re: disabled code in ksh tree.c

[Michael W. Bombardieri]

On Sun, Jan 14, 2018 at 05:47:43PM +0100, Jeremie Courreges-Anglas wrote:
> On Sun, Jan 14 2018, Anton Lindqvist <an...@openbsd.org> wrote:
> > On Thu, Jan 11, 2018 at 03:25:15PM +0800, Michael W. Bombardieri wrote:
> >> Hello,
> >> 
> >> Revision 1.9 of tree.c (from 1999) added the disabled code and it
> >> is still disabled. Would it be better to remove it?
> >
> > Fine with me. Anyone else willing OK?
> 
> I'd rather understand exactly what this code does and what the comment
> actually means.  The TEXEC case can definitely be reached.

The TEXEC case in ptree() happens if fptreef() or snptreef() are called with
format specifier %T.
>From what I see it is called like this:

exchild() -> snptreef() -> vfptreef() -> ptree().

Here snptreef() is used to copy the command string to be executed into 
a new Proc structure before exchild() forks and calls execute().

When calling ptree() by running shell command "ls -l", t->type is first TEXEC,
then after the goto t->type is TCOM.
So the switch statement appears to happen twice.

The comment "print original vars" does happen, but after the goto in TCOM case.
The if(t->vars) code in TCOM case is the same as the disabled if(t->left->vars)
code in TEXEC case.
Also, t->vars appears to be NULL in TEXEC case, but t->vars is set in TCOM case.
In terms of how the code is structured, vars belongs to TCOM not TEXEC.
So afaics the disabled code doesn't do anything useful.

- Michael

ksh: unused param in print_expansions()

[Michael W. Bombardieri]

Hello,

The local function print_expansions() is a wrapper for
x_print_expansions(). Going back to revision 1.1 of vi.c
reveals the command parameter wasn't used then either.

- Michael


Index: vi.c
===
RCS file: /cvs/src/bin/ksh/vi.c,v
retrieving revision 1.53
diff -u -p -u -r1.53 vi.c
--- vi.c6 Jan 2018 16:28:58 -   1.53
+++ vi.c11 Jan 2018 09:28:10 -
@@ -61,7 +61,7 @@ static void   display(char *, char *, int)
 static voided_mov_opt(int, char *);
 static int expand_word(int);
 static int complete_word(int, int);
-static int print_expansions(struct edstate *, int);
+static int print_expansions(struct edstate *);
 static int char_len(int);
 static voidx_vi_zotc(int);
 static voidvi_pprompt(int);
@@ -648,7 +648,7 @@ vi_insert(int ch)
break;
 
case CTRL('e'):
-   print_expansions(es, 0);
+   print_expansions(es);
break;
 
case CTRL('i'):
@@ -1125,7 +1125,7 @@ vi_cmd(int argcnt, const char *cmd)
 
case '=':   /* at ksh */
case CTRL('e'): /* Nonstandard vi/ksh */
-   print_expansions(es, 1);
+   print_expansions(es);
break;
 
 
@@ -2052,7 +2052,7 @@ complete_word(int command, int count)
 
/* Undo previous completion */
if (command == 0 && expanded == COMPLETE && buf) {
-   print_expansions(buf, 0);
+   print_expansions(buf);
expanded = PRINT;
return 0;
}
@@ -2143,7 +2143,7 @@ complete_word(int command, int count)
 }
 
 static int
-print_expansions(struct edstate *e, int command)
+print_expansions(struct edstate *e)
 {
int nwords;
int start, end;

disabled code in ksh tree.c

[Michael W. Bombardieri]

Hello,

Revision 1.9 of tree.c (from 1999) added the disabled code and it
is still disabled. Would it be better to remove it?

- Michael


Index: tree.c
===
RCS file: /cvs/src/bin/ksh/tree.c,v
retrieving revision 1.30
diff -u -p -u -r1.30 tree.c
--- tree.c  6 Jan 2018 16:28:58 -   1.30
+++ tree.c  11 Jan 2018 07:16:55 -
@@ -47,25 +47,8 @@ ptree(struct op *t, int indent, struct s
fptreef(shf, indent, "#no-args# ");
break;
case TEXEC:
-#if 0 /* ?not useful - can't be called? */
-   /* Print original vars */
-   if (t->left->vars)
-   for (w = t->left->vars; *w != NULL; )
-   fptreef(shf, indent, "%S ", *w++);
-   else
-   fptreef(shf, indent, "#no-vars# ");
-   /* Print expanded vars */
-   if (t->args)
-   for (w = t->args; *w != NULL; )
-   fptreef(shf, indent, "%s ", *w++);
-   else
-   fptreef(shf, indent, "#no-args# ");
-   /* Print original io */
-   t = t->left;
-#else
t = t->left;
goto Chain;
-#endif
case TPAREN:
fptreef(shf, indent + 2, "( %T) ", t->left);
break;

azalia_free_dmamem()

[Michael W. Bombardieri]

Hello,

This free function in azalia driver can be converted to void
because it has nothing useful to return. The old return value
was never used.
azalia's other free function azalia_freem() is already void.

- Michael


Index: azalia.c
===
RCS file: /cvs/src/sys/dev/pci/azalia.c,v
retrieving revision 1.239
diff -u -p -u -r1.239 azalia.c
--- azalia.c23 Dec 2017 01:44:24 -  1.239
+++ azalia.c9 Jan 2018 06:59:13 -
@@ -211,7 +211,7 @@ int azalia_get_response(azalia_t *, uint
 void   azalia_rirb_kick_unsol_events(void *);
 void   azalia_rirb_intr(azalia_t *);
 intazalia_alloc_dmamem(azalia_t *, size_t, size_t, azalia_dma_t *);
-intazalia_free_dmamem(const azalia_t *, azalia_dma_t*);
+void   azalia_free_dmamem(const azalia_t *, azalia_dma_t*);
 
 intazalia_codec_init(codec_t *);
 intazalia_codec_delete(codec_t *);
@@ -1331,17 +1331,16 @@ free:
return err;
 }
 
-int
+void
 azalia_free_dmamem(const azalia_t *az, azalia_dma_t* d)
 {
if (d->addr == NULL)
-   return 0;
+   return;
bus_dmamap_unload(az->dmat, d->map);
bus_dmamap_destroy(az->dmat, d->map);
bus_dmamem_unmap(az->dmat, d->addr, d->size);
bus_dmamem_free(az->dmat, d->segments, 1);
d->addr = NULL;
-   return 0;
 }
 
 int

ifdef bwfm_pci_debug_console()

[Michael W. Bombardieri]

Hello,

I noticed that calls to the function bwfm_pci_debug_console() 
are wrapped in ifdef but the function itself isn't.

- Michael


Index: if_bwfm_pci.c
===
RCS file: /cvs/src/sys/dev/pci/if_bwfm_pci.c,v
retrieving revision 1.8
diff -u -p -u -r1.8 if_bwfm_pci.c
--- if_bwfm_pci.c   8 Jan 2018 00:46:15 -   1.8
+++ if_bwfm_pci.c   8 Jan 2018 02:29:14 -
@@ -255,7 +255,9 @@ void bwfm_pci_flowring_delete(struct b
 
 voidbwfm_pci_stop(struct bwfm_softc *);
 int bwfm_pci_txdata(struct bwfm_softc *, struct mbuf *);
+#ifdef BWFM_DEBUG
 voidbwfm_pci_debug_console(struct bwfm_pci_softc *);
+#endif
 
 int bwfm_pci_msgbuf_query_dcmd(struct bwfm_softc *, int,
int, char *, size_t *);
@@ -1666,6 +1668,7 @@ bwfm_pci_txdata(struct bwfm_softc *bwfm,
return 0;
 }
 
+#ifdef BWFM_DEBUG
 void
 bwfm_pci_debug_console(struct bwfm_pci_softc *sc)
 {
@@ -1685,6 +1688,7 @@ bwfm_pci_debug_console(struct bwfm_pci_s
printf("%c", ch);
}
 }
+#endif 
 
 int
 bwfm_pci_intr(void *v)

yacc print_grammar(): loop -> fprintf()

[Michael W. Bombardieri]

Hello,

In yacc the function print_grammar() writes the file y.output if
the -v option is used. fprintf() can be used directly for writing
"spacing" space characters of indentation so a loop can be removed.

- Michael


Index: reader.c
===
RCS file: /cvs/src/usr.bin/yacc/reader.c,v
retrieving revision 1.34
diff -u -p -u -r1.34 reader.c
--- reader.c25 May 2017 20:11:03 -  1.34
+++ reader.c3 Jan 2018 12:40:54 -
@@ -1806,7 +1806,7 @@ pack_grammar(void)
 void
 print_grammar(void)
 {
-   int i, j, k;
+   int i, k;
int spacing = 0;
FILE *f = verbose_file;
 
@@ -1822,9 +1822,8 @@ print_grammar(void)
spacing = strlen(symbol_name[rlhs[i]]) + 1;
} else {
fprintf(f, "%4d  ", i - 2);
-   j = spacing;
-   while (--j >= 0)
-   putc(' ', f);
+   if (spacing > 0)
+   fprintf(f, "%*s", spacing, "");
putc('|', f);
}
 

size for free() in if_bwfm_pci.c

[Michael W. Bombardieri]

Hello,

In bwfm_pci_dmamem_alloc() and bwfm_pci_dmamem_free(), bdm points
to a single struct not an array. When freeing it we can just use
the struct size. Does this look correct?

- Michael


Index: if_bwfm_pci.c
===
RCS file: /cvs/src/sys/dev/pci/if_bwfm_pci.c,v
retrieving revision 1.3
diff -u -p -u -r1.3 if_bwfm_pci.c
--- if_bwfm_pci.c   1 Jan 2018 22:41:56 -   1.3
+++ if_bwfm_pci.c   3 Jan 2018 07:26:39 -
@@ -752,7 +752,7 @@ free:
 destroy:
bus_dmamap_destroy(sc->sc_dmat, bdm->bdm_map);
 bdmfree:
-   free(bdm, M_DEVBUF, 0);
+   free(bdm, M_DEVBUF, sizeof(*bdm));
 
return (NULL);
 }
@@ -763,7 +763,7 @@ bwfm_pci_dmamem_free(struct bwfm_pci_sof
bus_dmamem_unmap(sc->sc_dmat, bdm->bdm_kva, bdm->bdm_size);
bus_dmamem_free(sc->sc_dmat, >bdm_seg, 1);
bus_dmamap_destroy(sc->sc_dmat, bdm->bdm_map);
-   free(bdm, M_DEVBUF, 0);
+   free(bdm, M_DEVBUF, sizeof(*bdm));
 }
 
 /*

make: guard condition in shortened_curdir()

[Michael W. Bombardieri]

Hello,

make(1) has a guard in shortened_curdir() so
Buf_Init() should only happen once. I think this is
desirable because Buf_Init() malloc's space and old storage
pointers could be overwritten.
The guard doesn't work as intended because 'first'
is always true.

- Michael


Index: job.c
===
RCS file: /cvs/src/usr.bin/make/job.c,v
retrieving revision 1.139
diff -u -p -u -r1.139 job.c
--- job.c   21 Jan 2017 12:35:40 -  1.139
+++ job.c   19 Dec 2017 08:02:28 -
@@ -220,7 +220,7 @@ static const char *
 shortened_curdir(void)
 {
static BUFFER buf;
-   bool first = true;
+   static bool first = true;
if (first) {
Buf_Init(, 0);
buf_addcurdir();

one XXX in ksh(1)

[Michael W. Bombardieri]

Hello,

In my understanding the reason why texec had to be static was
because the struct member ioact was never initialised.
The call tree is execute() -> comexec() -> exchild() -> execute()
and a fork happens in exchild().
The second execute() dereferences t->ioact on line 115 which causes
SEGV. Setting ioact to NULL explicitly seems better than
doing this via "static".
Does this work for you?

- Michael


Index: exec.c
===
RCS file: /cvs/src/bin/ksh/exec.c,v
retrieving revision 1.68
diff -u -p -u -r1.68 exec.c
--- exec.c  11 Dec 2016 17:49:19 -  1.68
+++ exec.c  18 Dec 2017 07:05:47 -
@@ -409,7 +409,7 @@ comexec(struct op *t, struct tbl *volati
volatile int rv = 0;
char *cp;
char **lastp;
-   static struct op texec; /* Must be static (XXX but why?) */
+   struct op texec;
int type_flags;
int keepasn_ok;
int fcflags = FC_BI|FC_FUNC|FC_PATH;
@@ -688,6 +688,7 @@ comexec(struct op *t, struct tbl *volati
texec.left = t; /* for tprint */
texec.str = tp->val.s;
texec.args = ap;
+   texec.ioact = NULL;
rv = exchild(, flags, xerrok, -1);
break;
}

csh dounsetenv()

[Michael W. Bombardieri]

Hello,

The free() at the top of dounsetenv() in csh(1) isn't needed
because name is always freed before returning at bottom of function.
Also, name itself is never returned so it doesn't need to be static.

./csh
setenv HEY YU
unsetenv HEY
printenv

I ran the above and it seems to work the same as before.

- Michael


Index: func.c
===
RCS file: /cvs/src/bin/csh/func.c,v
retrieving revision 1.36
diff -u -p -u -r1.36 func.c
--- func.c  16 Dec 2017 10:27:21 -  1.36
+++ func.c  17 Dec 2017 09:41:01 -
@@ -924,11 +924,9 @@ void
 /*ARGSUSED*/
 dounsetenv(Char **v, struct command *t)
 {
-Char  **ep, *p, *n;
+Char  **ep, *p, *n, *name;
 int i, maxi;
-static Char *name = NULL;
 
-free(name);
 /*
  * Find the longest environment variable
  */

csh: NULL checks before free()

[Michael W. Bombardieri]

Hello,

Previously in csh(1) the xfree() function was removed in favour
of regular free(). This patch removes a couple of NULL checks
before free().

- Michael

PS: Thanks for putting hostname in the default prompt!


Index: csh.c
===
RCS file: /cvs/src/bin/csh/csh.c,v
retrieving revision 1.42
diff -u -p -u -r1.42 csh.c
--- csh.c   12 Dec 2017 00:18:58 -  1.42
+++ csh.c   15 Dec 2017 08:50:48 -
@@ -1013,10 +1013,8 @@ process(bool catch)
printprompt();
(void) fflush(cshout);
}
-   if (seterr) {
-   free(seterr);
-   seterr = NULL;
-   }
+   free(seterr);
+   seterr = NULL;
 
/*
 * Echo not only on VERBOSE, but also with history expansion. If there
Index: dol.c
===
RCS file: /cvs/src/bin/csh/dol.c,v
retrieving revision 1.20
diff -u -p -u -r1.20 dol.c
--- dol.c   26 Dec 2015 13:48:38 -  1.20
+++ dol.c   15 Dec 2017 08:50:48 -
@@ -408,8 +408,7 @@ Dgetdol(void)
if (dimen || bitset)
stderror(ERR_SYNTAX);
if (backpid != 0) {
-   if (dolbang)
-   free(dolbang);
+   free(dolbang);
setDolp(dolbang = putn(backpid));
}
goto eatbrac;
Index: error.c
===
RCS file: /cvs/src/bin/csh/error.c,v
retrieving revision 1.12
diff -u -p -u -r1.12 error.c
--- error.c 26 Dec 2015 13:48:38 -  1.12
+++ error.c 15 Dec 2017 08:50:48 -
@@ -346,10 +346,8 @@ stderror(int id, ...)
}
 }
 
-if (seterr) {
-   free(seterr);
-   seterr = NULL;
-}
+free(seterr);
+seterr = NULL;
 
 if ((v = pargv) != NULL)
pargv = 0, blkfree(v);
Index: func.c
===
RCS file: /cvs/src/bin/csh/func.c,v
retrieving revision 1.35
diff -u -p -u -r1.35 func.c
--- func.c  30 Aug 2017 06:42:21 -  1.35
+++ func.c  15 Dec 2017 08:50:48 -
@@ -824,8 +824,7 @@ wfree(void)
 
if (wp->w_fe0)
blkfree(wp->w_fe0);
-   if (wp->w_fename)
-   free(wp->w_fename);
+   free(wp->w_fename);
free(wp);
 }
 }
@@ -929,8 +928,7 @@ dounsetenv(Char **v, struct command *t)
 int i, maxi;
 static Char *name = NULL;
 
-if (name)
-   free(name);
+free(name);
 /*
  * Find the longest environment variable
  */

dc: strdup -> bstrdup

[Michael W. Bombardieri]

Hello,

Mostly dc(1) uses its own bstrdup() which exits on error.
It can be used in two more places.

- Michael


Index: dc.c
===
RCS file: /cvs/src/usr.bin/dc/dc.c,v
retrieving revision 1.19
diff -u -p -u -r1.19 dc.c
--- dc.c29 Nov 2017 14:34:17 -  1.19
+++ dc.c6 Dec 2017 08:44:30 -
@@ -47,8 +47,7 @@ dc_main(int argc, char *argv[])
char*buf, *p;
struct stat st;
 
-   if ((buf = strdup("")) == NULL)
-   err(1, NULL);
+   buf = bstrdup("");
/* accept and ignore a single dash to be 4.4BSD dc(1) compatible */
optind = 1;
optreset = 1;
Index: stack.c
===
RCS file: /cvs/src/usr.bin/dc/stack.c,v
retrieving revision 1.14
diff -u -p -u -r1.14 stack.c
--- stack.c 27 Mar 2016 15:55:13 -  1.14
+++ stack.c 6 Dec 2017 08:44:30 -
@@ -79,9 +79,7 @@ stack_dup_value(const struct value *a, s
copy->u.num = dup_number(a->u.num);
break;
case BCODE_STRING:
-   copy->u.string = strdup(a->u.string);
-   if (copy->u.string == NULL)
-   err(1, NULL);
+   copy->u.string = bstrdup(a->u.string);
break;
}
 

lex: simplify PRINT_SPACES() macro

[Michael W. Bombardieri]

Hello,

Upstream flex converted macro PRINT_SPACES() into a single fprintf() call.
https://github.com/westes/flex/commit/37a6184dabcd82fa1d17c24c000f3da469296195#diff-25d902c24283ab8cfbac54dfa101ad31
Applying this to OpenBSD lex reduces the differences in scanopt.c between the 
two.

- Michael


Index: scanopt.c
===
RCS file: /cvs/src/usr.bin/lex/scanopt.c,v
retrieving revision 1.6
diff -u -p -u -r1.6 scanopt.c
--- scanopt.c   31 May 2017 07:20:26 -  1.6
+++ scanopt.c   30 Nov 2017 01:50:43 -
@@ -409,14 +409,8 @@ int scanopt_usage (scanner, fp, usag
}
desccol = maxlen[0] + indent * 2;
 
-#define PRINT_SPACES(fp,n)\
-do{\
-int _n;\
-_n=(n);\
-while(_n-- > 0)\
-fputc(' ',(fp));\
-}while(0)
-
+#define PRINT_SPACES(fp,n) \
+   fprintf((fp), "%*s", (n), "")
 
/* Second pass (same as above loop), this time we print. */
/* Sloppy hack: We iterate twice. The first time we print short and 
long options.

cvsweb: cvs annotate error

[Michael W. Bombardieri]

Hello,

Sorry if this is the wrong list, but I found a couple of branch
revisions in file src/sys/dev/midi.c which produce an error when
clicking "annotate" link in cvsweb.
http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/dev/midi.c?annotate=1.40.6.1
http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/dev/midi.c?annotate=1.42.4.1

The error is "Error: Error occured during annotate: error".
Posting it here in case someone has seen it before.

- Michael

ehci_alloc_itd_chain() tweak

[Michael W. Bombardieri]

Hello,

In ehci_alloc_itd_chain(), no need to compute uframes if nframes
is zero and function would return early.

- Michael


Index: src/sys/dev/usb/ehci.c
===
RCS file: /cvs/src/sys/dev/usb/ehci.c,v
retrieving revision 1.200
diff -u -p -u -r1.200 ehci.c
--- src/sys/dev/usb/ehci.c  15 May 2017 10:52:08 -  1.200
+++ src/sys/dev/usb/ehci.c  22 Nov 2017 07:08:43 -
@@ -3385,9 +3385,9 @@ ehci_alloc_itd_chain(struct ehci_softc *
break;
}
nframes = (xfer->nframes + (ufrperframe - 1)) / ufrperframe;
-   uframes = 8 / ufrperframe;
if (nframes == 0)
return (1);
+   uframes = 8 / ufrperframe;
 
for (i = 0; i < nframes; i++) {
uint32_t froffs = offs;

Re: yacc: malloc+copy -> strndup()

[Michael W. Bombardieri]

On Fri, Nov 03, 2017 at 09:29:16AM +0100, Otto Moerbeek wrote:
> On Fri, Nov 03, 2017 at 04:22:43PM +0800, Michael W. Bombardieri wrote:
> 
> > On Fri, Nov 03, 2017 at 04:03:44PM +0800, Michael W. Bombardieri wrote:
> > > Hello,
> > > 
> > > In yacc the dup_line() function malloc()'ed a buffer and copied
> > > a line into it. The copied line includes \n.
> > > Allocate-and-copy can be done by strndup() in one hit.
> > > I ran this on i386 with awk/awkgram.y and rcs/date.y and didn't
> > > see any difference in y.tab.c compared to the system's yacc.
> > > 
> > > - Michael
> > 
> > Forgot to mention. The assert() is triggered if '\0' is encountered before 
> > '\n'.
> > Even if that is ok, a better error message could be printed instead of 
> > assert().
> 
> Yes, we are very reluctant to use assert()... what if somebody compiles
> with NDEBUG?


When I put '\0' in an input file it seems to trigger syntax_error()
earlier on, so I decided to follow this. syntax_error() at least prints
the input line number.


Index: reader.c
===
RCS file: /cvs/src/usr.bin/yacc/reader.c,v
retrieving revision 1.34
diff -u -p -u -r1.34 reader.c
--- reader.c25 May 2017 20:11:03 -  1.34
+++ reader.c3 Nov 2017 09:07:47 -
@@ -171,21 +171,17 @@ get_line(void)
 char *
 dup_line(void)
 {
-   char *p, *s, *t;
+   char *p, *end;
+   size_t len;
 
if (line == NULL)
-   return (0);
-   s = line;
-   while (*s != '\n')
-   ++s;
-   p = malloc(s - line + 1);
+   return (NULL);
+   if ((end = strchr(line, '\n')) == NULL)
+   syntax_error(lineno, line, line + strlen(line));
+   len = end - line + 1;
+   p = strndup(line, len);
if (p == NULL)
no_space();
-
-   s = line;
-   t = p;
-   while ((*t++ = *s++) != '\n')
-   continue;
return (p);
 }
 

Re: yacc: malloc+copy -> strndup()

[Michael W. Bombardieri]

On Fri, Nov 03, 2017 at 04:03:44PM +0800, Michael W. Bombardieri wrote:
> Hello,
> 
> In yacc the dup_line() function malloc()'ed a buffer and copied
> a line into it. The copied line includes \n.
> Allocate-and-copy can be done by strndup() in one hit.
> I ran this on i386 with awk/awkgram.y and rcs/date.y and didn't
> see any difference in y.tab.c compared to the system's yacc.
> 
> - Michael

Forgot to mention. The assert() is triggered if '\0' is encountered before '\n'.
Even if that is ok, a better error message could be printed instead of assert().

> 
> 
> Index: reader.c
> ===
> RCS file: /cvs/src/usr.bin/yacc/reader.c,v
> retrieving revision 1.34
> diff -u -p -u -r1.34 reader.c
> --- reader.c  25 May 2017 20:11:03 -  1.34
> +++ reader.c  3 Nov 2017 07:48:08 -
> @@ -171,21 +171,17 @@ get_line(void)
>  char *
>  dup_line(void)
>  {
> - char *p, *s, *t;
> + char *p, *end;
> + size_t len;
>  
>   if (line == NULL)
> - return (0);
> - s = line;
> - while (*s != '\n')
> - ++s;
> - p = malloc(s - line + 1);
> + return (NULL);
> + end = strchr(line, '\n');
> + assert(end != NULL);
> + len = end - line + 1;
> + p = strndup(line, len);
>   if (p == NULL)
>   no_space();
> -
> - s = line;
> - t = p;
> - while ((*t++ = *s++) != '\n')
> - continue;
>   return (p);
>  }
>  

yacc: malloc+copy -> strndup()

[Michael W. Bombardieri]

Hello,

In yacc the dup_line() function malloc()'ed a buffer and copied
a line into it. The copied line includes \n.
Allocate-and-copy can be done by strndup() in one hit.
I ran this on i386 with awk/awkgram.y and rcs/date.y and didn't
see any difference in y.tab.c compared to the system's yacc.

- Michael


Index: reader.c
===
RCS file: /cvs/src/usr.bin/yacc/reader.c,v
retrieving revision 1.34
diff -u -p -u -r1.34 reader.c
--- reader.c25 May 2017 20:11:03 -  1.34
+++ reader.c3 Nov 2017 07:48:08 -
@@ -171,21 +171,17 @@ get_line(void)
 char *
 dup_line(void)
 {
-   char *p, *s, *t;
+   char *p, *end;
+   size_t len;
 
if (line == NULL)
-   return (0);
-   s = line;
-   while (*s != '\n')
-   ++s;
-   p = malloc(s - line + 1);
+   return (NULL);
+   end = strchr(line, '\n');
+   assert(end != NULL);
+   len = end - line + 1;
+   p = strndup(line, len);
if (p == NULL)
no_space();
-
-   s = line;
-   t = p;
-   while ((*t++ = *s++) != '\n')
-   continue;
return (p);
 }
 

makefs: malloc -> emalloc

[Michael W. Bombardieri]

Hello,

makefs has an xmalloc.c with emalloc() function, but one thing
was still using malloc() directly. This patch makes malloc()
always happen through emalloc().

- Michael


Index: msdos/mkfs_msdos.c
===
RCS file: /cvs/src/usr.sbin/makefs/msdos/mkfs_msdos.c,v
retrieving revision 1.4
diff -u -p -u -r1.4 mkfs_msdos.c
--- msdos/mkfs_msdos.c  28 Mar 2017 00:08:39 -  1.4
+++ msdos/mkfs_msdos.c  1 Nov 2017 02:58:21 -
@@ -592,8 +592,7 @@ mkfs_msdos(const char *fname, const char
 gettimeofday(, NULL);
 now = tv.tv_sec;
 tm = localtime();
-if (!(img = malloc(bpb.bps)))
-err(1, NULL);
+img = emalloc(bpb.bps);
 dir = bpb.res + (bpb.spf ? bpb.spf : bpb.bspf) * bpb.nft;
 signal(SIGINFO, infohandler);
 for (lsn = 0; lsn < dir + (o.fat_type == 32 ? bpb.spc : rds); lsn++) {

Re: makefs(8): add EFI platform-id for UFI

[Michael W. Bombardieri]

On Mon, Oct 30, 2017 at 05:27:29PM +0900, YASUOKA Masahiko wrote:
> Hi,
> 
> I'd like to add a platform-id for EFI boot.
> 
> ok?
> 
> diff --git a/usr.sbin/makefs/cd9660/cd9660_eltorito.c 
> b/usr.sbin/makefs/cd9660/cd9660_eltorito.c
> index 46ec432bc84..376c42f5dc3 100644
> --- a/usr.sbin/makefs/cd9660/cd9660_eltorito.c
> +++ b/usr.sbin/makefs/cd9660/cd9660_eltorito.c
> @@ -104,6 +104,8 @@ cd9660_add_boot_disk(iso9660_disk *diskStructure, const 
> char *boot_info)
>   new_image->system = ET_SYS_PPC;
>   else if (strcmp(sysname, "macppc") == 0)
>   new_image->system = ET_SYS_MAC;
> + else if (strcmp(sysname, "efi") == 0)
> + new_image->system = ET_SYS_EFI;
>   else {
>   warnx("boot disk system must be "
> "i386, macppc, or powerpc");

Should efi be added to the warning message too, or changing it to something like
Unknown boot disk system?

> @@ -335,12 +337,12 @@ cd9660_setup_boot(iso9660_disk *diskStructure, int 
> first_sector)
>   int used_sectors;
>   int num_entries = 0;
>   int catalog_sectors;
> - struct boot_catalog_entry *x86_head, *mac_head, *ppc_head,
> + struct boot_catalog_entry *x86_head, *mac_head, *ppc_head, *efi_head,
>   *valid_entry, *default_entry, *temp, *head, **headp, *next;
>   struct cd9660_boot_image *tmp_disk;
>  
>   headp = NULL;
> - x86_head = mac_head = ppc_head = NULL;
> + x86_head = mac_head = ppc_head = efi_head = NULL;
>  
>   /* If there are no boot disks, don't bother building boot information */
>   if (TAILQ_EMPTY(>boot_images))
> @@ -413,6 +415,9 @@ cd9660_setup_boot(iso9660_disk *diskStructure, int 
> first_sector)
>   case ET_SYS_MAC:
>   headp = _head;
>   break;
> + case ET_SYS_EFI:
> + headp = _head;
> + break;
>   default:
>   warnx("%s: internal error: unknown system type",
>   __func__);
> diff --git a/usr.sbin/makefs/cd9660/cd9660_eltorito.h 
> b/usr.sbin/makefs/cd9660/cd9660_eltorito.h
> index 43483018ef3..f527bc56b46 100644
> --- a/usr.sbin/makefs/cd9660/cd9660_eltorito.h
> +++ b/usr.sbin/makefs/cd9660/cd9660_eltorito.h
> @@ -41,6 +41,7 @@
>  #define  ET_SYS_X86  0
>  #define  ET_SYS_PPC  1
>  #define  ET_SYS_MAC  2
> +#define  ET_SYS_EFI  0xfe
>  
>  #define ET_BOOT_ENTRY_SIZE 0x20
>  
> 

Re: cdio: read_track: plug leak

[Michael W. Bombardieri]

Hi,

Patch was also posted here, but I didn't test it.
https://marc.info/?l=openbsd-tech=149784342025304=2

- Michael


On Sun, Sep 10, 2017 at 04:23:49PM -0500, Scott Cheloha wrote:
> Hi,
> 
> Saw this when preparing the monotonic clock patch.
> 
> This is a leak, right?  Every other return path in read_track()
> aside from the malloc failure frees sec.
> 
> I think the function itself is more confusing than it needs to be
> and could use a refactor but that belongs in a separate patch.
> 
> Feedback?
> 
> --
> Scott Cheloha
> 
> Index: usr.bin/cdio/rip.c
> ===
> RCS file: /cvs/src/usr.bin/cdio/rip.c,v
> retrieving revision 1.16
> diff -u -p -r1.16 rip.c
> --- usr.bin/cdio/rip.c20 Aug 2015 22:32:41 -  1.16
> +++ usr.bin/cdio/rip.c10 Sep 2017 21:16:51 -
> @@ -398,6 +398,7 @@ read_track(struct track *ti)
>   }
>   if (ti->hdl != NULL &&
>   (sio_write(ti->hdl, sec, blksize) == 0)) {
> + free(sec);
>   sio_close(ti->hdl);
>   ti->hdl = NULL;
>   warnx("\nerror while writing to audio output");
> 

Re: awk fpecatch() update

[Michael W. Bombardieri]

On Tue, Sep 05, 2017 at 01:27:31PM +0800, Michael W. Bombardieri wrote:
> Hello,
> 
> Updated version of awk (Dec 20, 2012) [1] has a simplification
> in fpecatch() function. I tried to test this to compare the
> error output. 

Oops, I failed to notice it was a local change from revision 1.9.
http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/awk/lib.c.diff?r1=1.8=1.9=date=h


> 
> Old version:
> $ /usr/bin/awk '{}'
> *** receive SIGFPE via kill(1) ***
> floating point exception
>  source line number 1
> 
> New version:
> $ ./awk '{}'
> *** receive SIGFPE via kill(1) ***
> awk: floating point exception 8
>  source line number 1
> 
> And with debugging flag set, the core dump still happens as expected...
> 
> Old version:
> $ awk -d9 '{}'
> awk version 20110810
> program = |{}|
> setsymtab set 0x7ac4f180: n=0 s="0" f=0 t=17
> *** SNIP ***
> lex token 123
> lex token 59
> lex token 125
> errorflag=0
> RS=<
> >, FS=< >, ARGC=1, FILENAME=
> argno=1, file=||
> *** receive SIGFPE via kill(1) ***
> floating point exception
>  source line number 1
> Abort trap (core dumped)
> 
> New version:
> $ ./awk -d9 '{}'
> awk version 20110810
> program = |{}|
> setsymtab set 0x80e58840: n=0 s="0" f=0 t=17
> *** SNIP ***
> lex token 123
> lex token 59
> lex token 125
> errorflag=0
> RS=<
> >, FS=< >, ARGC=1, FILENAME=
> argno=1, file=||
> *** receive SIGFPE via kill(1) ***
> awk: floating point exception 8
>  source line number 1
> Abort trap (core dumped)
> 
> No significant difference for this trivial test.
> All I notice is the error now starts with command name.
> 
> - Michael
> 
> 
> [1] http://www.cs.princeton.edu/~bwk/btl.mirror/awk.tar.gz
> 
> Index: lib.c
> ===
> RCS file: /cvs/src/usr.bin/awk/lib.c,v
> retrieving revision 1.22
> diff -u -p -u -r1.22 lib.c
> --- lib.c 12 Apr 2016 19:43:38 -  1.22
> +++ lib.c 5 Sep 2017 04:59:58 -
> @@ -529,39 +529,9 @@ void SYNTAX(const char *fmt, ...)
>   eprint();
>  }
>  
> -void fpecatch(int sig)
> +void fpecatch(int n)
>  {
> - extern Node *curnode;
> - char buf[1024];
> -
> - snprintf(buf, sizeof buf, "floating point exception\n");
> - write(STDERR_FILENO, buf, strlen(buf));
> -
> - if (compile_time != 2 && NR && *NR > 0) {
> - snprintf(buf, sizeof buf, " input record number %d", (int) 
> (*FNR));
> - write(STDERR_FILENO, buf, strlen(buf));
> -
> - if (strcmp(*FILENAME, "-") != 0) {
> - snprintf(buf, sizeof buf, ", file %s", *FILENAME);
> - write(STDERR_FILENO, buf, strlen(buf));
> - }
> - write(STDERR_FILENO, "\n", 1);
> - }
> - if (compile_time != 2 && curnode) {
> - snprintf(buf, sizeof buf, " source line number %d", 
> curnode->lineno);
> - write(STDERR_FILENO, buf, strlen(buf));
> - } else if (compile_time != 2 && lineno) {
> - snprintf(buf, sizeof buf, " source line number %d", lineno);
> - write(STDERR_FILENO, buf, strlen(buf));
> - }
> - if (compile_time == 1 && cursource() != NULL) {
> - snprintf(buf, sizeof buf, " source file %s", cursource());
> - write(STDERR_FILENO, buf, strlen(buf));
> - }
> - write(STDERR_FILENO, "\n", 1);
> - if (dbg > 1)/* core dump if serious debugging on */
> - abort();
> - _exit(1);
> + FATAL("floating point exception %d", n);
>  }
>  
>  extern int bracecnt, brackcnt, parencnt;

awk fpecatch() update

[Michael W. Bombardieri]

Hello,

Updated version of awk (Dec 20, 2012) [1] has a simplification
in fpecatch() function. I tried to test this to compare the
error output. 

Old version:
$ /usr/bin/awk '{}'
*** receive SIGFPE via kill(1) ***
floating point exception
 source line number 1

New version:
$ ./awk '{}'
*** receive SIGFPE via kill(1) ***
awk: floating point exception 8
 source line number 1

And with debugging flag set, the core dump still happens as expected...

Old version:
$ awk -d9 '{}'
awk version 20110810
program = |{}|
setsymtab set 0x7ac4f180: n=0 s="0" f=0 t=17
*** SNIP ***
lex token 123
lex token 59
lex token 125
errorflag=0
RS=<
>, FS=< >, ARGC=1, FILENAME=
argno=1, file=||
*** receive SIGFPE via kill(1) ***
floating point exception
 source line number 1
Abort trap (core dumped)

New version:
$ ./awk -d9 '{}'
awk version 20110810
program = |{}|
setsymtab set 0x80e58840: n=0 s="0" f=0 t=17
*** SNIP ***
lex token 123
lex token 59
lex token 125
errorflag=0
RS=<
>, FS=< >, ARGC=1, FILENAME=
argno=1, file=||
*** receive SIGFPE via kill(1) ***
awk: floating point exception 8
 source line number 1
Abort trap (core dumped)

No significant difference for this trivial test.
All I notice is the error now starts with command name.

- Michael


[1] http://www.cs.princeton.edu/~bwk/btl.mirror/awk.tar.gz

Index: lib.c
===
RCS file: /cvs/src/usr.bin/awk/lib.c,v
retrieving revision 1.22
diff -u -p -u -r1.22 lib.c
--- lib.c   12 Apr 2016 19:43:38 -  1.22
+++ lib.c   5 Sep 2017 04:59:58 -
@@ -529,39 +529,9 @@ void SYNTAX(const char *fmt, ...)
eprint();
 }
 
-void fpecatch(int sig)
+void fpecatch(int n)
 {
-   extern Node *curnode;
-   char buf[1024];
-
-   snprintf(buf, sizeof buf, "floating point exception\n");
-   write(STDERR_FILENO, buf, strlen(buf));
-
-   if (compile_time != 2 && NR && *NR > 0) {
-   snprintf(buf, sizeof buf, " input record number %d", (int) 
(*FNR));
-   write(STDERR_FILENO, buf, strlen(buf));
-
-   if (strcmp(*FILENAME, "-") != 0) {
-   snprintf(buf, sizeof buf, ", file %s", *FILENAME);
-   write(STDERR_FILENO, buf, strlen(buf));
-   }
-   write(STDERR_FILENO, "\n", 1);
-   }
-   if (compile_time != 2 && curnode) {
-   snprintf(buf, sizeof buf, " source line number %d", 
curnode->lineno);
-   write(STDERR_FILENO, buf, strlen(buf));
-   } else if (compile_time != 2 && lineno) {
-   snprintf(buf, sizeof buf, " source line number %d", lineno);
-   write(STDERR_FILENO, buf, strlen(buf));
-   }
-   if (compile_time == 1 && cursource() != NULL) {
-   snprintf(buf, sizeof buf, " source file %s", cursource());
-   write(STDERR_FILENO, buf, strlen(buf));
-   }
-   write(STDERR_FILENO, "\n", 1);
-   if (dbg > 1)/* core dump if serious debugging on */
-   abort();
-   _exit(1);
+   FATAL("floating point exception %d", n);
 }
 
 extern int bracecnt, brackcnt, parencnt;

align struct rcs_num between cvs(1) and rcs(1)

[Michael W. Bombardieri]

Hi,

The struct rcs_num in rcs(1) had a pointer for rn_id, but the version
in cvs(1) had an array of size RCSNUM_MAX. This patch tries to make
rcs(1) follow cvs(1) here, so rn_id doesn't need to be freed.
As a result the rcsnum_free() function is replaced by one free().

After applying this patch I tried "ci", "co", "ci" (2nd rev), then
"rlog" on a test file and it seemed to work fine. I didn't try
rcsdiff or rcsmerge though. Does this break anything for you?

- Michael


Index: ci.c
===
RCS file: /cvs/src/usr.bin/rcs/ci.c,v
retrieving revision 1.224
diff -u -p -u -r1.224 ci.c
--- ci.c4 Jul 2016 01:39:12 -   1.224
+++ ci.c4 Sep 2017 08:21:45 -
@@ -314,7 +314,7 @@ checkin_main(int argc, char **argv)
 
rcs_close(pb.file);
if (rev_str != NULL)
-   rcsnum_free(pb.newrev);
+   free(pb.newrev);
pb.newrev = NULL;
}
 
@@ -400,7 +400,7 @@ checkin_getlogmsg(RCSNUM *rev, RCSNUM *r
rcsnum_tostr(rcsnum_inc(tmprev), nrev, sizeof(nrev));
else
rcsnum_tostr(rev2, nrev, sizeof(nrev));
-   rcsnum_free(tmprev);
+   free(tmprev);
 
if (!(flags & QUIET))
(void)fprintf(stderr, "new revision: %s; "
@@ -635,7 +635,7 @@ skipdesc:
if (fetchlog == 1) {
pb->rcs_msg = checkin_getlogmsg(pb->frev, pb->newrev,
pb->flags);
-   rcsnum_free(pb->frev);
+   free(pb->frev);
}
 
/*
Index: co.c
===
RCS file: /cvs/src/usr.bin/rcs/co.c,v
retrieving revision 1.123
diff -u -p -u -r1.123 co.c
--- co.c29 Aug 2017 16:47:33 -  1.123
+++ co.c4 Sep 2017 08:21:45 -
@@ -199,7 +199,7 @@ checkout_main(int argc, char **argv)
if (checkout_rev(file, rev, argv[i], flags,
username, author, state, date) < 0) {
rcs_close(file);
-   rcsnum_free(rev);
+   free(rev);
ret = 1;
continue;
}
@@ -207,7 +207,7 @@ checkout_main(int argc, char **argv)
if (!(flags & QUIET))
(void)fprintf(stderr, "done\n");
 
-   rcsnum_free(rev);
+   free(rev);
 
rcs_write(file);
if (flags & PRESERVETIME)
Index: rcs.c
===
RCS file: /cvs/src/usr.bin/rcs/rcs.c,v
retrieving revision 1.85
diff -u -p -u -r1.85 rcs.c
--- rcs.c   9 May 2016 13:03:55 -   1.85
+++ rcs.c   4 Sep 2017 08:21:46 -
@@ -165,7 +165,7 @@ rcs_close(RCSFILE *rfp)
while (!TAILQ_EMPTY(&(rfp->rf_symbols))) {
rsp = TAILQ_FIRST(&(rfp->rf_symbols));
TAILQ_REMOVE(&(rfp->rf_symbols), rsp, rs_list);
-   rcsnum_free(rsp->rs_num);
+   free(rsp->rs_num);
free(rsp->rs_name);
free(rsp);
}
@@ -173,13 +173,13 @@ rcs_close(RCSFILE *rfp)
while (!TAILQ_EMPTY(&(rfp->rf_locks))) {
rlp = TAILQ_FIRST(&(rfp->rf_locks));
TAILQ_REMOVE(&(rfp->rf_locks), rlp, rl_list);
-   rcsnum_free(rlp->rl_num);
+   free(rlp->rl_num);
free(rlp->rl_name);
free(rlp);
}
 
-   rcsnum_free(rfp->rf_head);
-   rcsnum_free(rfp->rf_branch);
+   free(rfp->rf_head);
+   free(rfp->rf_branch);
 
if (rfp->rf_file != NULL)
fclose(rfp->rf_file);
@@ -579,7 +579,7 @@ rcs_sym_remove(RCSFILE *file, const char
 
TAILQ_REMOVE(&(file->rf_symbols), symp, rs_list);
free(symp->rs_name);
-   rcsnum_free(symp->rs_num);
+   free(symp->rs_num);
free(symp);
 
/* not synced anymore */
@@ -738,7 +738,7 @@ rcs_lock_remove(RCSFILE *file, const cha
}
 
TAILQ_REMOVE(&(file->rf_locks), lkp, rl_list);
-   rcsnum_free(lkp->rl_num);
+   free(lkp->rl_num);
free(lkp->rl_name);
free(lkp);
 
@@ -1228,10 +1228,10 @@ rcs_rev_remove(RCSFILE *rf, RCSNUM *rev)
if (rcs_head_set(rf, prevrdp->rd_num) < 0)
errx(1, "rcs_head_set failed");
} else if (nextrdp != NULL) {
-   rcsnum_free(nextrdp->rd_next);
+   free(nextrdp->rd_next);
nextrdp->rd_next = rcsnum_alloc();
} else {
-   rcsnum_free(rf->rf_head);
+   free(rf->rf_head);
rf->rf_head = NULL;
}
 
@@ -1292,7 +1292,7 @@ rcs_findrev(RCSFILE *rfp, RCSNUM *rev)
frev = rdp->rd_next;
}
 
-   rcsnum_free(brev);
+   free(brev);
return (rdp);
}
 
@@ -1405,8 +1405,8 @@ 

sendbug categories

[Michael W. Bombardieri]

Hello,

Are sparc and vax still considered valid categories for
sendbug or should people now use only sparc64?

- Michael


Index: sendbug.c
===
RCS file: /cvs/src/usr.bin/sendbug/sendbug.c,v
retrieving revision 1.78
diff -u -p -u -r1.78 sendbug.c
--- sendbug.c   21 Aug 2017 21:41:13 -  1.78
+++ sendbug.c   4 Sep 2017 06:44:39 -
@@ -44,7 +44,7 @@ void  template(FILE *);
 void   usbdevs(FILE *);
 
 const char *categories = "system user library documentation kernel "
-"alpha amd64 arm hppa i386 m88k mips64 powerpc sh sparc sparc64 vax";
+"alpha amd64 arm hppa i386 m88k mips64 powerpc sh sparc64";
 const char *comment[] = {
"",
"",

Re: lex: action_m4_define

[Michael W. Bombardieri]

I forgot to mention that action_m4_define() was previously removed from
upstream flex here:
https://github.com/westes/flex/commit/925549475edb3c8921227fdd96aa0d90a2c0745a

On Sun, Aug 27, 2017 at 06:41:31PM +0800, Michael W. Bombardieri wrote:
> Hello,
> 
> In lex(1) the function action_m4_define() would raise a fatal
> error if called. I couldn't see anything calling it though,
> and the program builds without it.
> 
> - Michael
> 
> 
> Index: misc.c
> ===
> RCS file: /cvs/src/usr.bin/lex/misc.c,v
> retrieving revision 1.19
> diff -u -p -u -r1.19 misc.c
> --- misc.c19 Nov 2015 23:34:56 -  1.19
> +++ misc.c27 Aug 2017 10:27:34 -
> @@ -116,28 +116,6 @@ action_define(defname, value)
>   buf_append(_buf, , 1);
>  }
>  
> -
> -/** Append "m4_define([[defname]],[[value]])m4_dnl\n" to the running buffer.
> - *  @param defname The macro name.
> - *  @param value The macro value, can be NULL, which is the same as the 
> empty string.
> - */
> -void 
> -action_m4_define(const char *defname, const char *value)
> -{
> - char buf[MAXLINE];
> -
> - flexfatal("DO NOT USE THIS FUNCTION!");
> -
> - if ((int) strlen(defname) > MAXLINE / 2) {
> - format_pinpoint_message(_
> - ("name \"%s\" ridiculously long"),
> - defname);
> - return;
> - }
> - snprintf(buf, sizeof(buf), "m4_define([[%s]],[[%s]])m4_dnl\n", defname, 
> value ? value : "");
> - add_action(buf);
> -}
> -
>  /* Append "new_text" to the running buffer. */
>  void 
>  add_action(new_text)

lex: unused variable

[Michael W. Bombardieri]

Hello,

I spotted an unused variable (r) in filter_apply_chain() function
of lex(1).

- Michael


Index: filter.c
===
RCS file: /cvs/src/usr.bin/lex/filter.c,v
retrieving revision 1.8
diff -u -p -u -r1.8 filter.c
--- filter.c17 Aug 2017 19:27:48 -  1.8
+++ filter.c27 Aug 2017 11:09:06 -
@@ -174,9 +174,7 @@ filter_apply_chain(struct filter * chain
 
/* run as a filter, either internally or by exec */
if (chain->filter_func) {
-   int r;
-
-   if ((r = chain->filter_func(chain)) == -1)
+   if (chain->filter_func(chain) == -1)
flexfatal(_("filter_func failed"));
exit(0);
} else {

lex: action_m4_define

[Michael W. Bombardieri]

Hello,

In lex(1) the function action_m4_define() would raise a fatal
error if called. I couldn't see anything calling it though,
and the program builds without it.

- Michael


Index: misc.c
===
RCS file: /cvs/src/usr.bin/lex/misc.c,v
retrieving revision 1.19
diff -u -p -u -r1.19 misc.c
--- misc.c  19 Nov 2015 23:34:56 -  1.19
+++ misc.c  27 Aug 2017 10:27:34 -
@@ -116,28 +116,6 @@ action_define(defname, value)
buf_append(_buf, , 1);
 }
 
-
-/** Append "m4_define([[defname]],[[value]])m4_dnl\n" to the running buffer.
- *  @param defname The macro name.
- *  @param value The macro value, can be NULL, which is the same as the empty 
string.
- */
-void 
-action_m4_define(const char *defname, const char *value)
-{
-   char buf[MAXLINE];
-
-   flexfatal("DO NOT USE THIS FUNCTION!");
-
-   if ((int) strlen(defname) > MAXLINE / 2) {
-   format_pinpoint_message(_
-   ("name \"%s\" ridiculously long"),
-   defname);
-   return;
-   }
-   snprintf(buf, sizeof(buf), "m4_define([[%s]],[[%s]])m4_dnl\n", defname, 
value ? value : "");
-   add_action(buf);
-}
-
 /* Append "new_text" to the running buffer. */
 void 
 add_action(new_text)

make: setenv or esetenv

[Michael W. Bombardieri]

Hi,

In make(1), setenv() was mostly called through a wrapper function
esetenv() which exits on error.
When setting MAKEBASEDIRECTORY the return value of setenv() was
not checked. Converting the call to esetenv() makes it more consistent.
The third param of setenv(), overwrite, doesn't matter here. The
result of getenv() is checked first so nothing is written over.

- Michael


Index: main.c
===
RCS file: /cvs/src/usr.bin/make/main.c,v
retrieving revision 1.122
diff -u -p -u -r1.122 main.c
--- main.c  20 Apr 2017 03:04:11 -  1.122
+++ main.c  21 Aug 2017 08:17:07 -
@@ -713,7 +713,7 @@ main(int argc, char **argv)

basedirectory = getenv("MAKEBASEDIRECTORY");
if (basedirectory == NULL)
-   setenv("MAKEBASEDIRECTORY", d.current, 0);
+   esetenv("MAKEBASEDIRECTORY", d.current);
 
MainParseArgs(argc, argv);
 

lex: malloc+memset -> calloc

[Michael W. Bombardieri]

Hello,

Two instances of memset() can be removed in lex if calloc() is
used instead of malloc().

- Michael


Index: filter.c
===
RCS file: /cvs/src/usr.bin/lex/filter.c,v
retrieving revision 1.7
diff -u -p -u -r1.7 filter.c
--- filter.c18 Dec 2016 06:11:23 -  1.7
+++ filter.c17 Aug 2017 05:46:13 -
@@ -50,10 +50,9 @@ filter_create_ext(struct filter * chain,
va_list ap;
 
/* allocate and initialize new filter */
-   f = malloc(sizeof(struct filter));
+   f = calloc(sizeof(struct filter), 1);
if (!f)
-   flexerror(_("malloc failed (f) in filter_create_ext"));
-   memset(f, 0, sizeof(*f));
+   flexerror(_("calloc failed (f) in filter_create_ext"));
f->filter_func = NULL;
f->extra = NULL;
f->next = NULL;
@@ -103,10 +102,9 @@ filter_create_int(struct filter * chain,
struct filter *f;
 
/* allocate and initialize new filter */
-   f = malloc(sizeof(struct filter));
+   f = calloc(sizeof(struct filter), 1);
if (!f)
-   flexerror(_("malloc failed in filter_create_int"));
-   memset(f, 0, sizeof(*f));
+   flexerror(_("calloc failed in filter_create_int"));
f->next = NULL;
f->argc = 0;
f->argv = NULL;

make: clarify an error string

[Michael W. Bombardieri]

Hi,

In make(1), do_run_command() has a sanity check for a null command string.
The error message passes the null string to printf(). Is this any better?

- Michael
 

Index: engine.c
===
RCS file: /cvs/src/usr.bin/make/engine.c,v
retrieving revision 1.53
diff -u -p -u -r1.53 engine.c
--- engine.c9 Jul 2017 15:28:00 -   1.53
+++ engine.c24 Jul 2017 06:58:31 -
@@ -752,7 +752,7 @@ do_run_command(Job *job)
/* How can we execute a null command ? we warn the user that the
 * command expanded to nothing (is this the right thing to do?).  */
if (*cmd == '\0') {
-   Error("%s expands to empty string", cmd);
+   Error("command expands to empty string");
return false;
}
 

make: efree/free

[Michael W. Bombardieri]

Hi,

Some parts of make(1) use free() and others use efree() but
efree is just a define for free. Remove efree?

- Michael


Index: arch.c
===
RCS file: /cvs/src/usr.bin/make/arch.c,v
retrieving revision 1.88
diff -u -p -u -r1.88 arch.c
--- arch.c  21 Jul 2017 09:29:42 -  1.88
+++ arch.c  24 Jul 2017 02:43:08 -
@@ -411,7 +411,7 @@ read_archive(const char *archive, const 
 
/*  Whole archive read ok.  */
if (n == 0 && feof(arch)) {
-   efree(list.fnametab);
+   free(list.fnametab);
fclose(arch);
return ar;
}
@@ -495,7 +495,7 @@ read_archive(const char *archive, const 
 
fclose(arch);
ohash_delete(>members);
-   efree(list.fnametab);
+   free(list.fnametab);
free(ar);
return NULL;
 }
@@ -762,7 +762,7 @@ ArchFindMember(
 #endif
if (length == sizeof(arHeaderPtr->ar_name) ||
memberName[length] == ' ') {
-   efree(list.fnametab);
+   free(list.fnametab);
return arch;
}
}
@@ -786,7 +786,7 @@ ArchFindMember(
continue;
/* Got the entry.  */
if (strcmp(memberName, member) == 0) {
-   efree(list.fnametab);
+   free(list.fnametab);
return arch;
}
}
@@ -812,7 +812,7 @@ ArchFindMember(
printf("ArchFind: Extended format entry for 
%s\n", ename);
/* Found as extended name.  */
if (strcmp(ename, member) == 0) {
-   efree(list.fnametab);
+   free(list.fnametab);
return arch;
}
}
@@ -826,7 +826,7 @@ ArchFindMember(
/* We did not find the member, or we ran into an error while reading
 * the archive.  */
 #ifdef SVRARCHIVES
-   efree(list.fnametab);
+   free(list.fnametab);
 #endif
fclose(arch);
return NULL;
Index: suff.c
===
RCS file: /cvs/src/usr.bin/make/suff.c,v
retrieving revision 1.91
diff -u -p -u -r1.91 suff.c
--- suff.c  21 Oct 2016 16:12:38 -  1.91
+++ suff.c  24 Jul 2017 02:43:09 -
@@ -1531,7 +1531,7 @@ sfnd_abort:
 * path to be the name so Dir_MTime won't go grovelling
 * for it.  */
gn->suffix = targ == NULL ? NULL : targ->suff;
-   efree(gn->path);
+   free(gn->path);
gn->path = estrdup(gn->name);
}
 
@@ -1602,7 +1602,7 @@ sfnd_abort:
gn->suffix = src->suff;
 
/* So Dir_MTime doesn't go questing for it...  */
-   efree(gn->path);
+   free(gn->path);
gn->path = estrdup(gn->name);
 
/* Nuke the transformation path and the Src structures left over in the
Index: memory.h
===
RCS file: /cvs/src/usr.bin/make/memory.h,v
retrieving revision 1.9
diff -u -p -u -r1.9 memory.h
--- memory.h18 May 2014 08:08:50 -  1.9
+++ memory.h24 Jul 2017 02:43:09 -
@@ -45,10 +45,6 @@ extern void *ereallocarray(void *, size_
 extern int eunlink(const char *);
 extern void esetenv(const char *, const char *);
 
-/* efree(x) works when x==NULL. STDC behavior, may need some different
- * definition for cross-builds on deficient systems */
-#define efree  free
-
 extern void *hash_calloc(size_t, size_t, void *);
 extern void hash_free(void *, void *);
 extern void *element_alloc(size_t, void *);

warning when building make(1)

[Michael W. Bombardieri]

Hi,

The function build_target_group() produces two warnings when make is
built with CDIAGFLAGS.

parse.c:1462: warning: ISO C90 forbids mixed declarations and code
parse.c:1462: warning: 'gn2' may be used uninitialized in this function

This patch attempts to silence the warnings.

- Michael


Index: parse.c
===
RCS file: /cvs/src/usr.bin/make/parse.c,v
retrieving revision 1.118
diff -u -p -u -r1.118 parse.c
--- parse.c 23 Oct 2016 14:54:14 -  1.118
+++ parse.c 10 Jul 2017 09:02:35 -
@@ -1427,6 +1427,7 @@ register_target(GNode *gn, struct ohash 
 static void
 build_target_group(struct growableArray *targets, struct ohash *t)
 {
+   GNode *gn, *gn2 = NULL;
LstNode ln;
bool seen_target = false;
unsigned int i;
@@ -1459,7 +1460,6 @@ build_target_group(struct growableArray 
if (seen_target)
return;
 
-   GNode *gn, *gn2;
/* targets may already participate in groupling lists, 
 * so rebuild the circular list "from scratch"
 */

bc makefile tweak

[Michael W. Bombardieri]

Hi,

When building bc, yacc can directly write bc.c instead of renaming
the file with mv. Does this look any better?

- Michael


Index: Makefile
===
RCS file: /cvs/src/usr.bin/bc/Makefile,v
retrieving revision 1.9
diff -u -p -u -r1.9 Makefile
--- Makefile3 Jul 2017 00:17:52 -   1.9
+++ Makefile4 Jul 2017 02:50:46 -
@@ -10,11 +10,10 @@ DPADD+= ${LIBEDIT} ${LIBCURSES} ${LIBCR
 
 .PATH: ${.CURDIR}/../dc
 
-bc.c y.tab.h: bc.y
-   ${YACC} -d ${.CURDIR}/bc.y
-   mv y.tab.c bc.c
+y.tab.h: bc.y
+   ${YACC} -o ${.CURDIR}/bc.c ${.CURDIR}/bc.y
 
-scan.o: y.tab.h
+bc.c scan.o: y.tab.h
 
 beforeinstall:
install -c -o ${BINOWN} -g ${BINGRP} -m 444 ${.CURDIR}/bc.library \

cdio read_track() malloc/free

[Michael W. Bombardieri]

Hello,

In cdio's read_track() the error condition around sio_write() doesn't free 
local storage "sec"
before it returns. The other error cases appear to free it.

- Michael


Index: rip.c
===
RCS file: /cvs/src/usr.bin/cdio/rip.c,v
retrieving revision 1.16
diff -u -p -u -r1.16 rip.c
--- rip.c   20 Aug 2015 22:32:41 -  1.16
+++ rip.c   19 Jun 2017 03:27:14 -
@@ -398,6 +398,7 @@ read_track(struct track *ti)
}
if (ti->hdl != NULL &&
(sio_write(ti->hdl, sec, blksize) == 0)) {
+   free(sec);
sio_close(ti->hdl);
ti->hdl = NULL;
warnx("\nerror while writing to audio output");

lex scanopt_destroy()

[Michael W. Bombardieri]

Hi,

In lex the return value of scanopt_destroy() is never used. 
This function is a wrapper for free(), and free() has no value to return.

- Michael


Index: scanopt.c
===
RCS file: /cvs/src/usr.bin/lex/scanopt.c,v
retrieving revision 1.5
diff -u -p -u -r1.5 scanopt.c
--- scanopt.c   11 Dec 2015 00:08:43 -  1.5
+++ scanopt.c   31 May 2017 02:23:18 -
@@ -857,7 +857,7 @@ int scanopt (svoid, arg, optindex)
 }
 
 
-int scanopt_destroy (svoid)
+void scanopt_destroy (svoid)
  scanopt_t *svoid;
 {
struct _scanopt_t *s;
@@ -867,5 +867,4 @@ int scanopt_destroy (svoid)
free(s->aux);
free (s);
}
-   return 0;
 }
Index: scanopt.h
===
RCS file: /cvs/src/usr.bin/lex/scanopt.h,v
retrieving revision 1.2
diff -u -p -u -r1.2 scanopt.h
--- scanopt.h   19 Nov 2015 22:16:43 -  1.2
+++ scanopt.h   31 May 2017 02:23:18 -
@@ -90,9 +90,8 @@ extern  "C" {
scanopt_t *scanopt_init PROTO ((const optspec_t * options,
int argc, char **argv, int flags));
 
-/* Frees memory used by scanner.
- * Always returns 0. */
-   int scanopt_destroy PROTO ((scanopt_t * scanner));
+/* Frees memory used by scanner. */
+   void scanopt_destroy PROTO ((scanopt_t * scanner));
 
 #ifndef NO_SCANOPT_USAGE
 /* Prints a usage message based on contents of optlist.

Re: uaudio_drain() not needed?

[Michael W. Bombardieri]

On Mon, Apr 24, 2017 at 08:29:17AM +0200, Alexandre Ratchov wrote:
> On Mon, Apr 24, 2017 at 01:04:12AM +0800, Michael W. Bombardieri wrote:
> > Hi,
> > 
> > When I remove uaudio_drain() on my laptop the attach/detach still
> > seems to work as expected.
> > I did a test with two usb soundcards. Audio files were played &
> > recorded using aucat.
> > 
> > * boot system (no audio device because I disabled azalia)
> > * attach device1 (Creative card)
> > * play wav file1 (reference file)
> > * detach device1
> > * attach device2 (Yamaha card)
> > * play wav file1
> > * record wav file2
> > * detach device2
> > * attach device2
> > * play wav file2
> > * detach device2
> > * attach device1
> > * play wav file2
> > * detach device1
> > 
> > So far this has only been tested on amd64. Maybe it produces
> > issues for your uaudio device though.
> > 
> 
> AFAICS this is correct.  uaudio_drain() used to be the "drain"
> method, but the audio(4) layer doesn't use it anymore.  IMO, it
> shouldn't be called by uaudio_detach(), the device is gone and
> there are no outstanding requests.

FWIW I have now tested this diff on i386. Everything worked ok.
One test I neglected earlier was to physically disconnect usb
audio device while playing or recording with aucat--that also
worked.

Re: marco's plaintext history patch

[Michael W. Bombardieri]

On Fri, Apr 28, 2017 at 10:01:06AM +0200, Theo Buehler wrote:
> This is an updated version of an old patch by marco [1], including some
> improvements and fixes by natano and me. Here's marco's description:
> 
> > I have had enough of corrupt ksh history so I had a look at the code to
> > try to fix it.  The magical code was very magical so I basically deleted
> > most of it and made ksh history into a flat text file.  It handles
> > multiple ksh instances writing to the same text file with locks just
> > like the current ksh does.  I haven't noticed any differences in
> > behavior running this.
> 
> If you wish to retain your current ksh history, you can create a
> plaintext version of it using a command like this (assuming HISTFILE and
> HISTSIZE are set): 
> 
>   cp $HISTFILE $HISTFILE.old
>   fc -ln 1 | sed 's/^ //' > ~/ksh_hist.txt
> 
> Then apply the patch and install the new ksh.
> 
> Once you've exited all interactive sessions with the old ksh (a reboot is
> probably your safest bet), you can use ksh_hist.txt as your history file.
> 
> Note that the old ksh recklessly truncates a history file that it
> doesn't understand, so be careful not to run old and new interactive ksh
> processes in parallel.
> 
> [1]: https://marc.info/?l=openbsd-tech=131490865525379=2
> 

In history_load() is there a reason for casting line?

+   histsave(line_co, (char *)line, 0);

vi: cs_next() return value

[Michael W. Bombardieri]

Hello,

In vi, cs_next() always returns zero so don't check its return value.
cs.cs_flags is always checked afterwards; cs.cs_flags seems to be
the effective return value.

I didn't change two invocations in v_word.c where cs_next() is 2nd
operand in && expression and might be skipped.

- Michael


Index: vi/getc.c
===
RCS file: /cvs/src/usr.bin/vi/vi/getc.c,v
retrieving revision 1.10
diff -u -p -u -r1.10 getc.c
--- vi/getc.c   6 Jan 2016 22:28:52 -   1.10
+++ vi/getc.c   27 Apr 2017 03:20:35 -
@@ -123,8 +123,7 @@ cs_fspace(SCR *sp, VCS *csp)
if (csp->cs_flags != 0 || !isblank(csp->cs_ch))
return (0);
for (;;) {
-   if (cs_next(sp, csp))
-   return (1);
+   cs_next(sp, csp);
if (csp->cs_flags != 0 || !isblank(csp->cs_ch))
break;
}
@@ -141,8 +140,7 @@ int
 cs_fblank(SCR *sp, VCS *csp)
 {
for (;;) {
-   if (cs_next(sp, csp))
-   return (1);
+   cs_next(sp, csp);
if (csp->cs_flags == CS_EOL || csp->cs_flags == CS_EMP ||
(csp->cs_flags == 0 && isblank(csp->cs_ch)))
continue;
Index: vi/v_sentence.c
===
RCS file: /cvs/src/usr.bin/vi/vi/v_sentence.c,v
retrieving revision 1.7
diff -u -p -u -r1.7 v_sentence.c
--- vi/v_sentence.c 12 Nov 2014 04:28:41 -  1.7
+++ vi/v_sentence.c 27 Apr 2017 03:20:35 -
@@ -82,14 +82,12 @@ v_sentencef(SCR *sp, VICMD *vp)
}
 
for (state = NONE;;) {
-   if (cs_next(sp, ))
-   return (1);
+   cs_next(sp, );
if (cs.cs_flags == CS_EOF)
break;
if (cs.cs_flags == CS_EOL) {
if ((state == PERIOD || state == BLANK) && --cnt == 0) {
-   if (cs_next(sp, ))
-   return (1);
+   cs_next(sp, );
if (cs.cs_flags == 0 &&
isblank(cs.cs_ch) && cs_fblank(sp, ))
return (1);
@@ -273,8 +271,7 @@ ret:slno = cs.cs_lno;
 * and special characters.
 */
do {
-   if (cs_next(sp, ))
-   return (1);
+   cs_next(sp, );
} while (!cs.cs_flags &&
(cs.cs_ch == ')' || cs.cs_ch == ']' ||
cs.cs_ch == '"' || cs.cs_ch == '\''));
Index: vi/v_word.c
===
RCS file: /cvs/src/usr.bin/vi/vi/v_word.c,v
retrieving revision 1.7
diff -u -p -u -r1.7 v_word.c
--- vi/v_word.c 12 Nov 2014 04:28:41 -  1.7
+++ vi/v_word.c 27 Apr 2017 03:20:35 -
@@ -140,8 +140,7 @@ fword(SCR *sp, VICMD *vp, enum which typ
if (type == BIGWORD)
while (cnt--) {
for (;;) {
-   if (cs_next(sp, ))
-   return (1);
+   cs_next(sp, );
if (cs.cs_flags == CS_EOF)
goto ret;
if (cs.cs_flags != 0 || isblank(cs.cs_ch))
@@ -172,8 +171,7 @@ fword(SCR *sp, VICMD *vp, enum which typ
state = cs.cs_flags == 0 &&
inword(cs.cs_ch) ? INWORD : NOTWORD;
for (;;) {
-   if (cs_next(sp, ))
-   return (1);
+   cs_next(sp, );
if (cs.cs_flags == CS_EOF)
goto ret;
if (cs.cs_flags != 0 || isblank(cs.cs_ch))
@@ -276,8 +274,7 @@ eword(SCR *sp, VICMD *vp, enum which typ
 * past the current one, it sets word "state" for the 'e' command.
 */
if (cs.cs_flags == 0 && !isblank(cs.cs_ch)) {
-   if (cs_next(sp, ))
-   return (1);
+   cs_next(sp, );
if (cs.cs_flags == 0 && !isblank(cs.cs_ch))
goto start;
}
@@ -293,8 +290,7 @@ eword(SCR *sp, VICMD *vp, enum which typ
 start: if (type == BIGWORD)
while (cnt--) {
for (;;) {
-   if (cs_next(sp, ))
-   return (1);
+   cs_next(sp, );
if (cs.cs_flags == CS_EOF)
goto ret;
   

uaudio_drain() not needed?

[Michael W. Bombardieri]

Hi,

When I remove uaudio_drain() on my laptop the attach/detach still
seems to work as expected.
I did a test with two usb soundcards. Audio files were played &
recorded using aucat.

* boot system (no audio device because I disabled azalia)
* attach device1 (Creative card)
* play wav file1 (reference file)
* detach device1
* attach device2 (Yamaha card)
* play wav file1
* record wav file2
* detach device2
* attach device2
* play wav file2
* detach device2
* attach device1
* play wav file2
* detach device1

So far this has only been tested on amd64. Maybe it produces
issues for your uaudio device though.

- Michael


Index: uaudio.c
===
RCS file: /cvs/src/sys/dev/usb/uaudio.c,v
retrieving revision 1.126
diff -u -p -u -r1.126 uaudio.c
--- uaudio.c8 Apr 2017 02:57:25 -   1.126
+++ uaudio.c23 Apr 2017 15:34:53 -
@@ -371,7 +371,6 @@ voiduaudio_chan_rintr
 
 intuaudio_open(void *, int);
 void   uaudio_close(void *);
-intuaudio_drain(void *);
 void   uaudio_get_minmax_rates
(int, const struct as_info *, const struct audio_params *,
 int, int, int, u_long *, u_long *);
@@ -563,7 +562,6 @@ int
 uaudio_detach(struct device *self, int flags)
 {
struct uaudio_softc *sc = (struct uaudio_softc *)self;
-   int rv = 0;
 
/*
 * sc_alts may be NULL if uaudio_identify_as() failed, in
@@ -571,14 +569,8 @@ uaudio_detach(struct device *self, int f
 * nothing to detach.
 */
if (sc->sc_alts == NULL)
-   return (rv);
-
-   /* Wait for outstanding requests to complete. */
-   uaudio_drain(sc);
-
-   rv = config_detach_children(self, flags);
-
-   return (rv);
+   return (0);
+   return (config_detach_children(self, flags));
 }
 
 const usb_interface_descriptor_t *
@@ -2107,24 +2099,6 @@ uaudio_close(void *addr)
uaudio_chan_close(sc, >sc_playchan);
if (sc->sc_recchan.altidx != -1)
uaudio_chan_close(sc, >sc_recchan);
-}
-
-int
-uaudio_drain(void *addr)
-{
-   struct uaudio_softc *sc = addr;
-   struct chan *pchan = >sc_playchan;
-   struct chan *rchan = >sc_recchan;
-   int ms = 0;
-
-   /* Wait for outstanding requests to complete. */
-   if (pchan->altidx != -1 && sc->sc_alts[pchan->altidx].sc_busy)
-   ms = max(ms, pchan->reqms);
-   if (rchan->altidx != -1 && sc->sc_alts[rchan->altidx].sc_busy)
-   ms = max(ms, rchan->reqms);
-   usbd_delay_ms(sc->sc_udev, UAUDIO_NCHANBUFS * ms);
-
-   return (0);
 }
 
 int

make.1 spellcheck

[Michael W. Bombardieri]

Hi,

Spellchecker found two non-dictionary words in the make(1) manual.

- Michael


Index: make.1
===
RCS file: /cvs/src/usr.bin/make/make.1,v
retrieving revision 1.124
diff -u -p -u -r1.124 make.1
--- make.1  1 Jan 2017 01:08:11 -   1.124
+++ make.1  19 Apr 2017 07:06:51 -
@@ -209,7 +209,7 @@ is defined,
 will wait between 0 and ${RANDOM_DELAY} seconds before starting a command.
 A given random seed can be forced by setting
 .Va RANDOM_SEED ,
-but this does not guarantee reproductibility.
+but this does not guarantee reproducibility.
 .It Ar q
 .Sq quick death
 option: after a fatal error, instead of waiting for other jobs to die,
@@ -1342,7 +1342,7 @@ recognizes standard special targets:
 .It Ic .DEFAULT
 If there is a
 .Ic .DEFAULT
-target rule, with commands but no prequisites, and
+target rule, with commands but no prerequisites, and
 .Nm
 can't figure out another way to build a target, it will use that
 list of commands, setting

vi unused variables

[Michael W. Bombardieri]

Hi,

Some unused variables in vi were identified by gcc 4.9.4.

- Michael


Index: common/recover.c
===
RCS file: /cvs/src/usr.bin/vi/common/recover.c,v
retrieving revision 1.25
diff -u -p -u -r1.25 recover.c
--- common/recover.c29 Jun 2016 20:38:39 -  1.25
+++ common/recover.c19 Apr 2017 02:45:32 -
@@ -313,7 +313,6 @@ static int
 rcv_mailfile(SCR *sp, int issync, char *cp_path)
 {
EXF *ep;
-   GS *gp;
struct passwd *pw;
size_t len;
time_t now;
@@ -323,7 +322,6 @@ rcv_mailfile(SCR *sp, int issync, char *
char *t1, *t2, *t3;
char host[HOST_NAME_MAX+1];
 
-   gp = sp->gp;
if ((pw = getpwuid(uid = getuid())) == NULL) {
msgq(sp, M_ERR,
"Information on user id %u not found", uid);
Index: ex/ex_bang.c
===
RCS file: /cvs/src/usr.bin/vi/ex/ex_bang.c,v
retrieving revision 1.11
diff -u -p -u -r1.11 ex_bang.c
--- ex/ex_bang.c18 Apr 2017 01:45:35 -  1.11
+++ ex/ex_bang.c19 Apr 2017 02:45:32 -
@@ -52,7 +52,6 @@ ex_bang(SCR *sp, EXCMD *cmdp)
EX_PRIVATE *exp;
MARK rm;
recno_t lno;
-   int rval;
const char *msg;
 
ap = cmdp->argv[0];
@@ -145,7 +144,7 @@ ex_bang(SCR *sp, EXCMD *cmdp)
ftype = FILTER_RBANG;
}
}
-   rval = ex_filter(sp, cmdp,
+   (void)ex_filter(sp, cmdp,
>addr1, >addr2, , ap->bp, ftype);
 
/*
Index: ex/ex_global.c
===
RCS file: /cvs/src/usr.bin/vi/ex/ex_global.c,v
retrieving revision 1.17
diff -u -p -u -r1.17 ex_global.c
--- ex/ex_global.c  27 May 2016 09:18:12 -  1.17
+++ ex/ex_global.c  19 Apr 2017 02:45:32 -
@@ -67,7 +67,6 @@ ex_g_setup(SCR *sp, EXCMD *cmdp, enum wh
RANGE *rp;
busy_t btype;
recno_t start, end;
-   regex_t *re;
regmatch_t match[1];
size_t len;
int cnt, delim, eval;
@@ -146,7 +145,6 @@ usage:  ex_emsg(sp, cmdp->cmd->usage, EX
 */
sp->searchdir = FORWARD;
}
-   re = >re_c;
 
/* The global commands always set the previous context mark. */
abs_mark.lno = sp->lno;
Index: vi/vs_split.c
===
RCS file: /cvs/src/usr.bin/vi/vi/vs_split.c,v
retrieving revision 1.16
diff -u -p -u -r1.16 vs_split.c
--- vi/vs_split.c   27 May 2016 09:18:12 -  1.16
+++ vi/vs_split.c   19 Apr 2017 02:45:32 -
@@ -458,11 +458,8 @@ vs_swap(SCR *sp, SCR **nspp, char *name)
 int
 vs_resize(SCR *sp, long count, adj_t adj)
 {
-   GS *gp;
SCR *g, *s;
size_t g_off, s_off;
-
-   gp = sp->gp;
 
/*
 * Figure out which screens will grow, which will shrink, and

vscsi_t2i() unused variable

[Michael W. Bombardieri]

Hello,

The variable rv in vscsi_t2i() appears to always be zero
and could be removed.

- Michael


Index: src/sys/dev/vscsi.c
===
RCS file: /cvs/src/sys/dev/vscsi.c,v
retrieving revision 1.41
diff -u -p -u -r1.41 vscsi.c
--- src/sys/dev/vscsi.c 12 Feb 2017 17:12:37 -  1.41
+++ src/sys/dev/vscsi.c 30 Mar 2017 08:16:27 -
@@ -429,7 +429,6 @@ vscsi_t2i(struct vscsi_softc *sc, struct
 {
struct vscsi_ccb*ccb;
struct scsi_xfer*xs;
-   int rv = 0;
 
TAILQ_FOREACH(ccb, >sc_ccb_t2i, ccb_entry) {
if (ccb->ccb_tag == t2i->tag)
@@ -464,7 +463,7 @@ vscsi_t2i(struct vscsi_softc *sc, struct
 
vscsi_done(sc, ccb);
 
-   return (rv);
+   return (0);
 }
 
 struct vscsi_devevent_task {

vi NULL check before free()

[Michael W. Bombardieri]

Hi,

Remove some checks for NULL around free() in vi.
vi still sees to work when I build it on i386 and amd64.

- Michael


Index: cl/cl_screen.c
===
RCS file: /cvs/src/usr.bin/vi/cl/cl_screen.c,v
retrieving revision 1.27
diff -u -p -u -r1.27 cl_screen.c
--- cl/cl_screen.c  28 May 2016 18:30:35 -  1.27
+++ cl/cl_screen.c  16 Apr 2017 13:06:45 -
@@ -434,14 +434,10 @@ cl_ex_init(SCR *sp)
 
/* Enter_standout_mode and exit_standout_mode are paired. */
if (clp->smso == NULL || clp->rmso == NULL) {
-   if (clp->smso != NULL) {
-   free(clp->smso);
-   clp->smso = NULL;
-   }
-   if (clp->rmso != NULL) {
-   free(clp->rmso);
-   clp->rmso = NULL;
-   }
+   free(clp->smso);
+   clp->smso = NULL;
+   free(clp->rmso);
+   clp->rmso = NULL;
}
 
/*
@@ -515,26 +511,16 @@ cl_getcap(SCR *sp, char *name, char **el
 static void
 cl_freecap(CL_PRIVATE *clp)
 {
-   if (clp->el != NULL) {
-   free(clp->el);
-   clp->el = NULL;
-   }
-   if (clp->cup != NULL) {
-   free(clp->cup);
-   clp->cup = NULL;
-   }
-   if (clp->cuu1 != NULL) {
-   free(clp->cuu1);
-   clp->cuu1 = NULL;
-   }
-   if (clp->rmso != NULL) {
-   free(clp->rmso);
-   clp->rmso = NULL;
-   }
-   if (clp->smso != NULL) {
-   free(clp->smso);
-   clp->smso = NULL;
-   }
+   free(clp->el);
+   clp->el = NULL;
+   free(clp->cup);
+   clp->cup = NULL;
+   free(clp->cuu1);
+   clp->cuu1 = NULL;
+   free(clp->rmso);
+   clp->rmso = NULL;
+   free(clp->smso);
+   clp->smso = NULL;
 }
 
 /*
Index: common/cut.c
===
RCS file: /cvs/src/usr.bin/vi/common/cut.c,v
retrieving revision 1.16
diff -u -p -u -r1.16 cut.c
--- common/cut.c27 May 2016 09:18:11 -  1.16
+++ common/cut.c16 Apr 2017 13:06:45 -
@@ -343,7 +343,6 @@ text_lfree(TEXTH *headp)
 void
 text_free(TEXT *tp)
 {
-   if (tp->lb != NULL)
-   free(tp->lb);
+   free(tp->lb);
free(tp);
 }
Index: common/exf.c
===
RCS file: /cvs/src/usr.bin/vi/common/exf.c,v
retrieving revision 1.44
diff -u -p -u -r1.44 exf.c
--- common/exf.c1 Aug 2016 18:27:35 -   1.44
+++ common/exf.c16 Apr 2017 13:06:46 -
@@ -76,8 +76,7 @@ file_add(SCR *sp, CHAR_T *name)
TAILQ_FOREACH_SAFE(frp, >frefq, q, tfrp) {
if (frp->name == NULL) {
TAILQ_REMOVE(>frefq, frp, q);
-   if (frp->name != NULL)
-   free(frp->name);
+   free(frp->name);
free(frp);
continue;
}
@@ -197,8 +196,7 @@ file_init(SCR *sp, FREF *frp, char *rcv_
F_SET(frp, FR_TMPFILE);
if ((frp->tname = strdup(tname)) == NULL ||
(frp->name == NULL && (frp->name = strdup(tname)) == NULL)) 
{
-   if (frp->tname != NULL)
-   free(frp->tname);
+   free(frp->tname);
msgq(sp, M_SYSERR, NULL);
(void)unlink(tname);
goto err;
@@ -410,10 +408,9 @@ file_init(SCR *sp, FREF *frp, char *rcv_
 
return (0);
 
-err:   if (frp->name != NULL) {
-   free(frp->name);
-   frp->name = NULL;
-   }
+err:
+   free(frp->name);
+   frp->name = NULL;
if (frp->tname != NULL) {
(void)unlink(frp->tname);
free(frp->tname);
@@ -422,10 +419,8 @@ err:   if (frp->name != NULL) {
 
 oerr:  if (F_ISSET(ep, F_RCV_ON))
(void)unlink(ep->rcv_path);
-   if (ep->rcv_path != NULL) {
-   free(ep->rcv_path);
-   ep->rcv_path = NULL;
-   }
+   free(ep->rcv_path);
+   ep->rcv_path = NULL;
if (ep->db != NULL)
(void)ep->db->close(ep->db);
free(ep);
@@ -659,8 +654,7 @@ file_end(SCR *sp, EXF *ep, int force)
frp->tname = NULL;
if (F_ISSET(frp, FR_TMPFILE)) {
TAILQ_REMOVE(>gp->frefq, frp, q);
-   if (frp->name != NULL)
-   free(frp->name);
+   free(frp->name);
free(frp);
}
sp->frp = NULL;
@@ -704,11 +698,8 @@ file_end(SCR *sp, EXF *ep, int force)

yacc: y.tab.c remains on error

[Michael W. Bombardieri]

Hi,

yacc will produce a y.tab.c file when it is fed input with bad
syntax. Maybe this is expected behaviour but is it better for
yacc to clean up after itself? When I feed the same badsyntax.y
to bison 3.0.4 it doesn't create y.tab.c.

yacc creates y.tab.c very early. Unlink the file in
done() if it's heading for exit with non-zero status?
As a result y.tab.c is removed for other errors, such as 
no_space() which means out of memory.

- Michael


Index: main.c
===
RCS file: /cvs/src/usr.bin/yacc/main.c,v
retrieving revision 1.28
diff -u -p -u -r1.28 main.c
--- main.c  27 Jul 2016 20:53:47 -  1.28
+++ main.c  16 Apr 2017 09:29:03 -
@@ -109,6 +109,8 @@ volatile sig_atomic_t sigdie;
 void
 done(int k)
 {
+   if (k != 0)
+   unlink(output_file_name);
if (action_file)
unlink(action_file_name);
if (text_file)

vmm/amd64 __func__

[Michael W. Bombardieri]

Hi,

This patch is a continuation of
https://marc.info/?l=openbsd-tech=149214401819485=2

* Make __func__ usage more consistent in vmm/amd64
* Typo cant->can't in the following error messages...

vcpu_run_vmx: cant read exit reason
vmm_get_exit_qualification: cant extract exit qual

* One error message was missing space in string continuation:

vcpu_run_vmx: can't readprocbased ctls on exit

- Michael


Index: vmm.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/vmm.c,v
retrieving revision 1.132
diff -u -p -u -r1.132 vmm.c
--- vmm.c   2 Apr 2017 20:21:44 -   1.132
+++ vmm.c   14 Apr 2017 08:18:03 -
@@ -427,7 +427,7 @@ vmmioctl(dev_t dev, u_long cmd, caddr_t 
ret = vm_rwregs((struct vm_rwregs_params *)data, 1);
break;
default:
-   DPRINTF("vmmioctl: unknown ioctl code 0x%lx\n", cmd);
+   DPRINTF("%s: unknown ioctl code 0x%lx\n", __func__, cmd);
ret = ENOTTY;
}
 
@@ -502,7 +502,7 @@ vm_resetcpu(struct vm_resetcpu_params *v
 
/* Not found? exit. */
if (error != 0) {
-   DPRINTF("vm_resetcpu: vm id %u not found\n",
+   DPRINTF("%s: vm id %u not found\n", __func__,
vrp->vrp_vm_id);
return (error);
}
@@ -515,25 +515,25 @@ vm_resetcpu(struct vm_resetcpu_params *v
rw_exit_read(>vm_vcpu_lock);
 
if (vcpu == NULL) {
-   DPRINTF("vm_resetcpu: vcpu id %u of vm %u not found\n",
-   vrp->vrp_vcpu_id, vrp->vrp_vm_id);
+   DPRINTF("%s: vcpu id %u of vm %u not found\n",
+   __func__, vrp->vrp_vcpu_id, vrp->vrp_vm_id);
return (ENOENT);
}
 
if (vcpu->vc_state != VCPU_STATE_STOPPED) {
-   DPRINTF("vm_resetcpu: reset of vcpu %u on vm %u attempted "
-   "while vcpu was in state %u (%s)\n", vrp->vrp_vcpu_id,
+   DPRINTF("%s: reset of vcpu %u on vm %u attempted "
+   "while vcpu was in state %u (%s)\n", __func__, 
vrp->vrp_vcpu_id,
vrp->vrp_vm_id, vcpu->vc_state,
vcpu_state_decode(vcpu->vc_state));

return (EBUSY);
}
 
-   DPRINTF("vm_resetcpu: resetting vm %d vcpu %d to power on defaults\n",
-   vm->vm_id, vcpu->vc_id);
+   DPRINTF("%s: resetting vm %d vcpu %d to power on defaults\n",
+   __func__, vm->vm_id, vcpu->vc_id);
 
if (vcpu_reset_regs(vcpu, >vrp_init_state)) {
-   printf("vm_resetcpu: failed\n");
+   printf("%s: failed\n", __func__);
 #ifdef VMM_DEBUG
dump_vcpu(vcpu);
 #endif /* VMM_DEBUG */
@@ -1095,7 +1095,7 @@ vm_impl_init_vmx(struct vm *vm, struct p
/* Create a new pmap for this VM */
pmap = pmap_create();
if (!pmap) {
-   printf("vm_impl_init_vmx: pmap_create failed\n");
+   printf("%s: pmap_create failed\n", __func__);
return (ENOMEM);
}
 
@@ -,20 +,20 @@ vm_impl_init_vmx(struct vm *vm, struct p
VM_MAP_ISVMSPACE | VM_MAP_PAGEABLE);
 
if (!vm->vm_map) {
-   printf("vm_impl_init_vmx: uvm_map_create failed\n");
+   printf("%s: uvm_map_create failed\n", __func__);
pmap_destroy(pmap);
return (ENOMEM);
}
 
/* Map the new map with an anon */
-   DPRINTF("vm_impl_init_vmx: created vm_map @ %p\n", vm->vm_map);
+   DPRINTF("%s: created vm_map @ %p\n", __func__, vm->vm_map);
for (i = 0; i < vm->vm_nmemranges; i++) {
vmr = >vm_memranges[i];
ret = uvm_share(vm->vm_map, vmr->vmr_gpa,
PROT_READ | PROT_WRITE | PROT_EXEC,
>p_vmspace->vm_map, vmr->vmr_va, vmr->vmr_size);
if (ret) {
-   printf("vm_impl_init_vmx: uvm_share failed (%d)\n",
+   printf("%s: uvm_share failed (%d)\n", __func__,
ret);
/* uvm_map_deallocate calls pmap_destroy for us */
uvm_map_deallocate(vm->vm_map);
@@ -1136,7 +1136,7 @@ vm_impl_init_vmx(struct vm *vm, struct p
/* Convert the low 512GB of the pmap to EPT */
ret = pmap_convert(pmap, PMAP_TYPE_EPT);
if (ret) {
-   printf("vm_impl_init_vmx: pmap_convert failed\n");
+   printf("%s: pmap_convert failed\n", __func__);
/* uvm_map_deallocate calls pmap_destroy for us */
uvm_map_deallocate(vm->vm_map);
vm->vm_map = NULL;
@@ -1174,7 +1174,7 @@ vm_impl_init_svm(struct vm *vm, struct p
/* Create a new pmap for this VM */
pmap = pmap_create();
if (!pmap) {
-   printf("vm_impl_init_svm: pmap_create failed\n");
+   printf("%s: 

vmm/i386 __func__

[Michael W. Bombardieri]

Hi,

Some printf() statements in vmm.c already used __func__
but some didn't. This diff adds more __func__.

Also, one printf() statement was missing a space:
   "vcpu_run_vmx: can't readprocbased ctls on exit"

- Michael


Index: vmm.c
===
RCS file: /cvs/src/sys/arch/i386/i386/vmm.c,v
retrieving revision 1.27
diff -u -p -u -r1.27 vmm.c
--- vmm.c   12 Apr 2017 05:46:59 -  1.27
+++ vmm.c   14 Apr 2017 04:11:33 -
@@ -540,7 +540,7 @@ vm_resetcpu(struct vm_resetcpu_params *v
vm->vm_id, vcpu->vc_id);
 
if (vcpu_reset_regs(vcpu, >vrp_init_state)) {
-   printf("vm_resetcpu: failed\n");
+   printf("%s: failed\n", __func__);
 #ifdef VMM_DEBUG
dump_vcpu(vcpu);
 #endif /* VMM_DEBUG */
@@ -1102,7 +1102,7 @@ vm_impl_init_vmx(struct vm *vm, struct p
/* Create a new pmap for this VM */
pmap = pmap_create();
if (!pmap) {
-   printf("vm_impl_init_vmx: pmap_create failed\n");
+   printf("%s: pmap_create failed\n", __func__);
return (ENOMEM);
}
 
@@ -1118,7 +1118,7 @@ vm_impl_init_vmx(struct vm *vm, struct p
VM_MAP_ISVMSPACE | VM_MAP_PAGEABLE);
 
if (!vm->vm_map) {
-   printf("vm_impl_init_vmx: uvm_map_create failed\n");
+   printf("%s: uvm_map_create failed\n", __func__);
pmap_destroy(pmap);
return (ENOMEM);
}
@@ -1131,7 +1131,7 @@ vm_impl_init_vmx(struct vm *vm, struct p
PROT_READ | PROT_WRITE | PROT_EXEC,
>p_vmspace->vm_map, vmr->vmr_va, vmr->vmr_size);
if (ret) {
-   printf("vm_impl_init_vmx: uvm_share failed (%d)\n",
+   printf("%s: uvm_share failed (%d)\n", __func__,
ret);
/* uvm_map_deallocate calls pmap_destroy for us */
uvm_map_deallocate(vm->vm_map);
@@ -1143,7 +1143,7 @@ vm_impl_init_vmx(struct vm *vm, struct p
/* Convert the low 512GB of the pmap to EPT */
ret = pmap_convert(pmap, PMAP_TYPE_EPT);
if (ret) {
-   printf("vm_impl_init_vmx: pmap_convert failed\n");
+   printf("%s: pmap_convert failed\n", __func__);
/* uvm_map_deallocate calls pmap_destroy for us */
uvm_map_deallocate(vm->vm_map);
vm->vm_map = NULL;
@@ -1181,7 +1181,7 @@ vm_impl_init_svm(struct vm *vm, struct p
/* Create a new pmap for this VM */
pmap = pmap_create();
if (!pmap) {
-   printf("vm_impl_init_svm: pmap_create failed\n");
+   printf("%s: pmap_create failed\n", __func__);
return (ENOMEM);
}
 
@@ -1199,7 +1199,7 @@ vm_impl_init_svm(struct vm *vm, struct p
VM_MAP_ISVMSPACE | VM_MAP_PAGEABLE);
 
if (!vm->vm_map) {
-   printf("vm_impl_init_svm: uvm_map_create failed\n");
+   printf("%s: uvm_map_create failed\n", __func__);
pmap_destroy(pmap);
return (ENOMEM);
}
@@ -1212,7 +1212,7 @@ vm_impl_init_svm(struct vm *vm, struct p
PROT_READ | PROT_WRITE | PROT_EXEC,
>p_vmspace->vm_map, vmr->vmr_va, vmr->vmr_size);
if (ret) {
-   printf("vm_impl_init_svm: uvm_share failed (%d)\n",
+   printf("%s: uvm_share failed (%d)\n", __func__,
ret);
/* uvm_map_deallocate calls pmap_destroy for us */
uvm_map_deallocate(vm->vm_map);
@@ -3447,8 +3447,8 @@ vcpu_run_vmx(struct vcpu *vcpu, struct v
/* Is there an interrupt pending injection? */
if (irq != 0x) {
if (!vcpu->vc_irqready) {
-   printf("vcpu_run_vmx: error - irq injected"
-   " while not ready\n");
+   printf("%s: error - irq injected"
+   " while not ready\n", __func__);
ret = EINVAL;
break;
}
@@ -3457,8 +3457,8 @@ vcpu_run_vmx(struct vcpu *vcpu, struct v
eii |= (1ULL << 31);/* Valid */
eii |= (0ULL << 8); /* Hardware Interrupt */
if (vmwrite(VMCS_ENTRY_INTERRUPTION_INFO, eii)) {
-   printf("vcpu_run_vmx: can't vector "
-   "interrupt to guest\n");
+   printf("%s: can't vector "
+   "interrupt to guest\n", __func__);
ret = EINVAL;
break;
}
@@ -3469,15 +3469,15 @@ vcpu_run_vmx(struct 

Re: vmm/i386 VMM_DEBUG

[Michael W. Bombardieri]

Works for me here.

On Tue, Apr 11, 2017 at 09:35:13PM -0700, Philip Guenther wrote:
> On Wed, 12 Apr 2017, Michael W. Bombardieri wrote:
> > Building with VMM_DEBUG enabled failed because a printf() warning was 
> > treated as an error.
> ...
> > DPRINTF("%s: function 0x07 (SEFF) unsupported subleaf "
> > -   "0x%llx not supported\n", __func__, *ecx);
> > +   "0x%lx not supported\n", __func__,
> > +   (unsigned long)*ecx);
> 
> Hmm, uint32_t is expected to be unsigned int in other printf formats in 
> the file (and elsewhere in the kernel), so I think the cast is unnecessary 
> here and we can just go with:
> 
> --- vmm.c 25 Mar 2017 22:24:01 -  1.26
> +++ vmm.c 12 Apr 2017 04:33:04 -
> @@ -4353,7 +4353,7 @@ vmm_handle_cpuid(struct vcpu *vcpu)
>   } else {
>   /* Unsupported subleaf */
>   DPRINTF("%s: function 0x07 (SEFF) unsupported subleaf "
> - "0x%llx not supported\n", __func__, *ecx);
> + "0x%x not supported\n", __func__, *ecx);
>   *eax = 0;
>   *ebx = 0;
>   *ecx = 0;
> 

vmm/i386 VMM_DEBUG

[Michael W. Bombardieri]

Hi,

Building with VMM_DEBUG enabled failed because a printf() warning
was treated as an error.

- Michael


cc1: warnings being treated as errors
/usr/src/src/sys/arch/i386/i386/vmm.c: In function 'vmm_handle_cpuid':
/usr/src/src/sys/arch/i386/i386/vmm.c:4355: warning: format '%llx' expects type 
'long long unsigned int', but argument 3 has type 'uint32_t'
*** Error 1 in /usr/src/src/sys/arch/i386/compile/GENERIC (Makefile:1021 
'vmm.o')

Index: vmm.c
===
RCS file: /cvs/src/sys/arch/i386/i386/vmm.c,v
retrieving revision 1.26
diff -u -p -u -r1.26 vmm.c
--- vmm.c   25 Mar 2017 22:24:01 -  1.26
+++ vmm.c   12 Apr 2017 05:00:03 -
@@ -4353,7 +4353,8 @@ vmm_handle_cpuid(struct vcpu *vcpu)
} else {
/* Unsupported subleaf */
DPRINTF("%s: function 0x07 (SEFF) unsupported subleaf "
-   "0x%llx not supported\n", __func__, *ecx);
+   "0x%lx not supported\n", __func__,
+   (unsigned long)*ecx);
*eax = 0;
*ebx = 0;
*ecx = 0;

clean up disabled 386 code

[Michael W. Bombardieri]

Hi,

The code in "#if 0" has been disabled since revision 1.1.
>From what I see netbsd removed it too:
http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/arch/i386/i386/trap.c.diff?r1=1.237=1.238=date_with_tag=MAIN=h

- Michael


Index: src/sys/arch/i386/i386/trap.c
===
RCS file: /cvs/src/sys/arch/i386/i386/trap.c,v
retrieving revision 1.128
diff -u -p -u -r1.128 trap.c
--- src/sys/arch/i386/i386/trap.c   9 Mar 2017 20:31:41 -   1.128
+++ src/sys/arch/i386/i386/trap.c   6 Apr 2017 03:32:50 -
@@ -375,11 +375,6 @@ trap(struct trapframe *frame)
goto we_re_toast;
 
pcb = >p_addr->u_pcb;
-#if 0
-   /* XXX - check only applies to 386's and 486's with WP off */
-   if (frame->tf_err & PGEX_P)
-   goto we_re_toast;
-#endif
cr2 = rcr2();
KERNEL_LOCK();
/* This will only trigger if SMEP is enabled */

usage() in chpass(1)

[Michael W. Bombardieri]

Hi,

The following patch makes chpass(1) fail even faster when
the wrong options are provided and usage() would be printed.
In other words, no point accessing environment variables
before checking result of getopt().

- Michael


Index: chpass.c
===
RCS file: /cvs/src/usr.bin/chpass/chpass.c,v
retrieving revision 1.43
diff -u -p -u -r1.43 chpass.c
--- chpass.c26 Nov 2015 19:01:47 -  1.43
+++ chpass.c31 Mar 2017 08:03:37 -
@@ -66,13 +66,6 @@ main(int argc, char *argv[])
char *tz, *arg = NULL;
sigset_t fullset;
 
-   /* We need to use the system timezone for date conversions. */
-   if ((tz = getenv("TZ")) != NULL) {
-   unsetenv("TZ");
-   tzset();
-   setenv("TZ", tz, 1);
-   }
-
op = EDITENTRY;
while ((ch = getopt(argc, argv, "a:s:")) != -1)
switch(ch) {
@@ -90,6 +83,13 @@ main(int argc, char *argv[])
}
argc -= optind;
argv += optind;
+
+   /* We need to use the system timezone for date conversions. */
+   if ((tz = getenv("TZ")) != NULL) {
+   unsetenv("TZ");
+   tzset();
+   setenv("TZ", tz, 1);
+   }
 
uid = getuid();
 

softraid_crypto.c malloc failure case

[Michael W. Bombardieri]

Hi,

In sr_crypto_change_maskkey() p was being checked for NULL
twice, once after malloc() and once at goto label.
I think malloc() failure would be the only case where p doesn't
need to be freed, so add a special goto label for this.
Sorry if I got it wrong.

- Michael


Index: softraid_crypto.c
===
RCS file: /cvs/src/sys/dev/softraid_crypto.c,v
retrieving revision 1.131
diff -u -p -u -r1.131 softraid_crypto.c
--- softraid_crypto.c   8 Sep 2016 17:39:08 -   1.131
+++ softraid_crypto.c   17 Jan 2017 04:34:57 -
@@ -548,7 +548,7 @@ sr_crypto_change_maskkey(struct sr_disci
ksz = sizeof(sd->mds.mdd_crypto.scr_key);
p = malloc(ksz, M_DEVBUF, M_WAITOK | M_CANFAIL | M_ZERO);
if (p == NULL)
-   goto out;
+   goto out_nomem;
 
if (sr_crypto_decrypt(c, p, kdfinfo1->maskkey, ksz,
sd->mds.mdd_crypto.scr_meta->scm_mask_alg) == -1)
@@ -597,11 +597,10 @@ sr_crypto_change_maskkey(struct sr_disci
rv = 0; /* Success */
 
 out:
-   if (p) {
-   explicit_bzero(p, ksz);
-   free(p, M_DEVBUF, ksz);
-   }
+   explicit_bzero(p, ksz);
+   free(p, M_DEVBUF, ksz);
 
+out_nomem:
explicit_bzero(check_digest, sizeof(check_digest));
explicit_bzero(>maskkey, sizeof(kdfinfo1->maskkey));
explicit_bzero(>maskkey, sizeof(kdfinfo2->maskkey));

sys/malloc.h in sys/dev/usb

[Michael W. Bombardieri]

Hi,

Some usb drivers don't appear to need malloc().
I recently found this for umidi_quirks.c and decided to look further.
The patch was built on i386.

- Michael


Index: if_urtw.c
===
RCS file: /cvs/src/sys/dev/usb/if_urtw.c,v
retrieving revision 1.62
diff -u -p -u -r1.62 if_urtw.c
--- if_urtw.c   13 Apr 2016 11:03:37 -  1.62
+++ if_urtw.c   9 Jan 2017 04:38:49 -
@@ -25,7 +25,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
Index: ugold.c
===
RCS file: /cvs/src/sys/dev/usb/ugold.c,v
retrieving revision 1.12
diff -u -p -u -r1.12 ugold.c
--- ugold.c 9 Jan 2016 04:14:42 -   1.12
+++ ugold.c 9 Jan 2017 04:38:54 -
@@ -26,7 +26,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 
Index: umass.c
===
RCS file: /cvs/src/sys/dev/usb/umass.c,v
retrieving revision 1.73
diff -u -p -u -r1.73 umass.c
--- umass.c 3 Aug 2016 13:44:49 -   1.73
+++ umass.c 9 Jan 2017 04:39:14 -
@@ -130,7 +130,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #undef KASSERT
 #define KASSERT(cond, msg)
Index: uoak_subr.c
===
RCS file: /cvs/src/sys/dev/usb/uoak_subr.c,v
retrieving revision 1.7
diff -u -p -u -r1.7 uoak_subr.c
--- uoak_subr.c 25 May 2015 12:53:12 -  1.7
+++ uoak_subr.c 9 Jan 2017 04:39:14 -
@@ -22,7 +22,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
Index: usbdi_util.c
===
RCS file: /cvs/src/sys/dev/usb/usbdi_util.c,v
retrieving revision 1.41
diff -u -p -u -r1.41 usbdi_util.c
--- usbdi_util.c14 Mar 2015 03:38:50 -  1.41
+++ usbdi_util.c9 Jan 2017 04:39:14 -
@@ -35,7 +35,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 
 #include 
Index: uthum.c
===
RCS file: /cvs/src/sys/dev/usb/uthum.c,v
retrieving revision 1.31
diff -u -p -u -r1.31 uthum.c
--- uthum.c 19 Mar 2016 11:34:22 -  1.31
+++ uthum.c 9 Jan 2017 04:39:19 -
@@ -21,7 +21,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 

yacc: loop -> memcpy

[Michael W. Bombardieri]

Hi,

Two loops in yacc/lalr.c can be converted into memcpy().
I quickly tested this on i386 and amd64 against rcs/date.y
and the output matched y.tab.c of unpatched yacc.

- Michael


Index: lalr.c
===
RCS file: /cvs/src/usr.bin/yacc/lalr.c,v
retrieving revision 1.18
diff -u -p -u -r1.18 lalr.c
--- lalr.c  11 Dec 2015 20:25:47 -  1.18
+++ lalr.c  5 Jan 2017 08:56:11 -
@@ -324,8 +324,7 @@ initialize_F(void)
if (nedges) {
reads[i] = rp = NEW2(nedges + 1, short);
 
-   for (j = 0; j < nedges; j++)
-   rp[j] = edge[j];
+   memcpy(rp, edge, nedges * 2);
 
rp[nedges] = -1;
nedges = 0;
@@ -403,8 +402,7 @@ build_relations(void)
 
if (nedges) {
includes[i] = shortp = NEW2(nedges + 1, short);
-   for (j = 0; j < nedges; j++)
-   shortp[j] = edge[j];
+   memcpy(shortp, edge, nedges * 2);
shortp[nedges] = -1;
}
}

const in crypto code

[Michael W. Bombardieri]

Hi,

Add "const" to weak_keys array which is read only (read via bcmp()).
>From what I see all other arrays in src/sys/crypto are already "static const".

- Michael


Index: set_key.c
===
RCS file: /cvs/src/sys/crypto/set_key.c,v
retrieving revision 1.4
diff -u -p -u -r1.4 set_key.c
--- set_key.c   10 Dec 2015 21:00:51 -  1.4
+++ set_key.c   4 Jan 2017 06:34:27 -
@@ -84,7 +84,7 @@ check_parity(des_cblock (*key))
  * (and actual cblock values).
  */
 #define NUM_WEAK_KEY   16
-static des_cblock weak_keys[NUM_WEAK_KEY]={
+static const des_cblock weak_keys[NUM_WEAK_KEY]={
/* weak keys */
{0x01,0x01,0x01,0x01,0x01,0x01,0x01,0x01},
{0xFE,0xFE,0xFE,0xFE,0xFE,0xFE,0xFE,0xFE},

uaudio: preliminary patch for yamaha ur12 device support

[Michael W. Bombardieri]

Hi,

Sending this patch to tech@ in case people have usb
audio devices to test it.

When connecting yamaha ur12 usb audio interface I was getting
USBD_INVAL because of the aclen check. netbsd previously removed
the check for aclen here:
http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/dev/usb/uaudio.c.diff?r1=1.119=1.120=h=u

So this patch is a copy, without the Roland SD-90 stuff.
I originally applied this to uaudio.c r1.116 and it allowed
ur12 device to progress further. usb device attach
then failed later related to unexpected
format type or other sample encoding details. That would need
to be addressed later in uaudio_process_as().


- Michael


Index: uaudio.c
===
RCS file: /cvs/src/sys/dev/usb/uaudio.c,v
retrieving revision 1.122
diff -u -p -u -r1.122 uaudio.c
--- uaudio.c3 Jan 2017 06:45:58 -   1.122
+++ uaudio.c3 Jan 2017 08:14:27 -
@@ -1788,7 +1788,7 @@ uaudio_identify_ac(struct uaudio_softc *
const struct usb_audio_output_terminal *pot;
struct terminal_list *tml;
const char *buf, *ibuf, *ibufend;
-   int size, offs, aclen, ndps, i, j;
+   int size, offs, ndps, i, j;
 
size = UGETW(cdesc->wTotalLength);
buf = (char *)cdesc;
@@ -1807,26 +1807,23 @@ uaudio_identify_ac(struct uaudio_softc *
 
/* A class-specific AC interface header should follow. */
ibuf = buf + offs;
+   ibufend = buf + size;
acdp = (const struct usb_audio_control_descriptor *)ibuf;
if (acdp->bDescriptorType != UDESC_CS_INTERFACE ||
acdp->bDescriptorSubtype != UDESCSUB_AC_HEADER)
return (USBD_INVAL);
-   aclen = UGETW(acdp->wTotalLength);
-   if (offs + aclen > size)
-   return (USBD_INVAL);
 
if (!(sc->sc_quirks & UAUDIO_FLAG_BAD_ADC) &&
 UGETW(acdp->bcdADC) != UAUDIO_VERSION)
return (USBD_INVAL);
 
sc->sc_audio_rev = UGETW(acdp->bcdADC);
-   DPRINTFN(2,("%s: found AC header, vers=%03x, len=%d\n",
-__func__, sc->sc_audio_rev, aclen));
+   DPRINTFN(2,("%s: found AC header, vers=%03x\n",
+__func__, sc->sc_audio_rev));
 
sc->sc_nullalt = -1;
 
/* Scan through all the AC specific descriptors */
-   ibufend = ibuf + aclen;
dp = (const usb_descriptor_t *)ibuf;
ndps = 0;
iot = malloc(256 * sizeof(struct io_terminal),
@@ -1844,11 +1841,8 @@ uaudio_identify_ac(struct uaudio_softc *
free(iot, M_TEMP, 0);
return (USBD_INVAL);
}
-   if (dp->bDescriptorType != UDESC_CS_INTERFACE) {
-   printf("%s: skip desc type=0x%02x\n",
-  __func__, dp->bDescriptorType);
-   continue;
-   }
+   if (dp->bDescriptorType != UDESC_CS_INTERFACE)
+   break;
i = ((const struct usb_audio_input_terminal *)dp)->bTerminalId;
iot[i].d.desc = dp;
if (i > ndps)

dev/rnd.c more explicit_bzero

[Michael W. Bombardieri]

Hi,

Some functions in rnd have a timespec; make sure to zero it
as already done with other buffers. Also do buf in
dequeue_randomness().

- Michael


Index: src/sys/dev/rnd.c
===
RCS file: /cvs/src/sys/dev/rnd.c,v
retrieving revision 1.191
diff -u -p -u -r1.191 rnd.c
--- src/sys/dev/rnd.c   8 Dec 2016 05:32:49 -   1.191
+++ src/sys/dev/rnd.c   13 Dec 2016 04:49:24 -
@@ -312,6 +312,7 @@ enqueue_randomness(u_int state, u_int va
timeout_add(_timeout, 1);
 
mtx_leave();
+   explicit_bzero(, sizeof(ts));
 }
 
 /*
@@ -388,6 +389,7 @@ dequeue_randomness(void *v)
mtx_enter();
}
mtx_leave();
+   explicit_bzero(buf, sizeof(buf));
 }
 
 /*
@@ -458,6 +460,7 @@ suspend_randomness(void)
dequeue_randomness(NULL);
rs_count = 0;
arc4random_buf(entropy_pool, sizeof(entropy_pool));
+   explicit_bzero(, sizeof(ts));
 }
 
 void
@@ -473,6 +476,7 @@ resume_randomness(char *buf, size_t bufl
 
dequeue_randomness(NULL);
rs_count = 0;
+   explicit_bzero(, sizeof(ts));
 }
 
 static inline void _rs_rekey(u_char *dat, size_t datlen);
@@ -523,6 +527,7 @@ _rs_stir(int do_lock)
mtx_leave();
 
explicit_bzero(buf, sizeof(buf));
+   explicit_bzero(, sizeof(ts));
 }
 
 static inline void

dev/rnd.c comment typo

[Michael W. Bombardieri]

Hi,

Two typos in comments & explicit return at end of function returning void.

I am a little confused by mixture of memset and explicit_bzero in rnd.c for
clearing data. I understand explicit_bzero is meant for clearing sensitive
data. Would it be harmful for use only explicit_bzero here?

- Michael


Index: src/sys/dev/rnd.c
===
RCS file: /cvs/src/sys/dev/rnd.c,v
retrieving revision 1.190
diff -u -p -u -r1.190 rnd.c
--- src/sys/dev/rnd.c   18 Oct 2016 13:40:59 -  1.190
+++ src/sys/dev/rnd.c   8 Dec 2016 02:22:59 -
@@ -186,7 +186,7 @@
  * distance from evenly spaced; except for the last tap, which is 1 to
  * get the twisting happening as fast as possible.
  *
- * The reultant polynomial is:
+ * The resultant polynomial is:
  *   2^POOLWORDS + 2^POOL_TAP1 + 2^POOL_TAP2 + 2^POOL_TAP3 + 2^POOL_TAP4 + 1
  */
 #define POOLWORDS  2048
@@ -363,7 +363,7 @@ add_entropy_words(const u_int32_t *buf, 
 }
 
 /*
- * Pulls entropy out of the queue and throws merges it into the pool
+ * Pulls entropy out of the queue and merges it into the pool
  * with the CRC.
  */
 /* ARGSUSED */
@@ -631,7 +631,6 @@ _rs_random_u32(u_int32_t *val)
memcpy(val, rs_buf + RSBUFSZ - rs_have, sizeof(*val));
memset(rs_buf + RSBUFSZ - rs_have, 0, sizeof(*val));
rs_have -= sizeof(*val);
-   return;
 }
 
 /* Return one word of randomness from a ChaCha20 generator */

Re: KERNEL PATCH: add process hiding (fixed)

[Michael W. Bombardieri]

Should this patch also add see_other_uids in sysctl(8) manual?

On Sun, Dec 04, 2016 at 07:49:12PM -0500, Ian Walker wrote:
> (( Resending my last from a client that (hopefully) won't mangle the email.
>Sorry about the noise, folks. ))
> 
> 
> Hello OpenBSD Community -
> 
> OpenBSD should have the ability to prevent users from seeing each other's 
> processes even if this ability is disabled by default. 
> In addition to the small security benefit this provides, it also affords each 
> user a much greater amount of privacy. Linux and 
> FreeBSD already support similar features ( 
> https://www.cyberciti.biz/faq/linux-hide-processes-from-other-users/  && 
> https://www.cyberciti.biz/faq/freebsd-disable-ps-sockstat-command-information-leakage/
>  ) and the implementation itself is fairly 
> trivial.
> 
> Below is a patch which implements basic process hiding for non-superusers and 
> is activated with a sysctl knob. Similar to that of 
> FreeBSD it is called "kern.see_other_uids??. The idea is that if process 
> spying is a security or privacy concern for you, you 
> would add "kern.see_other_uids=0" to sysctl.conf and reboot (assuming 
> securelevel > 0).
> 
> I look forward to your comments.
> 
> Thanks and cheers all -
> Ian Walker
> 
> 
> 
> Index: sys/kern/kern_sysctl.c
> ===
> RCS file: /cvs/src/sys/kern/kern_sysctl.c,v
> retrieving revision 1.320
> diff -u -p -u -r1.320 kern_sysctl.c
> --- sys/kern/kern_sysctl.c11 Nov 2016 18:59:09 -1.320
> +++ sys/kern/kern_sysctl.c4 Dec 2016 20:43:53 -
> @@ -263,6 +263,7 @@ size_t disknameslen;
>  struct diskstats *diskstats = NULL;
>  size_t diskstatslen;
>  int securelevel;
> +int seeotheruids = 1; /* on by default */
>  
>  /*
>   * kernel related system variables.
> @@ -632,6 +633,13 @@ kern_sysctl(int *name, u_int namelen, vo
>  dnsjackport = port;
>  return 0;
>  }
> +case KERN_SEEOTHERUIDS: {
> +if (securelevel > 0)
> +return (sysctl_rdint(oldp, oldlenp, newp,
> +seeotheruids));
> +return (sysctl_int(oldp, oldlenp, newp, newlen,
> +));
> +}
>  default:
>  return (EOPNOTSUPP);
>  }
> @@ -1427,7 +1435,8 @@ sysctl_doproc(int *name, u_int namelen,
>  int arg, buflen, doingzomb, elem_size, elem_count;
>  int error, needed, op;
>  int dothreads = 0;
> -int show_pointers;
> +int is_suser, show_pointers, show_otheruids;
> +uid_t euid;
>  
>  dp = where;
>  buflen = where != NULL ? *sizep : 0;
> @@ -1444,7 +1453,10 @@ sysctl_doproc(int *name, u_int namelen,
>  dothreads = op & KERN_PROC_SHOW_THREADS;
>  op &= ~KERN_PROC_SHOW_THREADS;
>  
> -show_pointers = suser(curproc, 0) == 0;
> +is_suser = suser(curproc, 0) == 0;
> +show_pointers = is_suser;
> +show_otheruids = seeotheruids || is_suser;
> +euid = curproc->p_ucred->cr_uid;
>  
>  if (where != NULL)
>  kproc = malloc(sizeof(*kproc), M_TEMP, M_WAITOK);
> @@ -1461,6 +1473,9 @@ again:
>   * Skip embryonic processes.
>   */
>  if (pr->ps_flags & PS_EMBRYO)
> +continue;
> +
> +if (!show_otheruids && pr->ps_ucred->cr_uid != euid)
>  continue;
>  
>  /*
> Index: sys/sys/sysctl.h
> ===
> RCS file: /cvs/src/sys/sys/sysctl.h,v
> retrieving revision 1.170
> diff -u -p -u -r1.170 sysctl.h
> --- sys/sys/sysctl.h7 Nov 2016 00:26:32 -1.170
> +++ sys/sys/sysctl.h4 Dec 2016 20:43:55 -
> @@ -184,7 +184,8 @@ struct ctlname {
>  #defineKERN_GLOBAL_PTRACE81/* allow ptrace globally */
>  #defineKERN_CONSBUFSIZE82/* int: console message buffer size */
>  #defineKERN_CONSBUF83/* console message buffer */
> -#defineKERN_MAXID84/* number of valid kern ids */
> +#defineKERN_SEEOTHERUIDS84/* see other users' proceesses */
> +#defineKERN_MAXID85/* number of valid kern ids */
>  
>  #defineCTL_KERN_NAMES { \
>  { 0, 0 }, \
> @@ -269,6 +270,9 @@ struct ctlname {
>  { "proc_nobroadcastkill", CTLTYPE_NODE }, \
>  { "proc_vmmap", CTLTYPE_NODE }, \
>  { "global_ptrace", CTLTYPE_INT }, \
> +{ "gap", 0 }, \
> +{ "gap", 0 }, \
> +{ "see_other_uids", CTLTYPE_INT }, \
>  }
>  
>  /*
> 

steinberg/yamaha ur22 audio device + openbsd 6.0

[Michael W. Bombardieri]

Hi tech,

I had some success using Yamaha/Steinberg UR22 usb audio
interface box on OpenBSD/i386 6.0.
I heard that Linux supports the device but I never tested it under Linux.
I spent AUD $150 on the device last year and wondered if it could ever
be used with OpenBSD aucat(1).

The USB-quirks patch (below) allows UR22 to attach as uaudio device.
Patch was originally built against current kernel checked out a few days ago.
I've re-applied it against uaudio.c r1.115.
Regenerating usbdevs.h/usbdevs_data.h isn't included in patch.

I also own Creative Soundblaster X-Fi usb soundcard;
this worked on OpenBSD for some time but I wouldn't use it for recording.

Configuration:
* Test PC is "bios0: Compaq-Presario KJ335AA-ABG SR5490AN"
* UR22 audio input 1 connected to AKG P120 microphone via XLR cable
* UR22 audio input 2 connected to electric guitar via instrument cable
* Hi-Z toggle is enabled for input 2 on front panel
* +48V "phantom power" toggle is enabled for input 1 on back panel
* UR22 is attached directly to PC via USB cable (no intermediate hubs)
* label on front panel says "24-bit / 192 kHz"
* white "status" LED on front panel is solid (stops flashing when uaudio 
attaches)
* no speakers attached to line output (L/R) on back panel
* i am listening to audio playback via headphone jack on front panel
* headphone jack is also used for monitoring audio inputs 1 & 2

Simple playback:
* "aucat -i stereo48khz.wav" plays file ok

Stereo record (default encoding of s16):
* aucat -r $RATE -o test.wav
...with the following rates: 44100 48000 64000 96000 112000 192000
* all recorded files play back ok

Stereo record (different encoding):
* i'm fairly sure i have recorded in 24-bit mode under M$ Windows 8
* aucat -r 44100 -e s24le3 -o test.wav # caused segmentation fault
* also tried aucat with "-h raw" but it still dies
* audioctl reports encoding=s24le3 so i am confused
* i need to compile non-stripped aucat to see more

Other tests performed:
* Two concurrent instances of aucat; one recording, one playing
* Two concurrent instances of aucat; both playing
* One instance of aucat (record+play): aucat -r 96000 -i in.wav -o out.wav
* One instance of aucat (play+play): aucat -i in.wav -i out.wav

TODO:
* re-test on another PC, and on amd64
* determine why aucat segfaults for 24bit encodings (above)
* test umidi with this device (attaches successfully on unmodified 6.0 kernel)
* phone grandmother with voip/sip software and play guitar down the line?

- Michael


Index: uaudio.c
===
RCS file: /cvs/src/sys/dev/usb/uaudio.c,v
retrieving revision 1.115
diff -u -r1.115 uaudio.c
--- uaudio.c19 Sep 2016 06:46:44 -  1.115
+++ uaudio.c22 Sep 2016 01:13:04 -
@@ -177,6 +177,8 @@
struct usb_devno uv_dev;
int  flags;
 } uaudio_devs[] = {
+   { { USB_VENDOR_YAMAHA, USB_PRODUCT_YAMAHA_UR22 },
+   UAUDIO_FLAG_VENDOR_CLASS },
{ { USB_VENDOR_ALTEC, USB_PRODUCT_ALTEC_ADA70 },
UAUDIO_FLAG_BAD_ADC } ,
{ { USB_VENDOR_ALTEC, USB_PRODUCT_ALTEC_ASC495 },
Index: usbdevs
===
RCS file: /cvs/src/sys/dev/usb/usbdevs,v
retrieving revision 1.669
diff -u -r1.669 usbdevs
--- usbdevs 21 Sep 2016 08:55:21 -  1.669
+++ usbdevs 22 Sep 2016 01:13:05 -
@@ -4443,6 +4443,7 @@
 product YEDATA FLASHBUSTERU0x  Flashbuster-U
 
 /* Yamaha products */
+product YAMAHA UR220x1509  UR22
 product YAMAHA UX256   0x1000  UX256 MIDI I/F
 productYAMAHA UX96 0x1008  UX96 MIDI I/F
 product YAMAHA RPU200  0x3104  RP-U200

Re: Enabling rasops24

[Michael W. Bombardieri]

On Mon, Sep 19, 2016 at 06:52:00PM -0400, sven falempin wrote:
> Tested here and it works ! @OK
> 
> The terminal is white on red for first part of boot then goes back to white
> on
> blue.
> Also some kind of flicker/bufering during the start,
> aftfer the kernel loading no update for like half a second.
> 
> But it s booting completly :
> 
> i got some
> dmesg: sysctl: KERN_MSGBUF: Cannot allocate memmory.
> probably related to my  tree i used to compile the kernel with the
> option
> rasops24
> 
> ( QEMU emulator version 2.6.0, with OVMF Bios, OpenBSD directly on phyical
> hard drive )
Any difference under QEMU 2.7.0?

> On Tue, Aug 30, 2016 at 12:49 PM, YASUOKA Masahiko 
> wrote:
> 
> > Enabling rasops24 in files.amd64 makes QEMU with UEFI start working.
> >
> > But.. the background color of the kernel message is sometimes red or
> > green where it should be blue.
> >
> > ok for files.amd64?
> > comments for the color problem?
> >
> > Index: sys/arch/amd64/conf/files.amd64
> > ===
> > RCS file: /cvs/src/sys/arch/amd64/conf/files.amd64,v
> > retrieving revision 1.85
> > diff -u -p -r1.85 files.amd64
> > --- sys/arch/amd64/conf/files.amd64 8 Jan 2016 15:54:13 -
> >  1.85
> > +++ sys/arch/amd64/conf/files.amd64 30 Aug 2016 16:38:19 -
> > @@ -109,7 +109,7 @@ filearch/amd64/amd64/ioapic.c
> >  ioapic n
> >  #
> >  # EFI Framebuffer
> >  #
> > -device efifb: wsemuldisplaydev, rasops32, rasops16, rasops8, rasops4
> > +device efifb: wsemuldisplaydev, rasops32, rasops24, rasops16, rasops8,
> > rasops4
> >  attach efifb at mainbus
> >  file arch/amd64/amd64/efifb.c  efifb needs-flag
> >
> >
> >
> 
> 
> -- 
> -
> () ascii ribbon campaign - against html e-mail
> /\

Re: remove /dev/sound*

[Michael W. Bombardieri]

Hi Alexandre,

Do you know if any applications in ports use /dev/sound as default audio device.
Maybe they are not smart enough to try /dev/audio if /dev/sound fails.

- Michael


On Thu, Sep 08, 2016 at 08:12:45AM +0200, Alexandre Ratchov wrote:
> As audio(4) manual says "In all respects /dev/audio and /dev/sound
> are identical".  Only one of them is needed and this diff is to
> remove /dev/sound.
> 
> OK?
> 
> Index: etc/MAKEDEV.common
> ===
> RCS file: /cvs/src/etc/MAKEDEV.common,v
> retrieving revision 1.91
> diff -u -p -u -p -r1.91 MAKEDEV.common
> --- etc/MAKEDEV.common4 Sep 2016 15:38:59 -   1.91
> +++ etc/MAKEDEV.common8 Sep 2016 05:48:20 -
> @@ -418,13 +418,11 @@ _mkdev(acpi, acpi*, {-M acpic major_acp
>  __devitem(pctr, pctr*, PC Performance Tuning Register access device)dnl
>  _mkdev(pctr, pctr, {-M pctr c major_pctr_c 0 644-})dnl
>  __devitem(au, audio*, Audio devices,audio)dnl
> -_mkdev(au, audio*, {-M sound$U   c major_au_c $U
> +_mkdev(au, audio*, {-M audio$U   c major_au_c $U
>   M mixer$U   c major_au_c Add($U, 16)
> - M audio$U   c major_au_c Add($U, 128)
>   M audioctl$Uc major_au_c Add($U, 192)
>   MKlist[${#MKlist[*]}]=";[ -e audio ] || ln -s audio$U audio"
>   MKlist[${#MKlist[*]}]=";[ -e mixer ] || ln -s mixer$U mixer"
> - MKlist[${#MKlist[*]}]=";[ -e sound ] || ln -s sound$U sound"
>   MKlist[${#MKlist[*]}]=";[ -e audioctl ] || ln -s audioctl$U 
> audioctl"-})dnl
>  __devitem(vi, video*, Video V4L2 devices,video)dnl
>  _mkdev(vi, video*, {-M video$U  c major_vi_c $U 600
> Index: share/man/man4/audio.4
> ===
> RCS file: /cvs/src/share/man/man4/audio.4,v
> retrieving revision 1.74
> diff -u -p -u -p -r1.74 audio.4
> --- share/man/man4/audio.48 Sep 2016 05:18:20 -   1.74
> +++ share/man/man4/audio.48 Sep 2016 05:48:20 -
> @@ -53,14 +53,11 @@ underlying hardware configuration suppor
>  .Pp
>  There are four device files available for audio operation:
>  .Pa /dev/audio ,
> -.Pa /dev/sound ,
>  .Pa /dev/audioctl ,
>  and
>  .Pa /dev/mixer .
>  .Pa /dev/audio
> -and
> -.Pa /dev/sound
> -are used for recording or playback of digital samples.
> +is used for recording or playback of digital samples.
>  .Pa /dev/mixer
>  is used to manipulate volume, recording source, or other audio mixer
>  functions.
> @@ -68,10 +65,10 @@ functions.
>  accepts the same
>  .Xr ioctl 2
>  operations as
> -.Pa /dev/sound ,
> +.Pa /dev/audio ,
>  but no other operations.
>  In contrast to
> -.Pa /dev/sound ,
> +.Pa /dev/audio ,
>  which has the exclusive open property,
>  .Pa /dev/audioctl
>  can be opened at any time and can be used to read the
> @@ -80,18 +77,11 @@ device variables while it is in use.
>  .Sh SAMPLING DEVICES
>  When
>  .Pa /dev/audio
> -or
> -.Pa /dev/sound
>  is opened, it attempts to maintain the previous audio sample format and
>  record/playback mode.
>  In addition, if it is opened read-only
>  (write-only) the device is set to half-duplex record (play) mode with
>  recording (playing) unpaused.
> -In all respects
> -.Pa /dev/audio
> -and
> -.Pa /dev/sound
> -are identical.
>  .Pp
>  Only one process may hold open a sampling device at a given time
>  (although file descriptors may be shared between processes once the
> @@ -514,7 +504,6 @@ string values.
>  .Bl -tag -width /dev/audioctl -compact
>  .It Pa /dev/audio
>  .It Pa /dev/audioctl
> -.It Pa /dev/sound
>  .It Pa /dev/mixer
>  .El
>  .Sh SEE ALSO
> 

[patch] opencvs rcsnum_free()

[Michael W. Bombardieri]

Hi,

If people are interested in opencvs diffs again, sharing a rcsnum_free()->free()
clean-up item. Note that rcs(1) also has a version of rcsnum_free() which does 
more
than simply call free().

- Michael


Index: add.c
===
RCS file: /cvs/src/usr.bin/cvs/add.c,v
retrieving revision 1.112
diff -u -p -u -r1.112 add.c
--- add.c   5 Nov 2015 09:48:21 -   1.112
+++ add.c   24 Jun 2016 06:10:45 -
@@ -485,8 +485,7 @@ add_file(struct cvs_file *cf)
break;
}
 
-   if (head != NULL)
-   rcsnum_free(head);
+   free(head);
 
if (stop == 1)
return;
Index: admin.c
===
RCS file: /cvs/src/usr.bin/cvs/admin.c,v
retrieving revision 1.66
diff -u -p -u -r1.66 admin.c
--- admin.c 5 Nov 2015 09:48:21 -   1.66
+++ admin.c 24 Jun 2016 06:10:45 -
@@ -334,11 +334,11 @@ cvs_admin_local(struct cvs_file *cf)
if (rcs_rev_setlog(cf->file_rcs, rev, logmsg) < 0) {
cvs_log(LP_ERR, "failed to set logmsg for `%s' to `%s'",
logstr, logmsg);
-   rcsnum_free(rev);
+   free(rev);
return;
}
 
-   rcsnum_free(rev);
+   free(rev);
}
 
if (orange != NULL) {
@@ -380,7 +380,7 @@ cvs_admin_local(struct cvs_file *cf)
 
(void)rcs_state_set(cf->file_rcs, rev, state);
 
-   rcsnum_free(rev);
+   free(rev);
}
 
if (lkmode != RCS_LOCK_INVAL)
Index: annotate.c
===
RCS file: /cvs/src/usr.bin/cvs/annotate.c,v
retrieving revision 1.65
diff -u -p -u -r1.65 annotate.c
--- annotate.c  5 Nov 2015 09:48:21 -   1.65
+++ annotate.c  24 Jun 2016 06:10:45 -
@@ -178,7 +178,7 @@ cvs_annotate_local(struct cvs_file *cf)
rev = rcsnum_parse(cvs_specified_tag);
if (rev == NULL)
fatal("no such tag %s", cvs_specified_tag);
-rcsnum_free(rev);
+free(rev);
rev = rcsnum_alloc();
rcsnum_cpy(cf->file_rcs->rf_head, rev, 0);
}
@@ -205,9 +205,9 @@ cvs_annotate_local(struct cvs_file *cf)
 */
if (bnum != rev) {
rcs_annotate_getlines(cf->file_rcs, rev, );
-   rcsnum_free(bnum);
+   free(bnum);
}
-   rcsnum_free(rev);
+   free(rev);
} else {
rcs_rev_getlines(cf->file_rcs, (cvs_specified_date != -1 ||
cvs_directory_date != -1) ? cf->file_rcsrev :
Index: commit.c
===
RCS file: /cvs/src/usr.bin/cvs/commit.c,v
retrieving revision 1.154
diff -u -p -u -r1.154 commit.c
--- commit.c5 Nov 2015 09:48:21 -   1.154
+++ commit.c24 Jun 2016 06:10:45 -
@@ -365,7 +365,7 @@ cvs_commit_check_files(struct cvs_file *
if (brev != NULL) {
if (RCSNUM_ISBRANCH(brev))
goto next;
-   rcsnum_free(brev);
+   free(brev);
}
 
brev = rcs_translate_tag(tag, cf->file_rcs);
@@ -382,7 +382,7 @@ cvs_commit_check_files(struct cvs_file *
"a branch for file %s", tag,
cf->file_path);
conflicts_found++;
-   rcsnum_free(brev);
+   free(brev);
return;
}
 
@@ -391,8 +391,8 @@ cvs_commit_check_files(struct cvs_file *
"a branch for file %s", tag,
cf->file_path);
conflicts_found++;
-   rcsnum_free(branch);
-   rcsnum_free(brev);
+   free(branch);
+   free(brev);
return;
}
 
@@ -401,18 +401,16 @@ cvs_commit_check_files(struct cvs_file *
"a branch for file %s", tag,
cf->file_path);
conflicts_found++;
-   rcsnum_free(branch);
-   rcsnum_free(brev);
+   free(branch);
+   free(brev);
return;
}
   

Re: opencvs - fix revision lookups for branches

[Michael W. Bombardieri]

Yes please. As noted in older thread that XXX block in rcs.c produced side 
effects with cvs annotate.
https://marc.info/?l=openbsd-tech=144757775319206=2


On Wed, Jun 22, 2016 at 05:20:01PM +0200, Joris Vink wrote:
> On Wed, Jun 22, 2016 at 09:07:03AM -0600, Todd C. Miller wrote:
> > On Wed, 22 Jun 2016 12:21:56 +0200, Joris Vink wrote:
> > > Index: rcs.c
> > > ===
> > > RCS file: /cvs/src/usr.bin/cvs/rcs.c,v
> > > retrieving revision 1.313
> > > diff -u -p -r1.313 rcs.c
> > > --- rcs.c 5 Nov 2015 09:48:21 -   1.313
> > > +++ rcs.c 22 Jun 2016 09:52:04 -
> > > @@ -1796,17 +1796,13 @@ rcs_rev_getlines(RCSFILE *rfp, RCSNUM *f
> > >  
> > >  again:
> > >   for (;;) {
> > > + if (rdp == NULL)
> > > + break;
> > 
> > Wouldn't this be easier to read as:
> > 
> > while (rdp != NULL) {
> 
> Yes, updated diff below.
> 
> .joris
> 
> Index: rcs.c
> ===
> RCS file: /cvs/src/usr.bin/cvs/rcs.c,v
> retrieving revision 1.313
> diff -u -p -r1.313 rcs.c
> --- rcs.c 5 Nov 2015 09:48:21 -   1.313
> +++ rcs.c 22 Jun 2016 15:13:14 -
> @@ -1795,18 +1795,11 @@ rcs_rev_getlines(RCSFILE *rfp, RCSNUM *f
>   goto done;
>  
>  again:
> - for (;;) {
> + while (rdp != NULL) {
>   if (rdp->rd_next->rn_len != 0) {
>   trdp = rcs_findrev(rfp, rdp->rd_next);
>   if (trdp == NULL)
>   fatal("failed to grab next revision");
> - } else {
> - /*
> -  * XXX Fail, although the caller does not always do the
> -  * right thing (eg cvs diff when the tree is ahead of
> -  * the repository).
> -  */
> - break;
>   }
>  
>   if (rdp->rd_tlen == 0) {
> @@ -1857,7 +1850,7 @@ again:
>   }
>  
>  next:
> - if (!rcsnum_differ(rdp->rd_num, frev))
> + if (rdp == NULL || !rcsnum_differ(rdp->rd_num, frev))
>   done = 1;
>  
>   if (RCSNUM_ISBRANCHREV(frev) && done != 1) {
> @@ -2045,6 +2038,7 @@ rcs_rev_getbuf(RCSFILE *rfp, RCSNUM *rev
>   struct rcs_delta *rdp;
>   struct rcs_lines *lines;
>   struct rcs_line *lp, *nlp;
> + char version[RCSNUM_MAXSTR];
>   BUF *bp;
>  
>   rdp = NULL;
> @@ -2057,8 +2051,12 @@ rcs_rev_getbuf(RCSFILE *rfp, RCSNUM *rev
>   expmode = rcs_kwexp_get(rfp);
>  
>   if (!(expmode & RCS_KWEXP_NONE)) {
> - if ((rdp = rcs_findrev(rfp, rev)) == NULL)
> - fatal("could not fetch revision");
> + if ((rdp = rcs_findrev(rfp, rev)) == NULL) {
> + rcsnum_tostr(rev, version, sizeof(version));
> + fatal("could not find desired version %s in %s",
> + version, rfp->rf_path);
> + }
> +
>   expand = 1;
>   }
>   }
> 

[patch] S_ISDIR() check in yacc TMPDIR

[Michael W. Bombardieri]

Hello,

Sending this to tech list as suggested by mmcc@...

The yacc(1) manual mentions TMPDIR is an extension.
The following patch re-factors the checks around TMPDIR.
This adds an explicit check for TMPDIR-is-a-directory.
With this patch applied, yacc can more reliably determine when it
should use _PATH_TMP.

- Michael


Index: main.c
===
RCS file: /cvs/src/usr.bin/yacc/main.c,v
retrieving revision 1.27
diff -u -p -u -r1.27 main.c
--- main.c  10 Oct 2015 14:23:47 -  1.27
+++ main.c  21 Jun 2016 03:47:30 -
@@ -33,6 +33,7 @@
  * SUCH DAMAGE.
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -225,6 +226,18 @@ allocate(size_t n)
return (v);
 }
 
+char *
+tmp_dir(void)
+{
+   struct stat st;
+   char *tmp;
+
+   tmp = getenv("TMPDIR");
+   if (tmp == NULL || *tmp == '\0' || lstat(tmp, ) == -1 || 
!S_ISDIR(st.st_mode))
+   return (_PATH_TMP);
+   return (tmp);
+}
+
 #define TEMPNAME(s, c, d, l)   \
(asprintf(&(s), "%.*s/yacc.%xXX", (int)(l), (d), (c)))
 
@@ -234,9 +247,7 @@ create_file_names(void)
size_t len;
char *tmpdir;
 
-   if ((tmpdir = getenv("TMPDIR")) == NULL || *tmpdir == '\0')
-   tmpdir = _PATH_TMP;
-
+   tmpdir = tmp_dir();
len = strlen(tmpdir);
if (tmpdir[len - 1] == '/')
len--;

[patch] macros in diff

[Michael W. Bombardieri]

Hi tech,

In diff & friends, use  macros MIN() and MAX() instead of
defining these locally. Worth doing?

- Michael


Index: diff/diffreg.c
===
RCS file: /cvs/src/usr.bin/diff/diffreg.c,v
retrieving revision 1.90
diff -u -p -r1.90 diffreg.c
--- diff/diffreg.c  26 Oct 2015 12:52:27 -  1.90
+++ diff/diffreg.c  30 Dec 2015 02:28:03 -
@@ -64,6 +64,7 @@
  * @(#)diffreg.c   8.1 (Berkeley) 6/6/93
  */
 
+#include 
 #include 
 #include 
 
@@ -83,9 +84,6 @@
 #include "diff.h"
 #include "xmalloc.h"
 
-#define MINIMUM(a, b)  (((a) < (b)) ? (a) : (b))
-#define MAXIMUM(a, b)  (((a) > (b)) ? (a) : (b))
-
 /*
  * diff - compare two files.
  */
@@ -595,7 +593,7 @@ stone(int *a, int n, int *b, int *c, int
bound = UINT_MAX;
else {
sq = isqrt(n);
-   bound = MAXIMUM(256, sq);
+   bound = MAX(256, sq);
}
 
k = 0;
@@ -1302,10 +1300,10 @@ dump_context_vec(FILE *f1, FILE *f2, int
return;
 
b = d = 0;  /* gcc */
-   lowa = MAXIMUM(1, cvp->a - diff_context);
-   upb = MINIMUM(len[0], context_vec_ptr->b + diff_context);
-   lowc = MAXIMUM(1, cvp->c - diff_context);
-   upd = MINIMUM(len[1], context_vec_ptr->d + diff_context);
+   lowa = MAX(1, cvp->a - diff_context);
+   upb = MIN(len[0], context_vec_ptr->b + diff_context);
+   lowc = MAX(1, cvp->c - diff_context);
+   upd = MIN(len[1], context_vec_ptr->d + diff_context);
 
diff_output("***");
if ((flags & D_PROTOTYPE)) {
@@ -1405,10 +1403,10 @@ dump_unified_vec(FILE *f1, FILE *f2, int
return;
 
b = d = 0;  /* gcc */
-   lowa = MAXIMUM(1, cvp->a - diff_context);
-   upb = MINIMUM(len[0], context_vec_ptr->b + diff_context);
-   lowc = MAXIMUM(1, cvp->c - diff_context);
-   upd = MINIMUM(len[1], context_vec_ptr->d + diff_context);
+   lowa = MAX(1, cvp->a - diff_context);
+   upb = MIN(len[0], context_vec_ptr->b + diff_context);
+   lowc = MAX(1, cvp->c - diff_context);
+   upd = MIN(len[1], context_vec_ptr->d + diff_context);
 
diff_output("@@ -");
uni_range(lowa, upb);
Index: rcs/diff.c
===
RCS file: /cvs/src/usr.bin/rcs/diff.c,v
retrieving revision 1.38
diff -u -p -r1.38 diff.c
--- rcs/diff.c  13 Jun 2015 20:15:21 -  1.38
+++ rcs/diff.c  30 Dec 2015 02:28:04 -
@@ -64,6 +64,7 @@
  * @(#)diffreg.c   8.1 (Berkeley) 6/6/93
  */
 
+#include 
 #include 
 
 #include 
@@ -81,9 +82,6 @@
 #include "diff.h"
 #include "xmalloc.h"
 
-#define MINIMUM(a, b)  (((a) < (b)) ? (a) : (b))
-#define MAXIMUM(a, b)  (((a) > (b)) ? (a) : (b))
-
 /*
  * diff - compare two files.
  */
@@ -532,7 +530,7 @@ stone(int *a, int n, int *b, int *c, int
bound = UINT_MAX;
else {
sq = isqrt(n);
-   bound = MAXIMUM(256, sq);
+   bound = MAX(256, sq);
}
 
k = 0;
@@ -1205,10 +1203,10 @@ dump_context_vec(FILE *f1, FILE *f2, int
return;
 
b = d = 0;  /* gcc */
-   lowa = MAXIMUM(1, cvp->a - diff_context);
-   upb = MINIMUM(len[0], context_vec_ptr->b + diff_context);
-   lowc = MAXIMUM(1, cvp->c - diff_context);
-   upd = MINIMUM(len[1], context_vec_ptr->d + diff_context);
+   lowa = MAX(1, cvp->a - diff_context);
+   upb = MIN(len[0], context_vec_ptr->b + diff_context);
+   lowc = MAX(1, cvp->c - diff_context);
+   upd = MIN(len[1], context_vec_ptr->d + diff_context);
 
diff_output("***");
if ((flags & D_PROTOTYPE)) {
@@ -1308,10 +1306,10 @@ dump_unified_vec(FILE *f1, FILE *f2, int
return;
 
d = 0; /* gcc */
-   lowa = MAXIMUM(1, cvp->a - diff_context);
-   upb = MINIMUM(len[0], context_vec_ptr->b + diff_context);
-   lowc = MAXIMUM(1, cvp->c - diff_context);
-   upd = MINIMUM(len[1], context_vec_ptr->d + diff_context);
+   lowa = MAX(1, cvp->a - diff_context);
+   upb = MIN(len[0], context_vec_ptr->b + diff_context);
+   lowc = MAX(1, cvp->c - diff_context);
+   upd = MIN(len[1], context_vec_ptr->d + diff_context);
 
diff_output("@@ -");
uni_range(lowa, upb);
Index: cvs/diff_internals.c
===
RCS file: /cvs/src/usr.bin/cvs/diff_internals.c,v
retrieving revision 1.38
diff -u -p -r1.38 diff_internals.c
--- cvs/diff_internals.c5 Nov 2015 09:48:21 -   1.38
+++ cvs/diff_internals.c30 Dec 2015 02:28:04 -
@@ -64,6 +64,7 @@
  * @(#)diffreg.c   8.1 (Berkeley) 6/6/93
  */
 
+#include 
 #include 
 #include 
 
@@ -81,9 +82,6 @@
 #include "cvs.h"
 #include "diff.h"
 
-#define MINIMUM(a, b)  (((a) < (b)) ? (a) : (b))
-#define MAXIMUM(a, b)  (((a) > (b)) ? (a) : (b))
-
 /*
  * 

[patch] playing with opencvs annotate

[Michael W. Bombardieri]

Hi tech@,

I had a simple RCS file and opencvs didn't annotate it properly.

 cat -n $CVSROOT/test/text,v
 1  head1.2;
 2  access;
 3  symbols
 4  start:1.1.1.1
 5  me:1.1.0.1;
 6  locks; strict;
 7  comment @# @;
 8
 9
10  1.2
11  date2015.11.15.06.43.34;author omm;  state Exp;
12  branches;
13  next1.1;
14
15  1.1
16  date2015.11.15.06.40.34;author omm;  state Exp;
17  branches
18  1.1.1.1;
19  next;
20
21  1.1.1.1
22  date2015.11.15.06.40.34;author omm;  state Exp;
23  branches;
24  next;
25
26
27  desc
28  @@
29
30
31  1.2
32  log
33  @thing
34  @
35  text
36  @no
37  woman
38  no
39  cry
40  @
41
42
43  1.1
44  log
45  @Initial revision
46  @
47  text
48  @d1 1
49  d4 1
50  @
51
52
53  1.1.1.1
54  log
55  @hi
56  @
57  text
58  @@


GNU cvs is correct...
$ cvs annotate text
Annotations for text
***
1.2  (omm   15-Nov-15): no
1.1  (omm   15-Nov-15): woman
1.1  (omm   15-Nov-15): no
1.2  (omm   15-Nov-15): cry

$ opencvs annotate text
Annotations for text
***
1.1  (omm   15-Nov-15): no
1.1  (omm   15-Nov-15): woman
1.1  (omm   15-Nov-15): no
1.1  (omm   15-Nov-15): cry

After appling patch (below) opencvs matches GNU cvs annotation and blame.

$ blame -V
blame 1.3.1; emulating RCS version 5

$ blame $CVSROOT/test/text,v | sha256
Annotations for text
***
2bcdf1fddf23fecd0710bba4376ba21e394bc97a6a2403b5453320bcea109d5c

$ cvs ann text | sha256
Annotations for text
***
2bcdf1fddf23fecd0710bba4376ba21e394bc97a6a2403b5453320bcea109d5c

$ opencvs ann text | sha256
Annotations for text
***
2bcdf1fddf23fecd0710bba4376ba21e394bc97a6a2403b5453320bcea109d5c

I also tested this against a non-trival RCS file: r1.125 of OpenBSD's 
src/Makefile.

$ cvs ann Makefile | sha256
Annotations for Makefile
***
335b6c5735636e8358db6058bd4e9e2bd3173c80e70a0fc1dd86f541a73eb1c7

$ opencvs ann Makefile | sha256
Annotations for Makefile
***
335b6c5735636e8358db6058bd4e9e2bd3173c80e70a0fc1dd86f541a73eb1c7

Then I looked in history of rcs.c and found that my patch is
the opposite of a fix for an infinite loop bug...
http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/cvs/rcs.c#rev1.298

Below patch is for reference only; it's what I applied to "fix" local annotate.
Did the previous infinite loop issue only affect server/remote mode of opencvs?

- Michael


Index: rcs.c
===
RCS file: /cvs/src/usr.bin/cvs/rcs.c,v
retrieving revision 1.313
diff -U 5 -p -r1.313 rcs.c
--- rcs.c   5 Nov 2015 09:48:21 -   1.313
+++ rcs.c   15 Nov 2015 08:29:18 -
@@ -1798,17 +1798,10 @@ again:
for (;;) {
if (rdp->rd_next->rn_len != 0) {
trdp = rcs_findrev(rfp, rdp->rd_next);
if (trdp == NULL)
fatal("failed to grab next revision");
-   } else {
-   /*
-* XXX Fail, although the caller does not always do the
-* right thing (eg cvs diff when the tree is ahead of
-* the repository).
-*/
-   break;
}
 
if (rdp->rd_tlen == 0) {
if (rcsparse_deltatexts(rfp, rdp->rd_num))
fatal("rcs_rev_getlines: rcsparse_deltatexts");

Re: unify xmalloc (was Re: [patch] cvs: retire xfree())

[Michael W. Bombardieri]

> > ok for removing xfree from aucat?
> > 
> 
> yes, ok ratchov; if later this causes me merges i'll find another
> solution.  Feel free to do the same in usr.bin/sndiod, as it's
> almost the same.
> 

Same thing for sndiod...


Index: abuf.c
===
RCS file: /cvs/src/usr.bin/sndiod/abuf.c,v
retrieving revision 1.3
diff -u -p -u -r1.3 abuf.c
--- abuf.c  16 Feb 2015 06:11:33 -  1.3
+++ abuf.c  12 Nov 2015 07:07:57 -
@@ -62,7 +62,7 @@ abuf_done(struct abuf *buf)
}
}
 #endif
-   xfree(buf->data);
+   free(buf->data);
buf->data = (void *)0xdeadbeef;
 }
 
Index: dev.c
===
RCS file: /cvs/src/usr.bin/sndiod/dev.c,v
retrieving revision 1.18
diff -u -p -u -r1.18 dev.c
--- dev.c   5 Sep 2015 11:19:20 -   1.18
+++ dev.c   12 Nov 2015 07:07:57 -
@@ -15,6 +15,7 @@
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 #include 
+#include 
 #include 
 
 #include "abuf.h"
@@ -838,10 +839,8 @@ dev_cycle(struct dev *d)
 */
s->pstate = SLOT_INIT;
abuf_done(>mix.buf);
-   if (s->mix.decbuf)
-   xfree(s->mix.decbuf);
-   if (s->mix.resampbuf)
-   xfree(s->mix.resampbuf);
+   free(s->mix.decbuf);
+   free(s->mix.resampbuf);
s->ops->eof(s->arg);
*ps = s->next;
dev_mix_adjvol(d);
@@ -1143,14 +1142,12 @@ dev_close(struct dev *d)
d->slot_list = NULL;
dev_sio_close(d);
if (d->mode & MODE_PLAY) {
-   if (d->encbuf != NULL)
-   xfree(d->encbuf);
-   xfree(d->pbuf);
+   free(d->encbuf);
+   free(d->pbuf);
}
if (d->mode & MODE_REC) {
-   if (d->decbuf != NULL)
-   xfree(d->decbuf);
-   xfree(d->rbuf);
+   free(d->decbuf);
+   free(d->rbuf);
}
 }
 
@@ -1256,7 +1253,7 @@ dev_del(struct dev *d)
}
midi_del(d->midi);
*p = d->next;
-   xfree(d);
+   free(d);
 }
 
 unsigned int
@@ -1829,16 +1826,12 @@ slot_detach(struct slot *s)
}   
*ps = s->next;
if (s->mode & MODE_RECMASK) {
-   if (s->sub.encbuf)
-   xfree(s->sub.encbuf);
-   if (s->sub.resampbuf)
-   xfree(s->sub.resampbuf);
+   free(s->sub.encbuf);
+   free(s->sub.resampbuf);
}
if (s->mode & MODE_PLAY) {
-   if (s->mix.decbuf)
-   xfree(s->mix.decbuf);
-   if (s->mix.resampbuf)
-   xfree(s->mix.resampbuf);
+   free(s->mix.decbuf);
+   free(s->mix.resampbuf);
dev_mix_adjvol(s->dev);
}
 }
Index: file.c
===
RCS file: /cvs/src/usr.bin/sndiod/file.c,v
retrieving revision 1.15
diff -u -p -u -r1.15 file.c
--- file.c  27 Aug 2015 07:38:38 -  1.15
+++ file.c  12 Nov 2015 07:07:57 -
@@ -328,7 +328,7 @@ file_poll(void)
while ((f = *pf) != NULL) {
if (f->state == FILE_ZOMB) {
*pf = f->next;
-   xfree(f);
+   free(f);
} else
pf = >next;
}
Index: listen.c
===
RCS file: /cvs/src/usr.bin/sndiod/listen.c,v
retrieving revision 1.2
diff -u -p -u -r1.2 listen.c
--- listen.c13 Mar 2013 08:28:33 -  1.2
+++ listen.c12 Nov 2015 07:07:57 -
@@ -70,13 +70,12 @@ listen_close(struct listen *f)
}
*pf = f->next;
 
-   if (f->path != NULL) {
+   if (f->path != NULL)
unlink(f->path);
-   xfree(f->path);
-   }
+   free(f->path);
file_del(f->file);
close(f->fd);
-   xfree(f);
+   free(f);
 }
 
 void
Index: midi.c
===
RCS file: /cvs/src/usr.bin/sndiod/midi.c,v
retrieving revision 1.10
diff -u -p -u -r1.10 midi.c
--- midi.c  28 Sep 2013 18:49:32 -  1.10
+++ midi.c  12 Nov 2015 07:07:57 -
@@ -461,7 +461,7 @@ port_del(struct port *c)
 #endif
}
*p = c->next;
-   xfree(c);
+   free(c);
 }
 
 int
Index: opt.c
===
RCS file: /cvs/src/usr.bin/sndiod/opt.c,v
retrieving revision 1.2
diff -u -p -u -r1.2 opt.c
--- opt.c   7 Dec 2012 08:04:58 -   1.2
+++ opt.c   12 Nov 2015 07:07:57 -
@@ -136,5 +136,5 @@ opt_del(struct 

[patch] dc: array_free usage

[Michael W. Bombardieri]

Hi tech@,

In dc(1) the function array_free() knows how to skip a null pointer
so calling code doesn't need to avoid passing null.
Also, minor style (braces) clean-up.

- Michael
 

Index: bcode.c
===
RCS file: /cvs/src/usr.bin/dc/bcode.c,v
retrieving revision 1.48
diff -u -p -u -r1.48 bcode.c
--- bcode.c 3 Oct 2015 16:24:53 -   1.48
+++ bcode.c 10 Nov 2015 03:50:43 -
@@ -937,9 +937,8 @@ badd(void)
struct number   *r;
 
a = pop_number();
-   if (a == NULL) {
+   if (a == NULL)
return;
-   }
b = pop_number();
if (b == NULL) {
push_number(a);
@@ -965,9 +964,8 @@ bsub(void)
struct number   *r;
 
a = pop_number();
-   if (a == NULL) {
+   if (a == NULL)
return;
-   }
b = pop_number();
if (b == NULL) {
push_number(a);
@@ -1014,9 +1012,8 @@ bmul(void)
struct number   *r;
 
a = pop_number();
-   if (a == NULL) {
+   if (a == NULL)
return;
-   }
b = pop_number();
if (b == NULL) {
push_number(a);
@@ -1040,9 +1037,8 @@ bdiv(void)
BN_CTX  *ctx;
 
a = pop_number();
-   if (a == NULL) {
+   if (a == NULL)
return;
-   }
b = pop_number();
if (b == NULL) {
push_number(a);
@@ -1078,9 +1074,8 @@ bmod(void)
BN_CTX  *ctx;
 
a = pop_number();
-   if (a == NULL) {
+   if (a == NULL)
return;
-   }
b = pop_number();
if (b == NULL) {
push_number(a);
@@ -1116,9 +,8 @@ bdivmod(void)
BN_CTX  *ctx;
 
a = pop_number();
-   if (a == NULL) {
+   if (a == NULL)
return;
-   }
b = pop_number();
if (b == NULL) {
push_number(a);
@@ -1158,9 +1152,8 @@ bexp(void)
u_int   rscale;
 
p = pop_number();
-   if (p == NULL) {
+   if (p == NULL)
return;
-   }
a = pop_number();
if (a == NULL) {
push_number(p);
@@ -1282,9 +1275,8 @@ bsqrt(void)
 
onecount = 0;
n = pop_number();
-   if (n == NULL) {
+   if (n == NULL)
return;
-   }
if (BN_is_zero(n->number)) {
r = new_number();
push_number(r);
@@ -1325,9 +1317,8 @@ not(void)
struct number   *a;
 
a = pop_number();
-   if (a == NULL) {
+   if (a == NULL)
return;
-   }
a->scale = 0;
bn_check(BN_set_word(a->number, BN_get_word(a->number) ? 0 : 1));
push_number(a);
@@ -1345,9 +1336,8 @@ equal_numbers(void)
struct number *a, *b, *r;
 
a = pop_number();
-   if (a == NULL) {
+   if (a == NULL)
return;
-   }
b = pop_number();
if (b == NULL) {
push_number(a);
@@ -1365,9 +1355,8 @@ less_numbers(void)
struct number *a, *b, *r;
 
a = pop_number();
-   if (a == NULL) {
+   if (a == NULL)
return;
-   }
b = pop_number();
if (b == NULL) {
push_number(a);
@@ -1385,9 +1374,8 @@ lesseq_numbers(void)
struct number *a, *b, *r;
 
a = pop_number();
-   if (a == NULL) {
+   if (a == NULL)
return;
-   }
b = pop_number();
if (b == NULL) {
push_number(a);
@@ -1711,9 +1699,8 @@ eval_tos(void)
char *p;
 
p = pop_string();
-   if (p == NULL)
-   return;
-   eval_string(p);
+   if (p != NULL)
+   eval_string(p);
 }
 
 void
Index: stack.c
===
RCS file: /cvs/src/usr.bin/dc/stack.c,v
retrieving revision 1.13
diff -u -p -u -r1.13 stack.c
--- stack.c 1 Dec 2014 13:13:00 -   1.13
+++ stack.c 10 Nov 2015 03:50:43 -
@@ -62,10 +62,8 @@ stack_free_value(struct value *v)
free(v->u.string);
break;
}
-   if (v->array != NULL) {
-   array_free(v->array);
-   v->array = NULL;
-   }
+   array_free(v->array);
+   v->array = NULL;
 }
 
 /* Copy number or string content into already allocated target */
@@ -210,10 +208,8 @@ stack_popnumber(struct stack *stack)
 {
if (stack_empty(stack))
return NULL;
-   if (stack->stack[stack->sp].array != NULL) {
-   array_free(stack->stack[stack->sp].array);
-   stack->stack[stack->sp].array = NULL;
-   }
+   array_free(stack->stack[stack->sp].array);
+   stack->stack[stack->sp].array = NULL;
if (stack->stack[stack->sp].type != BCODE_NUMBER) {
warnx("not a number"); /* XXX remove */
return 

Re: unify xmalloc (was Re: [patch] cvs: retire xfree())

[Michael W. Bombardieri]

On Thu, Nov 05, 2015 at 03:50:29PM +0100, Tobias Stoeckmann wrote:
> On Thu, Nov 05, 2015 at 09:50:48AM +, Nicholas Marriott wrote:
> > I don't know why cvs and rcs xmalloc.c has ended up so different.
> 
> It's not just about cvs and rcs:
> 
> /usr/src/usr.bin/cvs/xmalloc.c
> /usr/src/usr.bin/diff/xmalloc.c
> /usr/src/usr.bin/file/xmalloc.c
> /usr/src/usr.bin/rcs/xmalloc.c
> /usr/src/usr.bin/ssh/xmalloc.c
> /usr/src/usr.bin/tmux/xmalloc.c (probably not same origin)
> 


Also note that aucat(1)'s utils.c contains xmalloc() and xfree().
Its version of xfree() contains no special logic so remove it?


Index: abuf.c
===
RCS file: /cvs/src/usr.bin/aucat/abuf.c,v
retrieving revision 1.26
diff -u -p -u -r1.26 abuf.c
--- abuf.c  21 Jan 2015 08:43:55 -  1.26
+++ abuf.c  8 Nov 2015 02:16:41 -
@@ -62,7 +62,7 @@ abuf_done(struct abuf *buf)
}
}
 #endif
-   xfree(buf->data);
+   free(buf->data);
buf->data = (void *)0xdeadbeef;
 }
 
Index: aucat.c
===
RCS file: /cvs/src/usr.bin/aucat/aucat.c,v
retrieving revision 1.149
diff -u -p -u -r1.149 aucat.c
--- aucat.c 27 Aug 2015 07:25:56 -  1.149
+++ aucat.c 8 Nov 2015 02:16:42 -
@@ -214,7 +214,7 @@ slot_new(char *path, int mode, struct ap
if (!afile_open(>afile, path, hdr,
mode == SIO_PLAY ? AFILE_FREAD : AFILE_FWRITE,
par, rate, cmax - cmin + 1)) {
-   xfree(s);
+   free(s);
return 0;
}
s->cmin = cmin;
@@ -413,15 +413,13 @@ slot_del(struct slot *s)
}
 #endif
abuf_done(>buf);
-   if (s->resampbuf)
-   xfree(s->resampbuf);
-   if (s->convbuf)
-   xfree(s->convbuf);
+   free(s->resampbuf);
+   free(s->convbuf);
}
for (ps = _list; *ps != s; ps = &(*ps)->next)
; /* nothing */
*ps = s->next;
-   xfree(s);
+   free(s);
 }
 
 static int 
@@ -672,9 +670,9 @@ dev_close(void)
if (dev_mh)
mio_close(dev_mh);
if (dev_mode & SIO_PLAY)
-   xfree(dev_pbuf);
+   free(dev_pbuf);
if (dev_mode & SIO_REC)
-   xfree(dev_rbuf);
+   free(dev_rbuf);
 }
 
 static void
@@ -999,7 +997,7 @@ offline(void)
slot_list_copy(todo, dev_pchan, dev_pbuf);
slot_list_iodo();
}
-   xfree(dev_pbuf);
+   free(dev_pbuf);
while (slot_list)
slot_del(slot_list);
return 1;
@@ -1148,7 +1146,7 @@ playrec(char *dev, int mode, int bufsz, 
 
if (dev_pstate == DEV_START)
dev_mmcstop();
-   xfree(pfds);
+   free(pfds);
dev_close();
while (slot_list)
slot_del(slot_list);
Index: utils.c
===
RCS file: /cvs/src/usr.bin/aucat/utils.c,v
retrieving revision 1.1
diff -u -p -u -r1.1 utils.c
--- utils.c 21 Jan 2015 08:43:55 -  1.1
+++ utils.c 8 Nov 2015 02:16:42 -
@@ -158,15 +158,6 @@ xmalloc(size_t size)
 }
 
 /*
- * free memory allocated with xmalloc()
- */
-void
-xfree(void *p)
-{
-   free(p);
-}
-
-/*
  * xmalloc-style strdup(3)
  */
 char *
Index: utils.h
===
RCS file: /cvs/src/usr.bin/aucat/utils.h,v
retrieving revision 1.1
diff -u -p -u -r1.1 utils.h
--- utils.h 21 Jan 2015 08:43:55 -  1.1
+++ utils.h 8 Nov 2015 02:16:42 -
@@ -29,7 +29,6 @@ void log_flush(void);
 
 void *xmalloc(size_t);
 char *xstrdup(char *);
-void xfree(void *);
 
 /*
  * Log levels:

[patch] cvs: retire xfree()

[Michael W. Bombardieri]

Hi tech@,

Function xfree() was previously removed from rcs, so drop it from
opencvs too...

http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/rcs/xmalloc.c?f=h#rev1.9

Footnote:
I noticed that rcsnum_free() is just free() so maybe that could be
removed also (not included in this patch).

- Michael


Index: add.c
===
RCS file: /cvs/src/usr.bin/cvs/add.c,v
retrieving revision 1.111
diff -u -p -u -r1.111 add.c
--- add.c   16 Jan 2015 06:40:06 -  1.111
+++ add.c   5 Nov 2015 02:49:20 -
@@ -20,6 +20,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -146,7 +147,7 @@ cvs_add_entry(struct cvs_file *cf)
entlist = cvs_ent_open(cf->file_wd);
cvs_ent_add(entlist, entry);
 
-   xfree(entry);
+   free(entry);
} else {
add_entry(cf);
}
@@ -252,7 +253,7 @@ cvs_add_tobranch(struct cvs_file *cf, ch
(void)xsnprintf(attic, PATH_MAX, "%s/%s/%s%s", repo,
CVS_PATH_ATTIC, cf->file_name, RCS_FILE_EXT);
 
-   xfree(cf->file_rpath);
+   free(cf->file_rpath);
cf->file_rpath = xstrdup(attic);
 
cf->repo_fd = open(cf->file_rpath, O_CREAT|O_RDONLY);
@@ -277,7 +278,7 @@ cvs_add_tobranch(struct cvs_file *cf, ch
if (rcs_rev_add(cf->file_rcs, RCS_HEAD_REV, msg, -1, NULL) == -1)
fatal("cvs_add_tobranch: failed to create first branch "
"revision");
-   xfree(msg);
+   free(msg);
 
if (rcs_findrev(cf->file_rcs, cf->file_rcs->rf_head) == NULL)
fatal("cvs_add_tobranch: cannot find newly added revision");
@@ -359,7 +360,7 @@ add_directory(struct cvs_file *cf)
 
entlist = cvs_ent_open(cf->file_wd);
cvs_ent_add(entlist, p);
-   xfree(p);
+   free(p);
}
}
 
@@ -381,10 +382,8 @@ add_directory(struct cvs_file *cf)
}
cvs_printf("%s\n", msg);
 
-   if (tag != NULL)
-   xfree(tag);
-   if (date != NULL)
-   xfree(date);
+   free(tag);
+   free(date);
 
cvs_get_repository_name(cf->file_path, repo, PATH_MAX);
line_list = cvs_trigger_getlines(CVS_PATH_LOGINFO, repo);
@@ -400,8 +399,7 @@ add_directory(struct cvs_file *cf)
 
cvs_trigger_freeinfo(_info);
cvs_trigger_freelist(line_list);
-   if (loginfo != NULL)
-   xfree(loginfo);
+   free(loginfo);
}
}
 
@@ -564,5 +562,5 @@ add_entry(struct cvs_file *cf)
entlist = cvs_ent_open(cf->file_wd);
cvs_ent_add(entlist, entry);
}
-   xfree(entry);
+   free(entry);
 }
Index: admin.c
===
RCS file: /cvs/src/usr.bin/cvs/admin.c,v
retrieving revision 1.65
diff -u -p -u -r1.65 admin.c
--- admin.c 16 Jan 2015 06:40:06 -  1.65
+++ admin.c 5 Nov 2015 02:49:20 -
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -304,8 +305,8 @@ cvs_admin_local(struct cvs_file *cf)
while (!TAILQ_EMPTY(&(cf->file_rcs->rf_access))) {
rap = TAILQ_FIRST(&(cf->file_rcs->rf_access));
TAILQ_REMOVE(&(cf->file_rcs->rf_access), rap, ra_list);
-   xfree(rap->ra_name);
-   xfree(rap);
+   free(rap->ra_name);
+   free(rap);
}
/* no synced anymore */
cf->file_rcs->rf_flags &= ~RCS_SYNCED;
Index: annotate.c
===
RCS file: /cvs/src/usr.bin/cvs/annotate.c,v
retrieving revision 1.64
diff -u -p -u -r1.64 annotate.c
--- annotate.c  16 Jan 2015 06:40:06 -  1.64
+++ annotate.c  5 Nov 2015 02:49:20 -
@@ -235,7 +235,7 @@ cvs_annotate_local(struct cvs_file *cf)
p[line->l_len] = '\0';
 
if (line->l_needsfree)
-   xfree(line->l_line);
+   free(line->l_line);
line->l_line = p;
line->l_len++;
line->l_needsfree = 1;
@@ -244,9 +244,9 @@ cvs_annotate_local(struct cvs_file *cf)
line->l_delta->rd_author, date, line->l_line);
 
if (line->l_needsfree)
-   xfree(line->l_line);
-   xfree(line);
+   free(line->l_line);
+   free(line);
}
 
-   xfree(alines);
+   free(alines);
 }
Index: buf.c
===
RCS 

Re: [patch] cvs: retire xfree()

[Michael W. Bombardieri]

> Good catch, thanks.
> 
> My eyes glazed over a little while reviewing this, but tentative ok
> mmcc@
> 
> Below is what I consider a better refactoring of buf_free(). It makes it
> NULL-safe and has more classic free function logic.

I didn't include buf_free() in my patch but that looks good.

> > Footnote:
> > I noticed that rcsnum_free() is just free() so maybe that could be
> > removed also (not included in this patch).
> 
> I'll have to look, but this sounds like a good idea to me.

In rcs(1) rcsnum_free() is more complicated since rn_id is dynamically 
allocated.

> Index: buf.c
> ===
> RCS file: /cvs/src/usr.bin/cvs/buf.c,v
> retrieving revision 1.82
> diff -u -p -r1.82 buf.c
> --- buf.c 5 Feb 2015 12:59:57 -   1.82
> +++ buf.c 5 Nov 2015 05:08:57 -
> @@ -119,15 +119,15 @@ buf_load_fd(int fd)
>  void
>  buf_free(BUF *b)
>  {
> - if (b->cb_buf != NULL)
> - xfree(b->cb_buf);
> - xfree(b);
> + if (b != NULL) {
> + free(b->cb_buf);
> + free(b);
> + }
>  }
>  
>  /*
>   * Free the buffer 's structural information but do not free the contents
> - * of the buffer.  Instead, they are returned and should be freed later using
> - * xfree().
> + * of the buffer.  Instead, they are returned and should be freed later.
>   */
>  void *
>  buf_release(BUF *b)
> @@ -135,7 +135,7 @@ buf_release(BUF *b)
>   void *tmp;
>  
>   tmp = b->cb_buf;
> - xfree(b);
> + free(b);
>   return (tmp);
>  }
>  

Re: [PATCH] rcs: buf_free/rcsnum_free

[Michael W. Bombardieri]

I remember reading a while ago that people were thinking to
use opencvs for anoncvs mirrors. I don't see any mirrors
reporting as opencvs though...

$ for h in $hosts; do printf "%-30s" $h; export CVSROOT="anoncvs@$h:/cvs"; cvs 
version | grep -v Client; done ;
anoncvs.au.openbsd.orgServer: Concurrent Versions System (CVS) 1.11.1p1 
(client/server)
ftp5.eu.openbsd.org   Server: Concurrent Versions System (CVS) 1.11.1p1 
(client/server)
anoncvs1.ca.openbsd.org   Server: Concurrent Versions System (CVS) 1.11.1p1 
(client/server)
anoncvs.comstyle.com  Server: Concurrent Versions System (CVS) 1.11.1p1 
(client/server)
openbsd.cs.toronto.eduServer: Concurrent Versions System (CVS) 1.11.1p1 
(client/server)
anoncvs.fr.openbsd.orgServer: Concurrent Versions System (CVS) 1.11.1p1 
(client/server)
openbsd.cs.fau.de Server: Concurrent Versions System (CVS) 1.11.1p1 
(client/server)
mirror.osn.de Server: Concurrent Versions System (CVS) 1.11.1p1 
(client/server)
ftp.hostserver.de Server: Concurrent Versions System (CVS) 1.11.1p1 
(client/server)
anoncvs.jp.openbsd.orgServer: Concurrent Versions System (CVS) 1.11.1p1 
(client/server)
openbsd.park.rambler.ru   Server: Concurrent Versions System (CVS) 1.11.1p1 
(client/server)
anoncvs.obsd.si   Server: Concurrent Versions System (CVS) 1.11.1p1 
(client/server)
anoncvs.eu.openbsd.orgServer: Concurrent Versions System (CVS) 1.11.1p1 
(client/server)
anoncvs.spacehopper.org   Server: Concurrent Versions System (CVS) 1.11.1p1 
(client/server)
anoncvs1.usa.openbsd.org  Server: Concurrent Versions System (CVS) 1.11.1p1 
(client/server)
anoncvs3.usa.openbsd.org  Server: Concurrent Versions System (CVS) 1.11.1p1 
(client/server)
mirror.planetunix.net Server: Concurrent Versions System (CVS) 1.12.13 
(client/server)

On Fri, Oct 30, 2015 at 10:26:31AM +, Nicholas Marriott wrote:
> I think it is never going to rise from the dead.
> 
>  Original message 
> From: Tobias Stoeckmann <tob...@stoeckmann.org> 
> Date:30/10/2015  10:06  (GMT+00:00) 
> To: "Michael W. Bombardieri" <m...@ii.net> 
> Cc: Nicholas Marriott <nicholas.marri...@gmail.com>,tech@openbsd.org 
> Subject: Re: [PATCH] rcs: buf_free/rcsnum_free 
> 
> On Fri, Oct 30, 2015 at 08:52:02AM +0800, Michael W. Bombardieri wrote:
> > Sorry. Here is new diff. Hopefully I haven't missed anything else.
> 
> You missed OpenCVS, which shares the same code base.
> 
> But is OpenCVS worth it anymore?
> Even a harsher question: Is it time to tedu it?

[PATCH] rcs: buf_free/rcsnum_free

[Michael W. Bombardieri]

Hi tech@,

In case it's considered useful...
Submitting patch to shave a few lines from rcs(1) by allowing
buf_free() and rcsnum_free() to ignore NULL pointer.

- Michael


Index: buf.c
===
RCS file: /cvs/src/usr.bin/rcs/buf.c,v
retrieving revision 1.25
diff -u -p -u -r1.25 buf.c
--- buf.c   13 Jun 2015 20:15:21 -  1.25
+++ buf.c   29 Oct 2015 09:06:01 -
@@ -138,6 +138,8 @@ out:
 void
 buf_free(BUF *b)
 {
+   if (b == NULL)
+   return;
free(b->cb_buf);
free(b);
 }
Index: ci.c
===
RCS file: /cvs/src/usr.bin/rcs/ci.c,v
retrieving revision 1.222
diff -u -p -u -r1.222 ci.c
--- ci.c5 Sep 2015 09:38:23 -   1.222
+++ ci.c29 Oct 2015 09:06:01 -
@@ -369,12 +369,9 @@ checkin_diff_file(struct checkin_params 
 
return (b3);
 out:
-   if (b1 != NULL)
-   buf_free(b1);
-   if (b2 != NULL)
-   buf_free(b2);
-   if (b3 != NULL)
-   buf_free(b3);
+   buf_free(b1);
+   buf_free(b2);
+   buf_free(b3);
free(path1);
free(path2);
 
Index: diff3.c
===
RCS file: /cvs/src/usr.bin/rcs/diff3.c,v
retrieving revision 1.37
diff -u -p -u -r1.37 diff3.c
--- diff3.c 5 Sep 2015 09:47:08 -   1.37
+++ diff3.c 29 Oct 2015 09:06:01 -
@@ -234,14 +234,10 @@ merge_diff3(char **av, int flags)
warnx("warning: overlaps or other problems during merge");
 
 out:
-   if (b2 != NULL)
-   buf_free(b2);
-   if (b3 != NULL)
-   buf_free(b3);
-   if (d1 != NULL)
-   buf_free(d1);
-   if (d2 != NULL)
-   buf_free(d2);
+   buf_free(b2);
+   buf_free(b3);
+   buf_free(d1);
+   buf_free(d2);
 
(void)unlink(path1);
(void)unlink(path2);
@@ -354,14 +350,10 @@ rcs_diff3(RCSFILE *rf, char *workfile, R
warnx("warning: overlaps or other problems during merge");
 
 out:
-   if (b2 != NULL)
-   buf_free(b2);
-   if (b3 != NULL)
-   buf_free(b3);
-   if (d1 != NULL)
-   buf_free(d1);
-   if (d2 != NULL)
-   buf_free(d2);
+   buf_free(b2);
+   buf_free(b3);
+   buf_free(d1);
+   buf_free(d2);
 
(void)unlink(path1);
(void)unlink(path2);
Index: ident.c
===
RCS file: /cvs/src/usr.bin/rcs/ident.c,v
retrieving revision 1.30
diff -u -p -u -r1.30 ident.c
--- ident.c 2 Oct 2014 06:23:15 -   1.30
+++ ident.c 29 Oct 2015 09:06:01 -
@@ -156,8 +156,7 @@ ident_line(FILE *fp)
 
found++;
 out:
-   if (bp != NULL)
-   buf_free(bp);
+   buf_free(bp);
 }
 
 __dead void
Index: rcsclean.c
===
RCS file: /cvs/src/usr.bin/rcs/rcsclean.c,v
retrieving revision 1.54
diff -u -p -u -r1.54 rcsclean.c
--- rcsclean.c  16 Jan 2015 06:40:11 -  1.54
+++ rcsclean.c  29 Oct 2015 09:06:01 -
@@ -209,10 +209,8 @@ rcsclean_file(char *fname, const char *r
rcs_set_mtime(file, rcs_mtime);
 
 out:
-   if (b1 != NULL)
-   buf_free(b1);
-   if (b2 != NULL)
-   buf_free(b2);
+   buf_free(b1);
+   buf_free(b2);
if (file != NULL)
rcs_close(file);
 }
Index: rcsdiff.c
===
RCS file: /cvs/src/usr.bin/rcs/rcsdiff.c,v
retrieving revision 1.83
diff -u -p -u -r1.83 rcsdiff.c
--- rcsdiff.c   13 Jun 2015 20:15:21 -  1.83
+++ rcsdiff.c   29 Oct 2015 09:06:01 -
@@ -354,10 +354,8 @@ rcsdiff_file(RCSFILE *file, RCSNUM *rev,
 out:
if (fd != -1)
(void)close(fd);
-   if (b1 != NULL)
-   buf_free(b1);
-   if (b2 != NULL)
-   buf_free(b2);
+   buf_free(b1);
+   buf_free(b2);
free(path1);
free(path2);
 
@@ -431,10 +429,8 @@ rcsdiff_rev(RCSFILE *file, RCSNUM *rev1,
ret = diffreg(path1, path2, NULL, dflags);
 
 out:
-   if (b1 != NULL)
-   buf_free(b1);
-   if (b2 != NULL)
-   buf_free(b2);
+   buf_free(b1);
+   buf_free(b2);
free(path1);
free(path2);
 
Index: rcsmerge.c
===
RCS file: /cvs/src/usr.bin/rcs/rcsmerge.c,v
retrieving revision 1.55
diff -u -p -u -r1.55 rcsmerge.c
--- rcsmerge.c  16 Jan 2015 06:40:11 -  1.55
+++ rcsmerge.c  29 Oct 2015 09:06:01 -
@@ -173,12 +173,8 @@ rcsmerge_main(int argc, char **argv)
 
 out:
rcs_close(file);
-
-   if (rev1 != NULL)
-   rcsnum_free(rev1);
-   if (rev2 != NULL)
-   rcsnum_free(rev2);
-
+   rcsnum_free(rev1);
+   

Re: [PATCH] rcs: buf_free/rcsnum_free

[Michael W. Bombardieri]

On Thu, Oct 29, 2015 at 10:54:09AM +, Nicholas Marriott wrote:
> Hi
> 
> You missed ci.c:316 and a few in rcs.c and rcsdiff.c

Sorry. Here is new diff. Hopefully I haven't missed anything else.



Index: buf.c
===
RCS file: /cvs/src/usr.bin/rcs/buf.c,v
retrieving revision 1.25
diff -u -p -u -r1.25 buf.c
--- buf.c   13 Jun 2015 20:15:21 -  1.25
+++ buf.c   30 Oct 2015 01:11:28 -
@@ -138,6 +138,8 @@ out:
 void
 buf_free(BUF *b)
 {
+   if (b == NULL)
+   return;
free(b->cb_buf);
free(b);
 }
Index: ci.c
===
RCS file: /cvs/src/usr.bin/rcs/ci.c,v
retrieving revision 1.222
diff -u -p -u -r1.222 ci.c
--- ci.c5 Sep 2015 09:38:23 -   1.222
+++ ci.c30 Oct 2015 01:11:28 -
@@ -313,8 +313,7 @@ checkin_main(int argc, char **argv)
}
 
rcs_close(pb.file);
-   if (rev_str != NULL)
-   rcsnum_free(pb.newrev);
+   rcsnum_free(pb.newrev);
pb.newrev = NULL;
}
 
@@ -369,12 +368,9 @@ checkin_diff_file(struct checkin_params 
 
return (b3);
 out:
-   if (b1 != NULL)
-   buf_free(b1);
-   if (b2 != NULL)
-   buf_free(b2);
-   if (b3 != NULL)
-   buf_free(b3);
+   buf_free(b1);
+   buf_free(b2);
+   buf_free(b3);
free(path1);
free(path2);
 
Index: diff3.c
===
RCS file: /cvs/src/usr.bin/rcs/diff3.c,v
retrieving revision 1.37
diff -u -p -u -r1.37 diff3.c
--- diff3.c 5 Sep 2015 09:47:08 -   1.37
+++ diff3.c 30 Oct 2015 01:11:28 -
@@ -234,14 +234,10 @@ merge_diff3(char **av, int flags)
warnx("warning: overlaps or other problems during merge");
 
 out:
-   if (b2 != NULL)
-   buf_free(b2);
-   if (b3 != NULL)
-   buf_free(b3);
-   if (d1 != NULL)
-   buf_free(d1);
-   if (d2 != NULL)
-   buf_free(d2);
+   buf_free(b2);
+   buf_free(b3);
+   buf_free(d1);
+   buf_free(d2);
 
(void)unlink(path1);
(void)unlink(path2);
@@ -354,14 +350,10 @@ rcs_diff3(RCSFILE *rf, char *workfile, R
warnx("warning: overlaps or other problems during merge");
 
 out:
-   if (b2 != NULL)
-   buf_free(b2);
-   if (b3 != NULL)
-   buf_free(b3);
-   if (d1 != NULL)
-   buf_free(d1);
-   if (d2 != NULL)
-   buf_free(d2);
+   buf_free(b2);
+   buf_free(b3);
+   buf_free(d1);
+   buf_free(d2);
 
(void)unlink(path1);
(void)unlink(path2);
Index: ident.c
===
RCS file: /cvs/src/usr.bin/rcs/ident.c,v
retrieving revision 1.30
diff -u -p -u -r1.30 ident.c
--- ident.c 2 Oct 2014 06:23:15 -   1.30
+++ ident.c 30 Oct 2015 01:11:28 -
@@ -156,8 +156,7 @@ ident_line(FILE *fp)
 
found++;
 out:
-   if (bp != NULL)
-   buf_free(bp);
+   buf_free(bp);
 }
 
 __dead void
Index: rcs.c
===
RCS file: /cvs/src/usr.bin/rcs/rcs.c,v
retrieving revision 1.83
diff -u -p -u -r1.83 rcs.c
--- rcs.c   13 Jun 2015 20:15:21 -  1.83
+++ rcs.c   30 Oct 2015 01:11:28 -
@@ -177,10 +177,8 @@ rcs_close(RCSFILE *rfp)
free(rlp);
}
 
-   if (rfp->rf_head != NULL)
-   rcsnum_free(rfp->rf_head);
-   if (rfp->rf_branch != NULL)
-   rcsnum_free(rfp->rf_branch);
+   rcsnum_free(rfp->rf_head);
+   rcsnum_free(rfp->rf_branch);
 
if (rfp->rf_file != NULL)
fclose(rfp->rf_file);
@@ -1406,10 +1404,8 @@ rcs_freedelta(struct rcs_delta *rdp)
 {
struct rcs_branch *rb;
 
-   if (rdp->rd_num != NULL)
-   rcsnum_free(rdp->rd_num);
-   if (rdp->rd_next != NULL)
-   rcsnum_free(rdp->rd_next);
+   rcsnum_free(rdp->rd_num);
+   rcsnum_free(rdp->rd_next);
 
free(rdp->rd_author);
free(rdp->rd_locker);
Index: rcsclean.c
===
RCS file: /cvs/src/usr.bin/rcs/rcsclean.c,v
retrieving revision 1.54
diff -u -p -u -r1.54 rcsclean.c
--- rcsclean.c  16 Jan 2015 06:40:11 -  1.54
+++ rcsclean.c  30 Oct 2015 01:11:28 -
@@ -209,10 +209,8 @@ rcsclean_file(char *fname, const char *r
rcs_set_mtime(file, rcs_mtime);
 
 out:
-   if (b1 != NULL)
-   buf_free(b1);
-   if (b2 != NULL)
-   buf_free(b2);
+   buf_free(b1);
+   buf_free(b2);
if (file != NULL)
rcs_close(file);
 }
Index: rcsdiff.c
===
RCS file: 

[patch]rcs: remove a goto

[Michael W. Bombardieri]

Hi tech,

The following patch eliminates one goto in rcs(1).
The goto was used for skipping over rcs_set_description().
This can be handed in the if which already encloses rcs_set_description().

- Michael


Index: ci.c
===
RCS file: /cvs/src/usr.bin/rcs/ci.c,v
retrieving revision 1.220
diff -u -r1.220 ci.c
--- ci.c13 Jun 2015 20:15:21 -  1.220
+++ ci.c15 Jun 2015 01:20:46 -
@@ -620,17 +620,13 @@
checkin_keywordscan(bp, pb-newrev,
pb-date, pb-state, pb-author);
 
-   if (pb-flags  CI_SKIPDESC)
-   goto skipdesc;
-
/* Get description from user */
-   if (pb-description == NULL 
+   if (!(pb-flags  CI_SKIPDESC) 
+   pb-description == NULL 
rcs_set_description(pb-file, NULL) == -1) {
warn(%s, pb-filename);
return (-1);
}
-
-skipdesc:
 
/*
 * If the user had specified a zero-ending revision number e.g. 4.0

awk: xfree() macro

[Michael W. Bombardieri]

Hi tech,

Forwarding this in case it is considered useful.
Trying to clean up xfree() macro in awk.
Remove check for NULL before calling free().
As a side effect, we might set something to NULL which is already NULL (but I 
suppose that's ok).

- Michael


Index: awk.h
===
RCS file: /cvs/src/usr.bin/awk/awk.h,v
retrieving revision 1.13
diff -u -p -u -r1.13 awk.h
--- awk.h   6 Oct 2008 20:38:33 -   1.13
+++ awk.h   25 Feb 2015 05:29:37 -
@@ -31,7 +31,11 @@ typedef double   Awkfloat;
 
 typedefunsigned char uschar;
 
-#definexfree(a){ if ((a) != NULL) { free((void *) (a)); (a) = 
NULL; } }
+#definexfree(a)   \
+do {  \
+   free((void *)(a)); \
+   (a) = NULL;\
+} while (0)
 
 #defineNN(p)   ((p) ? (p) : (null))  /* guaranteed non-null for 
dprintf 
 */

[patch] libssl/src/ssl/ssl_rsa.c

[Michael W. Bombardieri]

Hi tech@,

In case this is considered important enough...
Remove unused ret from SSL_use_PrivateKey().

- Michael


Index: ssl_rsa.c
===
RCS file: /cvs/src/lib/libssl/src/ssl/ssl_rsa.c,v
retrieving revision 1.11
diff -u -r1.11 ssl_rsa.c
--- ssl_rsa.c   17 Apr 2014 21:37:37 -  1.11
+++ ssl_rsa.c   9 May 2014 03:46:58 -
@@ -273,8 +273,6 @@
 int
 SSL_use_PrivateKey(SSL *ssl, EVP_PKEY *pkey)
 {
-   int ret;
-
if (pkey == NULL) {
SSLerr(SSL_F_SSL_USE_PRIVATEKEY, ERR_R_PASSED_NULL_PARAMETER);
return (0);
@@ -283,8 +281,7 @@
SSLerr(SSL_F_SSL_USE_PRIVATEKEY, ERR_R_MALLOC_FAILURE);
return (0);
}
-   ret = ssl_set_pkey(ssl-cert, pkey);
-   return (ret);
+   return (ssl_set_pkey(ssl-cert, pkey));
 }
 
 #ifndef OPENSSL_NO_STDIO

patch: libssl/src/ssl/t1_enc.c

[Michael W. Bombardieri]

Hi tech@, 

Submitting patch to simplify code around free(3) in libssl.
free() already handles the NULL case. 

Does this look ok?

- Michael


Index: t1_enc.c
===
RCS file: /cvs/src/lib/libssl/src/ssl/t1_enc.c,v
retrieving revision 1.26
diff -u -r1.26 t1_enc.c
--- t1_enc.c21 Apr 2014 16:34:43 -  1.26
+++ t1_enc.c24 Apr 2014 05:29:52 -
@@ -1141,10 +1141,8 @@
SSLerr(SSL_F_TLS1_EXPORT_KEYING_MATERIAL, ERR_R_MALLOC_FAILURE);
rv = 0;
 ret:
-   if (buff != NULL)
-   free(buff);
-   if (val != NULL)
-   free(val);
+   free(buff);
+   free(val);
return (rv);
 }
 

small patch: CRYPTO_memcmp

[Michael W. Bombardieri]

Hi tech@,

Sending this patch for comment...

CRYPTO_memcmp() is different to memcmp() because it can only check
for equality, not greater-than/less-than.
If we check the string in reverse order we can remove a variable
from the comparison loop.

Does this look ok?

- Michael



Index: cryptlib.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/cryptlib.c,v
retrieving revision 1.23
diff -u -r1.23 cryptlib.c
--- cryptlib.c  21 Apr 2014 11:19:28 -  1.23
+++ cryptlib.c  23 Apr 2014 01:19:39 -
@@ -727,15 +727,13 @@
 }
 
 int
-CRYPTO_memcmp(const void *in_a, const void *in_b, size_t len)
+CRYPTO_memcmp(const void *in_a, const void *in_b, size_t n)
 {
-   size_t i;
const unsigned char *a = in_a;
const unsigned char *b = in_b;
unsigned char x = 0;
 
-   for (i = 0; i  len; i++)
-   x |= a[i] ^ b[i];
-
+   while (n--  0)
+   x |= a[n] ^ b[n];
return x;
 }

[PATCH] Potential bug fix for opencvs annotate

[Michael W. Bombardieri]

Hi,

I'm re-posting this in hope. :)

http://marc.info/?l=openbsd-techm=135698142814632w=2

Please let me know if I can provide any further info.

- Michael

[PATCH] knf src/sys/net/if_pppoe.c

[Michael W. Bombardieri]

Hi tech,

I have a minor KNF patch for if_pppoe.c...
Submitting this in case anyone is interested.

- Michael


Index: if_pppoe.c
===
RCS file: /cvs/src/sys/net/if_pppoe.c,v
retrieving revision 1.35
diff -u -r1.35 if_pppoe.c
--- if_pppoe.c  11 Apr 2012 17:42:53 -  1.35
+++ if_pppoe.c  11 Mar 2013 02:18:04 -
@@ -389,7 +389,8 @@
 }
 
 /* Analyze and handle a single received packet while not in session state. */
-static void pppoe_dispatch_disc_pkt(struct mbuf *m, int off)
+static void
+pppoe_dispatch_disc_pkt(struct mbuf *m, int off)
 {
struct pppoe_softc *sc;
struct pppoehdr *ph;
@@ -1108,9 +1109,8 @@
PPPOE_ADD_16(p, l1);
memcpy(p, sc-sc_service_name, l1);
p += l1;
-   } else {
+   } else
PPPOE_ADD_16(p, 0);
-   }
if (sc-sc_concentrator_name != NULL) {
PPPOE_ADD_16(p, PPPOE_TAG_ACNAME);
PPPOE_ADD_16(p, l2);

Re: [PATCH] awk: initialisation of struct

[Michael W. Bombardieri]

On Thu, Jan 03, 2013 at 09:23:50PM -0800, Philip Guenther wrote:
 On Thu, Jan 3, 2013 at 8:48 PM, Michael W. Bombardieri m...@ii.net wrote:
  I am submitting a patch for awk. struct Cell has 7 fields;
  the final field is an optional next pointer.
 ...
  Splint identified that instances of Cell are initialised with only 6
  fields, e.g.
 
  run.c:78:24: Initializer block for falsecell has 6 fields, but Cell has 7
  fields: 2, 12, 0, 0, 0.0, 01
 
 Patches to awk should be sent to the upstream maintainer listed in
 /usr/src/usr.bin/awk/README
 
 (I would tend to defer to that person's opinion on points of C style...)
 

bwk@ got back to me. He will include this style change in awk.

 
 Philip Guenther

[PATCH] awk: initialisation of struct

[Michael W. Bombardieri]

Hi,

I am submitting a patch for awk. struct Cell has 7 fields;
the final field is an optional next pointer.

 79 typedef struct Cell {
 80 uschar  ctype;  /* OCELL, OBOOL, OJUMP, etc. */
 81 uschar  csub;   /* CCON, CTEMP, CFLD, etc. */
 82 char*nval;  /* name, for variables only */
 83 char*sval;  /* string value */
 84 Awkfloat fval;  /* value as number */
 85 int  tval;  /* type info: STR|NUM|ARR|FCN|FLD|CON|DONTFREE */
 86 struct Cell *cnext; /* ptr to next if chained */
 87 } Cell;

Splint identified that instances of Cell are initialised with only 6
fields, e.g.

run.c:78:24: Initializer block for falsecell has 6 fields, but Cell has 7
fields: 2, 12, 0, 0, 0.0, 01

The following patch adds the 7th element as a NULL pointer to next list
element. Also, make Cell.nval and Cell.sval use NULL instead of 0.

- Michael


Index: run.c
===
RCS file: /cvs/src/usr.bin/awk/run.c,v
retrieving revision 1.33
diff -u -r1.33 run.c
--- run.c   28 Sep 2011 19:27:18 -  1.33
+++ run.c   4 Jan 2013 04:42:15 -
@@ -73,23 +73,23 @@
 Node   *winner = NULL; /* root of parse tree */
 Cell   *tmps;  /* free temporary cells for execution */
 
-static Celltruecell={ OBOOL, BTRUE, 0, 0, 1.0, NUM };
+static Celltruecell={ OBOOL, BTRUE, NULL, NULL, 1.0, NUM, NULL };
 Cell   *True   = truecell;
-static Cellfalsecell   ={ OBOOL, BFALSE, 0, 0, 0.0, NUM };
+static Cellfalsecell   ={ OBOOL, BFALSE, NULL, NULL, 0.0, NUM, NULL };
 Cell   *False  = falsecell;
-static Cellbreakcell   ={ OJUMP, JBREAK, 0, 0, 0.0, NUM };
+static Cellbreakcell   ={ OJUMP, JBREAK, NULL, NULL, 0.0, NUM, NULL };
 Cell   *jbreak = breakcell;
-static Cellcontcell={ OJUMP, JCONT, 0, 0, 0.0, NUM };
+static Cellcontcell={ OJUMP, JCONT, NULL, NULL, 0.0, NUM, NULL };
 Cell   *jcont  = contcell;
-static Cellnextcell={ OJUMP, JNEXT, 0, 0, 0.0, NUM };
+static Cellnextcell={ OJUMP, JNEXT, NULL, NULL, 0.0, NUM, NULL };
 Cell   *jnext  = nextcell;
-static Cellnextfilecell={ OJUMP, JNEXTFILE, 0, 0, 0.0, NUM };
+static Cellnextfilecell={ OJUMP, JNEXTFILE, NULL, NULL, 0.0, NUM, NULL 
};
 Cell   *jnextfile  = nextfilecell;
-static Cellexitcell={ OJUMP, JEXIT, 0, 0, 0.0, NUM };
+static Cellexitcell={ OJUMP, JEXIT, NULL, NULL, 0.0, NUM, NULL };
 Cell   *jexit  = exitcell;
-static Cellretcell ={ OJUMP, JRET, 0, 0, 0.0, NUM };
+static Cellretcell ={ OJUMP, JRET, NULL, NULL, 0.0, NUM, NULL };
 Cell   *jret   = retcell;
-static Celltempcell={ OCELL, CTEMP, 0, , 0.0, NUM|STR|DONTFREE };
+static Celltempcell={ OCELL, CTEMP, NULL, , 0.0, 
NUM|STR|DONTFREE, NULL };
 
 Node   *curnode = NULL;/* the node being executed, for debugging */
 
@@ -224,7 +224,7 @@
 
 Cell *call(Node **a, int n)/* function call.  very kludgy and fragile */
 {
-   static Cell newcopycell = { OCELL, CCOPY, 0, , 0.0, NUM|STR|DONTFREE 
};
+   static Cell newcopycell = { OCELL, CCOPY, NULL, , 0.0, 
NUM|STR|DONTFREE, NULL };
int i, ncall, ndef;
int freed = 0; /* handles potential double freeing when fcn  param 
share a tempcell */
Node *x;

[PATCH] iovec array size in pppoe

[Michael W. Bombardieri]

Hi,

I am submitting a small patch for pppoe...

The iovec array in send_padi() can contain either 7 or 8 elements.
The following patch declares the array as having size of 8 rather
than 10.

Does this look OK?

- Michael


Index: client.c
===
RCS file: /cvs/src/usr.sbin/pppoe/client.c,v
retrieving revision 1.24
diff -u -r1.24 client.c
--- client.c5 Nov 2011 09:20:36 -   1.24
+++ client.c3 Jan 2013 03:33:00 -
@@ -144,7 +144,7 @@
 static int
 send_padi(int fd, struct ether_addr *ea, u_int8_t *srv)
 {
-   struct iovec iov[10];
+   struct iovec iov[8];
struct pppoe_header ph = {
PPPOE_VERTYPE(1, 1),
PPPOE_CODE_PADI, 0, 0

[PATCH] minor knf in sys/net/bpf.c

[Michael W. Bombardieri]

Hi,

I am submitting a minor patch for sys/net/bpf.c.

* Wrap ROTATE_BUFFERS() macro in do-while according to knf,
  so it could be used in an if statement

No binary change on i386.
I'm not sure if this is important enough...

- Michael


Index: bpf.c
===
RCS file: /cvs/src/sys/net/bpf.c,v
retrieving revision 1.83
diff -u -r1.83 bpf.c
--- bpf.c   28 Dec 2012 17:52:06 -  1.83
+++ bpf.c   2 Jan 2013 05:09:58 -
@@ -369,12 +369,13 @@
  * into the hold slot, and the free buffer into the store slot.
  * Zero the length of the new store buffer.
  */
-#define ROTATE_BUFFERS(d) \
-   (d)-bd_hbuf = (d)-bd_sbuf; \
-   (d)-bd_hlen = (d)-bd_slen; \
-   (d)-bd_sbuf = (d)-bd_fbuf; \
-   (d)-bd_slen = 0; \
-   (d)-bd_fbuf = 0;
+#define ROTATE_BUFFERS(d) do { \
+   (d)-bd_hbuf = (d)-bd_sbuf; \
+   (d)-bd_hlen = (d)-bd_slen; \
+   (d)-bd_sbuf = (d)-bd_fbuf; \
+   (d)-bd_slen = 0; \
+   (d)-bd_fbuf = 0; \
+   } while (0)
 /*
  *  bpfread - read next chunk of packets from buffers
  */