move more code under UAUDIO_DEBUG
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
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
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'
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()
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
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
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
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
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
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
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
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()
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
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
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()
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
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()
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()
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()
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
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()
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)
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()
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()
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
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
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
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
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()
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()
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()
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
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
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
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
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
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)
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
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
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
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
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
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
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
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
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)
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
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
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()
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?
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
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
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?
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
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
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
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()
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
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__
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__
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
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
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
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)
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
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
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
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
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
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
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
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)
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
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
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*
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()
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
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
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
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
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())
> > 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
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())
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()
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()
> 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
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
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
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
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
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
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
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
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
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
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
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
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
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
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 */