Re: Remove NULL checks before free()

2015-09-14 Thread Michael McConville
Ted Unangst wrote:
> Michael McConville wrote:
> > There will probably be more similar patches to come if this is
> > acceptable. The legwork was done with the following Coccinelle script:
> 
> I think it should be split up, but I'm happy to see diffs like this.

Is this small enough? It's just lib/libc/gen.


Index: auth_subr.c
===
RCS file: /cvs/src/lib/libc/gen/auth_subr.c,v
retrieving revision 1.44
diff -u -p -r1.44 auth_subr.c
--- auth_subr.c 12 Sep 2015 15:20:14 -  1.44
+++ auth_subr.c 14 Sep 2015 13:15:28 -
@@ -284,14 +284,10 @@ auth_close(auth_session_t *as)
 */
if (as->service && as->service != defservice)
free(as->service);
-   if (as->challenge)
-   free(as->challenge);
-   if (as->class)
-   free(as->class);
-   if (as->style)
-   free(as->style);
-   if (as->name)
-   free(as->name);
+   free(as->challenge);
+   free(as->class);
+   free(as->style);
+   free(as->name);
 
free(as);
return (s);
@@ -466,8 +462,7 @@ auth_setitem(auth_session_t *as, auth_it
return (0);
if (value != NULL && (value = strdup(value)) == NULL)
return (-1);
-   if (as->challenge)
-   free(as->challenge);
+   free(as->challenge);
as->challenge = value;
return (0);
 
@@ -476,8 +471,7 @@ auth_setitem(auth_session_t *as, auth_it
return (0);
if (value != NULL && (value = strdup(value)) == NULL)
return (-1);
-   if (as->class)
-   free(as->class);
+   free(as->class);
as->class = value;
return (0);
 
@@ -486,8 +480,7 @@ auth_setitem(auth_session_t *as, auth_it
return (0);
if (value != NULL && (value = strdup(value)) == NULL)
return (-1);
-   if (as->name)
-   free(as->name);
+   free(as->name);
as->name = value;
return (0);
 
@@ -509,8 +502,7 @@ auth_setitem(auth_session_t *as, auth_it
if (value == NULL || strchr(value, '/') != NULL ||
(value = strdup(value)) == NULL)
return (-1);
-   if (as->style)
-   free(as->style);
+   free(as->style);
as->style = value;
return (0);
 
Index: authenticate.c
===
RCS file: /cvs/src/lib/libc/gen/authenticate.c,v
retrieving revision 1.23
diff -u -p -r1.23 authenticate.c
--- authenticate.c  12 Sep 2015 15:20:14 -  1.23
+++ authenticate.c  14 Sep 2015 13:15:28 -
@@ -259,8 +259,7 @@ auth_approval(auth_session_t *as, login_
login_close(lc);
syslog(LOG_ERR, "%m");
warn(NULL);
-   if (approve)
-   free(approve);
+   free(approve);
return (0);
}
 
@@ -294,8 +293,7 @@ auth_approval(auth_session_t *as, login_
lc->lc_class, type, (char *)NULL);
 
 out:
-   if (approve)
-   free(approve);
+   free(approve);
if (close_lc_on_exit)
login_close(lc);
 
Index: fts.c
===
RCS file: /cvs/src/lib/libc/gen/fts.c,v
retrieving revision 1.51
diff -u -p -r1.51 fts.c
--- fts.c   12 Sep 2015 13:32:24 -  1.51
+++ fts.c   14 Sep 2015 13:15:28 -
@@ -228,8 +228,7 @@ fts_close(FTS *sp)
/* Free up child linked list, sort array, path buffer, stream ptr.*/
if (sp->fts_child)
fts_lfree(sp->fts_child);
-   if (sp->fts_array)
-   free(sp->fts_array);
+   free(sp->fts_array);
free(sp->fts_path);
free(sp);
 
@@ -668,8 +667,7 @@ fts_build(FTS *sp, int type)
 * structures already allocated.
 */
 mem1:  saved_errno = errno;
-   if (p)
-   free(p);
+   free(p);
fts_lfree(head);
(void)closedir(dirp);
cur->fts_info = FTS_ERR;
@@ -889,8 +887,7 @@ fts_sort(FTS *sp, FTSENT *head, int nite
sp->fts_nitems = nitems + 40;
if ((a = reallocarray(sp->fts_array,
sp->fts_nitems, sizeof(FTSENT *))) == NULL) {
-   if (sp->fts_array)
-   free(sp->fts_array);
+   free(sp->fts_array);
  

Re: [patch] cleaner checksum modification for pf

2015-09-14 Thread Henning Brauer
* Martin Pieuchot  [2015-09-11 13:54]:
> On 11/09/15(Fri) 13:28, Henning Brauer wrote:
> > Ryan pointed me to this diff and we briefly discussed it; we remain
> > convinced that the in-tree approach is better than this.
> Could you elaborate why?

Well we've been thru it more than once; the argument presented here
was that modifying the cksum instead of verify + recalc is better as
it wouldn't hide cksum mismatches if the cksum engines on the NICs we
offload to misbehave. After many years with the verify + recalc
approach I think it is pretty safe to say that this is of no
concern...

And given that, the approach that has less and simpler code and makes
better use of offloading wins.

there's a more elaborate discussion with exactly the same people in
teh archives from around the time the cksum rewrite hit the tree, with
the same conclusion.

-- 
Henning Brauer, h...@bsws.de, henn...@openbsd.org
BS Web Services GmbH, http://bsws.de, Full-Service ISP
Secure Hosting, Mail and DNS. Virtual & Dedicated Servers, Root to Fully Managed
Henning Brauer Consulting, http://henningbrauer.com/



mpsafe vmx(4)

2015-09-14 Thread David Gwynne
this is an attempt to make the interrupt path in vmx mpsafe.

seems to hold up under load here, but more testing would be
appreciated.

Index: if_vmx.c
===
RCS file: /cvs/src/sys/dev/pci/if_vmx.c,v
retrieving revision 1.30
diff -u -p -r1.30 if_vmx.c
--- if_vmx.c24 Jun 2015 09:40:54 -  1.30
+++ if_vmx.c14 Sep 2015 11:08:09 -
@@ -61,8 +61,9 @@ struct vmxnet3_txring {
struct mbuf *m[NTXDESC];
bus_dmamap_t dmap[NTXDESC];
struct vmxnet3_txdesc *txd;
-   u_int head;
-   u_int next;
+   u_int prod;
+   u_int cons;
+   u_int free;
u_int8_t gen;
 };
 
@@ -107,6 +108,7 @@ struct vmxnet3_softc {
bus_space_handle_t sc_ioh0;
bus_space_handle_t sc_ioh1;
bus_dma_tag_t sc_dmat;
+   void *sc_ih;
 
struct vmxnet3_txqueue sc_txq[NTXQUEUE];
struct vmxnet3_rxqueue sc_rxq[NRXQUEUE];
@@ -167,7 +169,8 @@ void vmxnet3_reset(struct vmxnet3_softc 
 int vmxnet3_init(struct vmxnet3_softc *);
 int vmxnet3_ioctl(struct ifnet *, u_long, caddr_t);
 void vmxnet3_start(struct ifnet *);
-int vmxnet3_load_mbuf(struct vmxnet3_softc *, struct mbuf *);
+int vmxnet3_load_mbuf(struct vmxnet3_softc *, struct vmxnet3_txring *,
+struct mbuf *);
 void vmxnet3_watchdog(struct ifnet *);
 void vmxnet3_media_status(struct ifnet *, struct ifmediareq *);
 int vmxnet3_media_change(struct ifnet *);
@@ -239,8 +242,8 @@ vmxnet3_attach(struct device *parent, st
printf(": failed to map interrupt\n");
return;
}
-   pci_intr_establish(pa->pa_pc, ih, IPL_NET, vmxnet3_intr, sc,
-   self->dv_xname);
+   sc->sc_ih = pci_intr_establish(pa->pa_pc, ih, IPL_NET | IPL_MPSAFE,
+   vmxnet3_intr, sc, self->dv_xname);
intrstr = pci_intr_string(pa->pa_pc, ih);
if (intrstr)
printf(": %s", intrstr);
@@ -466,7 +469,8 @@ vmxnet3_txinit(struct vmxnet3_softc *sc,
struct vmxnet3_txring *ring = >cmd_ring;
struct vmxnet3_comp_ring *comp_ring = >comp_ring;
 
-   ring->head = ring->next = 0;
+   ring->cons = ring->prod = 0;
+   ring->free = NTXDESC;
ring->gen = 1;
comp_ring->next = 0;
comp_ring->gen = 1;
@@ -594,16 +598,19 @@ vmxnet3_intr(void *arg)
 
if (READ_BAR1(sc, VMXNET3_BAR1_INTR) == 0)
return 0;
-   if (sc->sc_ds->event)
+
+   if (sc->sc_ds->event) {
+   KERNEL_LOCK();
vmxnet3_evintr(sc);
-#ifdef VMXNET3_STAT
-   vmxstat.intr++;
-#endif
+   KERNEL_UNLOCK();
+   }
+
if (ifp->if_flags & IFF_RUNNING) {
vmxnet3_rxintr(sc, >sc_rxq[0]);
vmxnet3_txintr(sc, >sc_txq[0]);
vmxnet3_enable_intr(sc, 0);
}
+
return 1;
 }
 
@@ -649,7 +656,12 @@ vmxnet3_txintr(struct vmxnet3_softc *sc,
struct vmxnet3_comp_ring *comp_ring = >comp_ring;
struct vmxnet3_txcompdesc *txcd;
struct ifnet *ifp = >sc_arpcom.ac_if;
-   u_int sop;
+   bus_dmamap_t map;
+   struct mbuf *m;
+   u_int cons;
+   u_int free = 0;
+
+   cons = ring->cons;
 
for (;;) {
txcd = _ring->txcd[comp_ring->next];
@@ -664,21 +676,32 @@ vmxnet3_txintr(struct vmxnet3_softc *sc,
comp_ring->gen ^= 1;
}
 
-   sop = ring->next;
-   if (ring->m[sop] == NULL)
-   panic("%s: NULL ring->m[%u]", __func__, sop);
-   m_freem(ring->m[sop]);
-   ring->m[sop] = NULL;
-   bus_dmamap_unload(sc->sc_dmat, ring->dmap[sop]);
-   ring->next = (letoh32((txcd->txc_word0 >>
+   m = ring->m[cons];
+   ring->m[cons] = NULL;
+
+   KASSERT(m != NULL);
+
+   map = ring->dmap[cons];
+   free += map->dm_nsegs;
+   bus_dmamap_unload(sc->sc_dmat, map);
+   m_freem(m);
+
+   cons = (letoh32((txcd->txc_word0 >>
VMXNET3_TXC_EOPIDX_S) & VMXNET3_TXC_EOPIDX_M) + 1)
% NTXDESC;
-
-   ifp->if_flags &= ~IFF_OACTIVE;
}
-   if (ring->head == ring->next)
+
+   ring->cons = cons;
+
+   if (atomic_add_int_nv(>free, free) == NTXDESC)
ifp->if_timer = 0;
-   vmxnet3_start(ifp);
+
+   if (ISSET(ifp->if_flags, IFF_OACTIVE)) {
+   KERNEL_LOCK();
+   CLR(ifp->if_flags, IFF_OACTIVE);
+   vmxnet3_start(ifp);
+   KERNEL_UNLOCK();
+   }
 }
 
 void
@@ -911,6 +934,8 @@ vmxnet3_stop(struct ifnet *ifp)
 
WRITE_CMD(sc, VMXNET3_CMD_DISABLE);
 
+   intr_barrier(sc->sc_ih);
+
for (queue = 0; queue < NTXQUEUE; queue++)
vmxnet3_txstop(sc, >sc_txq[queue]);
for (queue = 0; queue < NRXQUEUE; queue++)
@@ -944,6 +969,11 @@ vmxnet3_init(struct vmxnet3_softc 

Re: Remove useless NULL casts from mv(1)

2015-09-14 Thread Philip Guenther
On Mon, Sep 14, 2015 at 3:39 PM, Michael McConville
 wrote:
...
> -   execl(_PATH_CP, "cp", "-PRp", "--", from, to, (char *)NULL);
> +   execl(_PATH_CP, "cp", "-PRp", "--", from, to, NULL);

Nope, this is a case in C where the cast is necessary.  To quote style(9):

 NULL is the preferred null pointer constant.  Use NULL instead of
 (type *)0 or (type *)NULL in all cases except for arguments to variadic
 functions where the compiler does not know the type.


Philip Guenther



Re: Remove useless NULL casts from mv(1)

2015-09-14 Thread Michael McConville
Nicholas Marriott wrote:
> These used to be necessary but now that NULL is (void *)0 they
> aren't.
> 
> I'd be inclined to keep them anyway - with the cast it is clearly not a
> bug, without is not so clear. There are also loads of them and it'd be a
> lot of churn to fix them all, for no real advantage. But I don't feel
> strongly about it, others may disagree.

No strong feelings here either, and apparently style(9) suggests the
cast. It was just something I came across while reading.

> On Mon, Sep 14, 2015 at 09:39:48AM -0400, Michael McConville wrote:
> > Index: mv.c
> > ===
> > RCS file: /cvs/src/bin/mv/mv.c,v
> > retrieving revision 1.40
> > diff -u -p -r1.40 mv.c
> > --- mv.c24 Aug 2015 00:10:59 -  1.40
> > +++ mv.c14 Sep 2015 13:38:13 -
> > @@ -348,7 +348,7 @@ copy(char *from, char *to)
> > pid_t pid;
> >  
> > if ((pid = vfork()) == 0) {
> > -   execl(_PATH_CP, "cp", "-PRp", "--", from, to, (char *)NULL);
> > +   execl(_PATH_CP, "cp", "-PRp", "--", from, to, NULL);
> > warn("%s", _PATH_CP);
> > _exit(1);
> > }
> > @@ -366,7 +366,7 @@ copy(char *from, char *to)
> > return (1);
> > }
> > if (!(pid = vfork())) {
> > -   execl(_PATH_RM, "cp", "-rf", "--", from, (char *)NULL);
> > +   execl(_PATH_RM, "cp", "-rf", "--", from, NULL);
> > warn("%s", _PATH_RM);
> > _exit(1);
> > }
> > 



Re: Purge route entries when an address is removed

2015-09-14 Thread Chris Cappuccio
Martin Pieuchot [m...@openbsd.org] wrote:
> Currently we leave RTF_STATIC route entries in the table when the
> address they are attached to is removed from a system.
> 
> That's why ifas need to be refcounted and that's why we have *a lot*
> of checks in the stack to not use cached routes attached to such ifa.
> 
> I'd like to simplify all of this by simply purging all the routes
> attached to an ifa being removed.  This behavior is coherent with
> the fact that routes *need* an ifa to be inserted in the table.
> 
> This makes the kernel simpler as it no longer try to find a new ifa
> when a route with a stale address is being used.
> 

I'm pretty sure this feature has been broken for a while. Once upon
a time in OpenBSD, this used to work:

ifconfig em0 192.168.1.2/24
route add default 192.168.1.1
ping 8.8.8.8
reply!
ifconfig em0 delete 192.168.1.2
ifconfig em1 192.168.1.2/24
ping 8.8.8.8
reply!

For some years, this has been required:

ifconfig em0 192.168.1.2/24
route add default 192.168.1.1
ping 8.8.8.8  
reply!
ifconfig em0 delete 192.168.1.2
ifconfig em1 192.168.1.2/24
**route delete default**
**route add default 192.168.1.1**
ping 8.8.8.8
reply!

It appears, what you're proposing is:

ifconfig em0 192.168.1.2/24
route add default 192.168.1.1
ping 8.8.8.8
reply!
ifconfig em0 delete 192.168.1.2
ifconfig em1 192.168.1.2/24
**route add default 192.168.1.1**
ping 8.8.8.8
reply!

As a user (only), I really like the original behavior, it seems to be
consistent with the behavior of some other systems, at least in the case
of the default router. If there was some way to do this without stupid
complexity, it might be worth considering.

In any event, what you propose seems much better than the current state
of affairs, where you end up with broken routes in the table that need
nothing more than to be removed.

Chris



Re: Remove useless NULL casts from mv(1)

2015-09-14 Thread Nicholas Marriott
These used to be necessary but now that NULL is (void *)0 they
aren't.

I'd be inclined to keep them anyway - with the cast it is clearly not a
bug, without is not so clear. There are also loads of them and it'd be a
lot of churn to fix them all, for no real advantage. But I don't feel
strongly about it, others may disagree.



On Mon, Sep 14, 2015 at 09:39:48AM -0400, Michael McConville wrote:
> Index: mv.c
> ===
> RCS file: /cvs/src/bin/mv/mv.c,v
> retrieving revision 1.40
> diff -u -p -r1.40 mv.c
> --- mv.c  24 Aug 2015 00:10:59 -  1.40
> +++ mv.c  14 Sep 2015 13:38:13 -
> @@ -348,7 +348,7 @@ copy(char *from, char *to)
>   pid_t pid;
>  
>   if ((pid = vfork()) == 0) {
> - execl(_PATH_CP, "cp", "-PRp", "--", from, to, (char *)NULL);
> + execl(_PATH_CP, "cp", "-PRp", "--", from, to, NULL);
>   warn("%s", _PATH_CP);
>   _exit(1);
>   }
> @@ -366,7 +366,7 @@ copy(char *from, char *to)
>   return (1);
>   }
>   if (!(pid = vfork())) {
> - execl(_PATH_RM, "cp", "-rf", "--", from, (char *)NULL);
> + execl(_PATH_RM, "cp", "-rf", "--", from, NULL);
>   warn("%s", _PATH_RM);
>   _exit(1);
>   }
> 



Re: D-Link DSR octeon-based models

2015-09-14 Thread Alexis de BRUYN

Hi Paul,

I have tried the last -current/octeon (and 5.7/octeon) on a D-Link 
DSR-500N (HW A1), but the bootoctlinux command failed.


Regards,

U-Boot 1.1.1 (Development build, svnversion: exported) (Build time: Sep 
 1 2010 - 16:21:09)


Warning: Board descriptor tuple not found in eeprom, using defaults
CUST_DSR500N board revision major:2, minor:0, serial #: unknown
OCTEON CN5010-SCP pass 1.1, Core clock: 300 MHz, DDR clock: 200 MHz (400 
Mhz data rate)

DRAM:  128 MB
Flash: 32 MB
Clearing DRAM.. done
BIST check passed.
Starting PCI
PCI Status: PCI 32-bit
PCI BAR 0: 0x, PCI BAR 1: Memory 0x  PCI 0xf800
Net:   octeth0, octeth1, octeth2

Hit any key to stop autoboot:  0
D-Link DSR-500N bootloader# dhcp
Interface 0 has 3 ports (RGMII)
BOOTP broadcast 1
octeth0: Up 1000 Mbps Full duplex (port  0)
*** Unhandled DHCP Option in OFFER/ACK: 28
*** Unhandled DHCP Option in OFFER/ACK: 42
*** Unhandled DHCP Option in OFFER/ACK: 28
*** Unhandled DHCP Option in OFFER/ACK: 42
DHCP client bound to address 192.168.0.39
D-Link DSR-500N bootloader# tftpboot 0 bsd.rd
Using octeth0 device
TFTP from server 192.168.0.236; our IP address is 192.168.0.39
Filename 'bsd.rd'.
Load address: 0x550
Loading: #
done
Bytes transferred = 7548970 (73302a hex), 3311 Kbytes/sec
D-Link DSR-500N bootloader# bootoctlinux
ELF file is 64 bit
Attempting to allocate memory for ELF segment: addr: 0x8100 
(adjusted to: 0x0100), size 0x764600

Allocated memory for ELF segment: addr: 0x8100, size 0x764600
Attempting to allocate memory for ELF segment: addr: 0x816ec790 
(adjusted to: 0x016ec790), size 0x2400

Error allocating memory for elf image!
## ERROR loading File!


On 02/13/14 11:52, Paul Irofti wrote:

Hi there,

I just updated the hardware section for the octeon port and added the
DSR-500 model to the supported list.

I was wondering if similar models might also work with what's in-tree.

Could people NFS-boot or at least try the ramdisk kernel on similar
models (e.g. DSR-500N, DSR-1000, DSR-1000N) and report back so we can
expand the supported list?

Thank you,
Paul



--
Alexis de BRUYN



Remove useless NULL casts from mv(1)

2015-09-14 Thread Michael McConville
Index: mv.c
===
RCS file: /cvs/src/bin/mv/mv.c,v
retrieving revision 1.40
diff -u -p -r1.40 mv.c
--- mv.c24 Aug 2015 00:10:59 -  1.40
+++ mv.c14 Sep 2015 13:38:13 -
@@ -348,7 +348,7 @@ copy(char *from, char *to)
pid_t pid;
 
if ((pid = vfork()) == 0) {
-   execl(_PATH_CP, "cp", "-PRp", "--", from, to, (char *)NULL);
+   execl(_PATH_CP, "cp", "-PRp", "--", from, to, NULL);
warn("%s", _PATH_CP);
_exit(1);
}
@@ -366,7 +366,7 @@ copy(char *from, char *to)
return (1);
}
if (!(pid = vfork())) {
-   execl(_PATH_RM, "cp", "-rf", "--", from, (char *)NULL);
+   execl(_PATH_RM, "cp", "-rf", "--", from, NULL);
warn("%s", _PATH_RM);
_exit(1);
}



Re: Floating point #define in ksh

2015-09-14 Thread Ted Unangst
Nicholas Marriott wrote:
> Here is a diff, also make %p better

that looks better. sigh. why does this even exist?



Re: Remove NULL checks before free()

2015-09-14 Thread Sebastien Marie
On Mon, Sep 14, 2015 at 09:18:35AM -0400, Michael McConville wrote:
> Ted Unangst wrote:
> > Michael McConville wrote:
> > > There will probably be more similar patches to come if this is
> > > acceptable. The legwork was done with the following Coccinelle script:
> > 
> > I think it should be split up, but I'm happy to see diffs like this.
> 
> Is this small enough? It's just lib/libc/gen.
> 

still a bit long to keep attention while reading it :)

if anybody wants to commit it, it is OK.

> Index: auth_subr.c
> ===
> RCS file: /cvs/src/lib/libc/gen/auth_subr.c,v
> retrieving revision 1.44
> diff -u -p -r1.44 auth_subr.c
> --- auth_subr.c   12 Sep 2015 15:20:14 -  1.44
> +++ auth_subr.c   14 Sep 2015 13:15:28 -
> @@ -284,14 +284,10 @@ auth_close(auth_session_t *as)
>*/
>   if (as->service && as->service != defservice)
>   free(as->service);
> - if (as->challenge)
> - free(as->challenge);
> - if (as->class)
> - free(as->class);
> - if (as->style)
> - free(as->style);
> - if (as->name)
> - free(as->name);
> + free(as->challenge);
> + free(as->class);
> + free(as->style);
> + free(as->name);
>  
>   free(as);
>   return (s);
> @@ -466,8 +462,7 @@ auth_setitem(auth_session_t *as, auth_it
>   return (0);
>   if (value != NULL && (value = strdup(value)) == NULL)
>   return (-1);
> - if (as->challenge)
> - free(as->challenge);
> + free(as->challenge);
>   as->challenge = value;
>   return (0);
>  
> @@ -476,8 +471,7 @@ auth_setitem(auth_session_t *as, auth_it
>   return (0);
>   if (value != NULL && (value = strdup(value)) == NULL)
>   return (-1);
> - if (as->class)
> - free(as->class);
> + free(as->class);
>   as->class = value;
>   return (0);
>  
> @@ -486,8 +480,7 @@ auth_setitem(auth_session_t *as, auth_it
>   return (0);
>   if (value != NULL && (value = strdup(value)) == NULL)
>   return (-1);
> - if (as->name)
> - free(as->name);
> + free(as->name);
>   as->name = value;
>   return (0);
>  
> @@ -509,8 +502,7 @@ auth_setitem(auth_session_t *as, auth_it
>   if (value == NULL || strchr(value, '/') != NULL ||
>   (value = strdup(value)) == NULL)
>   return (-1);
> - if (as->style)
> - free(as->style);
> + free(as->style);
>   as->style = value;
>   return (0);
>  
> Index: authenticate.c
> ===
> RCS file: /cvs/src/lib/libc/gen/authenticate.c,v
> retrieving revision 1.23
> diff -u -p -r1.23 authenticate.c
> --- authenticate.c12 Sep 2015 15:20:14 -  1.23
> +++ authenticate.c14 Sep 2015 13:15:28 -
> @@ -259,8 +259,7 @@ auth_approval(auth_session_t *as, login_
>   login_close(lc);
>   syslog(LOG_ERR, "%m");
>   warn(NULL);
> - if (approve)
> - free(approve);
> + free(approve);
>   return (0);
>   }
>  
> @@ -294,8 +293,7 @@ auth_approval(auth_session_t *as, login_
>   lc->lc_class, type, (char *)NULL);
>  
>  out:
> - if (approve)
> - free(approve);
> + free(approve);
>   if (close_lc_on_exit)
>   login_close(lc);
>  
> Index: fts.c
> ===
> RCS file: /cvs/src/lib/libc/gen/fts.c,v
> retrieving revision 1.51
> diff -u -p -r1.51 fts.c
> --- fts.c 12 Sep 2015 13:32:24 -  1.51
> +++ fts.c 14 Sep 2015 13:15:28 -
> @@ -228,8 +228,7 @@ fts_close(FTS *sp)
>   /* Free up child linked list, sort array, path buffer, stream ptr.*/
>   if (sp->fts_child)
>   fts_lfree(sp->fts_child);
> - if (sp->fts_array)
> - free(sp->fts_array);
> + free(sp->fts_array);
>   free(sp->fts_path);
>   free(sp);
>  
> @@ -668,8 +667,7 @@ fts_build(FTS *sp, int type)
>* structures already allocated.
>*/
>  mem1:saved_errno = errno;
> - if (p)
> - free(p);
> + free(p);
>   fts_lfree(head);
>   (void)closedir(dirp);
>   cur->fts_info = FTS_ERR;
> @@ -889,8 +887,7 @@ fts_sort(FTS *sp, FTSENT *head, int nite
>   sp->fts_nitems = nitems + 40;
>   

ksh remove Tflag

2015-09-14 Thread Nicholas Marriott
Hi

As far as I can tell there haven't been any new bits added for almost 20
years, so I expect we can do without the Tflag typedef.

ok?



Index: proto.h
===
RCS file: /cvs/src/bin/ksh/proto.h,v
retrieving revision 1.35
diff -u -p -r1.35 proto.h
--- proto.h 4 Sep 2013 15:49:19 -   1.35
+++ proto.h 14 Sep 2015 15:51:02 -
@@ -252,7 +252,7 @@ int setstr(struct tbl *, const char *, i
 struct tbl *setint_v(struct tbl *, struct tbl *, bool);
 void   setint(struct tbl *, long);
 intgetint(struct tbl *, long *, bool);
-struct tbl *   typeset(const char *, Tflag, Tflag, int, int);
+struct tbl *typeset(const char *, int, int, int, int);
 void   unset(struct tbl *, int);
 char  * skip_varname(const char *, int);
 char   *skip_wdvarname(const char *, int);
Index: sh.h
===
RCS file: /cvs/src/bin/ksh/sh.h,v
retrieving revision 1.36
diff -u -p -r1.36 sh.h
--- sh.h14 Sep 2015 09:42:33 -  1.36
+++ sh.h14 Sep 2015 15:51:03 -
@@ -46,9 +46,6 @@
 #definesizeofN(type, n) (sizeof(type) * (n))
 #defineBIT(i)  (1<<(i))/* define bit in flag */
 
-/* Table flag type - needs > 16 and < 32 bits */
-typedef int Tflag;
-
 #defineNUFILE  32  /* Number of user-accessible files */
 #defineFDBASE  10  /* First file usable by Shell */
 
@@ -365,7 +362,7 @@ extern const char ksh_version[];
 
 /* name of called builtin function (used by error functions) */
 EXTERN char*builtin_argv0;
-EXTERN Tflag   builtin_flag;   /* flags of called builtin (SPEC_BI, etc.) */
+EXTERN int builtin_flag;   /* flags of called builtin (SPEC_BI, etc.) */
 
 /* current working directory, and size of memory allocated for same */
 EXTERN char*current_wd;
Index: table.h
===
RCS file: /cvs/src/bin/ksh/table.h,v
retrieving revision 1.8
diff -u -p -r1.8 table.h
--- table.h 19 Feb 2012 07:52:30 -  1.8
+++ table.h 14 Sep 2015 15:51:03 -
@@ -13,7 +13,7 @@ struct table {
 };
 
 struct tbl {   /* table item */
-   Tflag   flag;   /* flags */
+   int flag;   /* flags */
int type;   /* command type (see below), base (if INTEGER),
 * or offset from val.s of value (if EXPORT) */
Area*areap; /* area to allocate from */
Index: c_ksh.c
===
RCS file: /cvs/src/bin/ksh/c_ksh.c,v
retrieving revision 1.37
diff -u -p -r1.37 c_ksh.c
--- c_ksh.c 10 Sep 2015 22:48:58 -  1.37
+++ c_ksh.c 14 Sep 2015 15:51:03 -
@@ -536,14 +536,10 @@ c_typeset(char **wp)
 {
struct block *l;
struct tbl *vp, **p;
-   Tflag fset = 0, fclr = 0;
-   int thing = 0, func = 0, local = 0;
+   int fset = 0, fclr = 0, thing = 0, func = 0, local = 0, pflag = 0;
const char *options = "L#R#UZ#fi#lprtux";   /* see comment below */
char *fieldstr, *basestr;
-   int field, base;
-   int optc;
-   Tflag flag;
-   int pflag = 0;
+   int field, base, optc, flag;
 
switch (**wp) {
case 'e':   /* export */
@@ -834,9 +830,8 @@ int
 c_alias(char **wp)
 {
struct table *t = 
-   int rv = 0, rflag = 0, tflag, Uflag = 0, pflag = 0;
-   int prefix = 0;
-   Tflag xflag = 0;
+   int rv = 0, rflag = 0, tflag, Uflag = 0, pflag = 0, prefix = 0;
+   int xflag = 0;
int optc;
 
builtin_opt.flags |= GF_PLUSOPT;
Index: exec.c
===
RCS file: /cvs/src/bin/ksh/exec.c,v
retrieving revision 1.52
diff -u -p -r1.52 exec.c
--- exec.c  10 Sep 2015 22:48:58 -  1.52
+++ exec.c  14 Sep 2015 15:51:03 -
@@ -527,8 +527,7 @@ comexec(struct op *t, struct tbl *volati
 
case CFUNC: /* function call */
{
-   volatile int old_xflag;
-   volatile Tflag old_inuse;
+   volatile int old_xflag, old_inuse;
const char *volatile old_kshname;
 
if (!(tp->flag & ISSET)) {
@@ -791,7 +790,7 @@ void
 builtin(const char *name, int (*func) (char **))
 {
struct tbl *tp;
-   Tflag flag;
+   int flag;
 
/* see if any flags should be set for this builtin */
for (flag = 0; ; name++) {
Index: var.c
===
RCS file: /cvs/src/bin/ksh/var.c,v
retrieving revision 1.45
diff -u -p -r1.45 var.c
--- var.c   14 Sep 2015 09:42:33 -  1.45
+++ var.c   14 Sep 2015 15:51:03 -
@@ -583,7 +583,7 @@ export(struct tbl *vp, const char *val)
  * LCASEV, UCASEV_AL), and optionally set its value if an assignment.
  */
 struct tbl *
-typeset(const 

Re: ksh remove Tflag

2015-09-14 Thread Ted Unangst
Nicholas Marriott wrote:
> Hi
> 
> As far as I can tell there haven't been any new bits added for almost 20
> years, so I expect we can do without the Tflag typedef.
> 
> ok?

yup



Re: Remove useless NULL casts from mv(1)

2015-09-14 Thread Ted Unangst
Michael McConville wrote:
> Index: mv.c
> ===
> RCS file: /cvs/src/bin/mv/mv.c,v
> retrieving revision 1.40
> diff -u -p -r1.40 mv.c
> --- mv.c  24 Aug 2015 00:10:59 -  1.40
> +++ mv.c  14 Sep 2015 13:38:13 -
> @@ -348,7 +348,7 @@ copy(char *from, char *to)
>   pid_t pid;
>  
>   if ((pid = vfork()) == 0) {
> - execl(_PATH_CP, "cp", "-PRp", "--", from, to, (char *)NULL);
> + execl(_PATH_CP, "cp", "-PRp", "--", from, to, NULL);
>   warn("%s", _PATH_CP);
>   _exit(1);
>   }
> @@ -366,7 +366,7 @@ copy(char *from, char *to)
>   return (1);
>   }
>   if (!(pid = vfork())) {
> - execl(_PATH_RM, "cp", "-rf", "--", from, (char *)NULL);
> + execl(_PATH_RM, "cp", "-rf", "--", from, NULL);
>   warn("%s", _PATH_RM);
>   _exit(1);
>   }
> 

I don't know where we stand on this. It was previously necessary to have the
cast because otherwise NULL would only get passed as an integer, not a pointer
(long). We have fixed NULL in our headers, but the cast may still be the
correct incantation for some systems. I think the best approach is to delete
casts in other more obvious places, but leave these alone for now.



Re: Remove useless NULL casts from mv(1)

2015-09-14 Thread Theo de Raadt
>Michael McConville wrote:
>> Index: mv.c
>> ===
>> RCS file: /cvs/src/bin/mv/mv.c,v
>> retrieving revision 1.40
>> diff -u -p -r1.40 mv.c
>> --- mv.c 24 Aug 2015 00:10:59 -  1.40
>> +++ mv.c 14 Sep 2015 13:38:13 -
>> @@ -348,7 +348,7 @@ copy(char *from, char *to)
>>  pid_t pid;
>>  
>>  if ((pid = vfork()) == 0) {
>> -execl(_PATH_CP, "cp", "-PRp", "--", from, to, (char *)NULL);
>> +execl(_PATH_CP, "cp", "-PRp", "--", from, to, NULL);
>>  warn("%s", _PATH_CP);
>>  _exit(1);
>>  }
>> @@ -366,7 +366,7 @@ copy(char *from, char *to)
>>  return (1);
>>  }
>>  if (!(pid = vfork())) {
>> -execl(_PATH_RM, "cp", "-rf", "--", from, (char *)NULL);
>> +execl(_PATH_RM, "cp", "-rf", "--", from, NULL);
>>  warn("%s", _PATH_RM);
>>  _exit(1);
>>  }
>> 
>
>I don't know where we stand on this. It was previously necessary to have the
>cast because otherwise NULL would only get passed as an integer, not a pointer
>(long). We have fixed NULL in our headers, but the cast may still be the
>correct incantation for some systems. I think the best approach is to delete
>casts in other more obvious places, but leave these alone for now.

Placing more burden on people who make -portable versions of important
software from our base.



Re: Remove useless NULL casts from mv(1)

2015-09-14 Thread Theo de Raadt
>These used to be necessary but now that NULL is (void *)0 they
>aren't.
>
>I'd be inclined to keep them anyway - with the cast it is clearly not a
>bug, without is not so clear. There are also loads of them and it'd be a
>lot of churn to fix them all, for no real advantage. But I don't feel
>strongly about it, others may disagree.

It is neccesary for portable code.

The discussion goes a bit like this:

1. It is not needed here.
2. It is not needed in src/bin
3. It is not needed in usr.bin
4. Oh look, we deleted it out of sshd...

Seems risky.



Re: panic with latest ugen.c and pcsc-lite

2015-09-14 Thread Grant Czajkowski
On Fri, Sep 11, 2015 at 02:41:04AM -0600, David Coppa wrote:
> 
> Hi!
> 
> Repeatedly hit the panic below with latest ugen.c code (v 1.88) and
> pcsc-lite.

Thanks for the report.  Could you please try this patch?

Grant

Index: ugen.c
===
RCS file: /cvs/src/sys/dev/usb/ugen.c,v
retrieving revision 1.88
diff -u -p -d -r1.88 ugen.c
--- ugen.c  7 Sep 2015 19:58:42 -   1.88
+++ ugen.c  15 Sep 2015 04:42:02 -
@@ -557,7 +557,9 @@ ugen_do_read(struct ugen_softc *sc, int 
flags, sce->timeout, NULL);
err = usbd_transfer(xfer);
if (err) {
-   usbd_clear_endpoint_stall(sce->pipeh);
+   if (err == USBD_STALLED)
+   usbd_clear_endpoint_stall(sce->pipeh);
+
if (err == USBD_INTERRUPTED)
error = EINTR;
else if (err == USBD_TIMEOUT)
@@ -691,7 +693,9 @@ ugen_do_write(struct ugen_softc *sc, int
flags, sce->timeout, NULL);
err = usbd_transfer(xfer);
if (err) {
-   usbd_clear_endpoint_stall(sce->pipeh);
+   if (err == USBD_STALLED)
+   usbd_clear_endpoint_stall(sce->pipeh);
+
if (err == USBD_INTERRUPTED)
error = EINTR;
else if (err == USBD_TIMEOUT)



Re: Remove unused vars from if_urtwn.c

2015-09-14 Thread Stefan Sperling
On Sun, Sep 13, 2015 at 05:31:10PM -0400, Michael McConville wrote:
> Index: if_urtwn.c
> ===
> RCS file: /cvs/src/sys/dev/usb/if_urtwn.c,v
> retrieving revision 1.51
> diff -u -p -r1.51 if_urtwn.c
> --- if_urtwn.c10 Sep 2015 11:53:05 -  1.51
> +++ if_urtwn.c13 Sep 2015 21:30:32 -
> @@ -2713,7 +2713,7 @@ urtwn_r88e_dma_init(struct urtwn_softc *
>  {
>   usb_interface_descriptor_t  *id;
>   uint32_t reg;
> - int nrempages, nqpages, nqueues = 1;
> + int nqueues = 1;
>   int error;
>  
>   /* Initialize LLT table. */
> @@ -2724,11 +2724,6 @@ urtwn_r88e_dma_init(struct urtwn_softc *
>   /* Get Tx queues to USB endpoints mapping. */
>   id = usbd_get_interface_descriptor(sc->sc_iface);
>   nqueues = id->bNumEndpoints - 1;
> -
> - /* Get the number of pages for each queue. */
> - nqpages = (R92C_TX_PAGE_COUNT - R92C_PUBQ_NPAGES) / nqueues;
> - /* The remaining pages are assigned to the high priority queue. */
> - nrempages = (R92C_TX_PAGE_COUNT - R92C_PUBQ_NPAGES) % nqueues;
>  
>   /* Set number of pages for normal priority queue. */
>   urtwn_write_2(sc, R92C_RQPN_NPQ, 0x000d);

ok



bye bye ksh/ksh_limval.h

2015-09-14 Thread Nicholas Marriott
ok?


Index: PROJECTS
===
RCS file: /cvs/src/bin/ksh/PROJECTS,v
retrieving revision 1.7
diff -u -p -r1.7 PROJECTS
--- PROJECTS28 Nov 2013 10:33:37 -  1.7
+++ PROJECTS14 Sep 2015 07:53:30 -
@@ -109,6 +109,3 @@ Things to be done in pdksh (see also the
 
* teach shf_vfprintf() about long long's (%lld); also make %p use
  long longs if appropriate.
-
-   * decide wether to keep currently disabled FP stuff in shf.c; if
- not, eliminate ksh_limval.h (moving BITS to var.c).
Index: ksh_limval.h
===
RCS file: ksh_limval.h
diff -N ksh_limval.h
--- ksh_limval.h13 Sep 2015 19:43:42 -  1.3
+++ /dev/null   1 Jan 1970 00:00:00 -
@@ -1,9 +0,0 @@
-/* $OpenBSD: ksh_limval.h,v 1.3 2015/09/13 19:43:42 tedu Exp $ */
-
-/* Wrapper around the values.h/limits.h includes/ifdefs */
-
-/* limits.h is included in sh.h */
-
-#ifndef BITS
-# define BITS(t)   (CHAR_BIT * sizeof(t))
-#endif
Index: sh.h
===
RCS file: /cvs/src/bin/ksh/sh.h,v
retrieving revision 1.35
diff -u -p -r1.35 sh.h
--- sh.h10 Sep 2015 22:48:58 -  1.35
+++ sh.h14 Sep 2015 07:53:30 -
@@ -52,6 +52,8 @@ typedef int Tflag;
 #defineNUFILE  32  /* Number of user-accessible files */
 #defineFDBASE  10  /* First file usable by Shell */
 
+#define BITS(t)(CHAR_BIT * sizeof(t))
+
 /* Make MAGIC a char that might be printed to make bugs more obvious, but
  * not a char that is used often.  Also, can't use the high bit as it causes
  * portability problems (calling strchr(x, 0x80|'x') is error prone).
Index: shf.c
===
RCS file: /cvs/src/bin/ksh/shf.c,v
retrieving revision 1.17
diff -u -p -r1.17 shf.c
--- shf.c   13 Sep 2015 19:43:42 -  1.17
+++ shf.c   14 Sep 2015 07:53:30 -
@@ -6,7 +6,6 @@
 
 #include "sh.h"
 #include 
-#include "ksh_limval.h"
 
 
 /* flags to shf_emptybuf() */
Index: var.c
===
RCS file: /cvs/src/bin/ksh/var.c,v
retrieving revision 1.44
diff -u -p -r1.44 var.c
--- var.c   10 Sep 2015 11:37:42 -  1.44
+++ var.c   14 Sep 2015 07:53:30 -
@@ -2,7 +2,6 @@
 
 #include "sh.h"
 #include 
-#include "ksh_limval.h"
 #include 
 #include 
 



Re: Floating point #define in ksh

2015-09-14 Thread Nicholas Marriott
Here is a diff, also make %p better

Index: shf.c
===
RCS file: /cvs/src/bin/ksh/shf.c,v
retrieving revision 1.17
diff -u -p -r1.17 shf.c
--- shf.c   13 Sep 2015 19:43:42 -  1.17
+++ shf.c   14 Sep 2015 07:47:02 -
@@ -706,23 +706,7 @@ shf_smprintf(const char *fmt, ...)
 }
 
 #define BUF_SIZE   128
-
-/*
- * What kinda of machine we on?  Hopefully the C compiler will optimize
- *  this out...
- *
- * For shorts, we want sign extend for %d but not for %[oxu] - on 16 bit
- *  machines it don't matter.  Assumes C compiler has converted shorts to
- *  ints before pushing them.
- */
-#define POP_INT(f, s, a) \
-   (((f) & FL_LLONG) ? va_arg((a), unsigned long long) :   \
-   ((f) & FL_LONG) ? va_arg((a), unsigned long) :  \
-   (sizeof(int) < sizeof(long) ? ((s) ?\
-   (long) va_arg((a), int) : va_arg((a), unsigned)) :  \
-   va_arg((a), unsigned)))
-
-#define ABIGNUM32000   /* big numer that will fit in a short */
+#define ABIGNUM32000   /* big number that will fit in a short 
*/
 #define LOG2_103.321928094887362347870319429   /* log base 2 
of 10 */
 
 #defineFL_HASH 0x001   /* `#' seen */
@@ -848,9 +832,8 @@ shf_vfprintf(struct shf *shf, const char
 
switch (c) {
case 'p': /* pointer */
-   flags &= ~(FL_LLONG | FL_LONG | FL_SHORT);
-   if (sizeof(char *) > sizeof(int))
-   flags |= FL_LONG; /* hope it fits.. */
+   flags &= ~(FL_LLONG | FL_SHORT);
+   flags |= FL_LONG;
/* aaahhh... */
case 'd':
case 'i':
@@ -859,7 +842,19 @@ shf_vfprintf(struct shf *shf, const char
case 'x':
flags |= FL_NUMBER;
s = [sizeof(numbuf)];
-   llnum = POP_INT(flags, c == 'd', args);
+   if (flags & FL_LLONG)
+   llnum = va_arg(args, unsigned long long);
+   else if (flags & FL_LONG) {
+   if (c == 'd' || c == 'i')
+   llnum = va_arg(args, long);
+   else
+   llnum = va_arg(args, unsigned long);
+   } else {
+   if (c == 'd' || c == 'i')
+   llnum = va_arg(args, int);
+   else
+   llnum = va_arg(args, unsigned int);
+   }
switch (c) {
case 'd':
case 'i':


On Sun, Sep 13, 2015 at 10:27:37AM +0100, Nicholas Marriott wrote:
> On Sat, Sep 12, 2015 at 08:45:43PM -0400, Michael McConville wrote:
> > Nicholas Marriott wrote:
> > > Works for me. ok anyone?
> > > 
> > > I think ksh_limval.h can go entirely after this, per the note in
> > > PROJECTS.
> > 
> > I also just found this gem. It only has one use, so it can probably be
> > replaced. Am I interpreting it correctly?
> 
> No this is not right. long can still be 32 or 64 bits on different
> platforms (such as i386 (ILP32) and amd64 (I32LP64)).
> 
> You need to keep the flags checks because long long or a 64-bit long
> take up more space on the stack than an int or a 32-bit long - if you
> ask va_arg for a long long when there is only an int or a 32-bit long,
> it will use the wrong size.
> 
> The second check is kind of confusing, because not only was the comment
> clearly not updated when changing from short/int to int/long but the
> macro itself was not updated when llnum was changed from unsigned long
> to unsigned long long.
> 
> When llnum was unsigned long, the intent was that -ve numbers were sign
> extended for %d but not %[oux]. So where int is 32 bits and long 64
> bits, -1 to %d gave (unsigned long)(int)-1 but -1 to %x gave (unsigned
> long)(unsigned int)-1. Where int and long are both 32 bits, it didn't
> matter.
> 
> But llnum is now always 64 bits (unsigned long long) even when long is
> 32 bits, so the check is now wrong and will not sign extend when it
> should (for example on i386).
> 
> So the (sizeof (int) < sizeof (long) condition can be removed - now that
> llnum is long long, it should actually be sizeof (int) < sizeof (long
> long), which is always true on OpenBSD. But we need to keep the true
> branch not the false, so that the sign extension happens correctly for
> [di] vs [oux].
> 
> There is another problem: %d is checked, but not %i - I think they
> should be the same.
> 
> Also if it is concerned about sign extension for %d vs %x, then it
> should be for %ld vs %lx too, because %ld and %d are the same on ILP32

[patch] if-free cleanup in sys/arch

2015-09-14 Thread Sebastien Marie
Hi,

Here a first sets of "if(x) free(x)" cleanup in sys/arch/

This patch contains only trivial if(x) removal. The size argument in
free is keep untouched (because it is already setted, or because it
makes sens to keep it to 0).

Comments ? OK ?
-- 
Sebastien Marie

Index: b/sys/arch/amd64/stand/libsa/biosdev.c
===
--- a/sys/arch/amd64/stand/libsa/biosdev.c  2015-09-12 10:35:37.034830676 
+0200
+++ b/sys/arch/amd64/stand/libsa/biosdev.c  2015-09-12 10:36:00.744851935 
+0200
@@ -327,8 +327,7 @@ biosd_io(int rw, bios_diskinfo_t *bd, u_
 
if (bb != buf && rw == F_READ)
bcopy(bb, buf, bbsize);
-   if (bb1 != NULL)
-   free(bb1, bbsize);
+   free(bb1, bbsize);
 
 #ifdef BIOS_DEBUG
if (debug) {
Index: b/sys/arch/i386/stand/libsa/biosdev.c
===
--- a/sys/arch/i386/stand/libsa/biosdev.c   2015-09-04 06:33:40.953443448 
+0200
+++ b/sys/arch/i386/stand/libsa/biosdev.c   2015-09-12 10:33:57.324705255 
+0200
@@ -328,8 +328,7 @@ biosd_io(int rw, bios_diskinfo_t *bd, u_
 
if (bb != buf && rw == F_READ)
bcopy(bb, buf, bbsize);
-   if (bb1 != NULL)
-   free(bb1, bbsize);
+   free(bb1, bbsize);
 
 #ifdef BIOS_DEBUG
if (debug) {
Index: b/sys/arch/loongson/dev/bonito.c
===
--- a/sys/arch/loongson/dev/bonito.c2015-09-10 06:14:11.727624362 +0200
+++ b/sys/arch/loongson/dev/bonito.c2015-09-12 10:39:25.345205004 +0200
@@ -1203,8 +1203,7 @@ bonito_get_resource_extent(pci_chipset_t
 #endif
 
 out:
-   if (exname != NULL)
-   free(exname, M_DEVBUF, exnamesz);
+   free(exname, M_DEVBUF, exnamesz);
 
return ex;
 }
Index: b/sys/arch/macppc/macppc/openprom.c
===
--- a/sys/arch/macppc/macppc/openprom.c 2014-07-19 17:34:21.0 +0200
+++ b/sys/arch/macppc/macppc/openprom.c 2015-09-12 10:41:28.745289870 +0200
@@ -255,10 +255,8 @@ openpromioctl(dev_t dev, u_long cmd, cad
return (ENOTTY);
}
 
-   if (name)
-   free(name, M_TEMP, 0);
-   if (value)
-   free(value, M_TEMP, 0);
+   free(name, M_TEMP, 0);
+   free(value, M_TEMP, 0);
 
return (error);
 }
Index: b/sys/arch/sgi/sgi/l1.c
===
--- a/sys/arch/sgi/sgi/l1.c 2015-09-10 06:14:14.617676019 +0200
+++ b/sys/arch/sgi/sgi/l1.c 2015-09-12 10:45:56.375547158 +0200
@@ -770,8 +770,7 @@ l1_read_board_ia(int16_t nasid, int type
return 0;
 
 fail:
-   if (ia != NULL)
-   free(ia, M_DEVBUF, ialen);
+   free(ia, M_DEVBUF, ialen);
return rc;
 }
 
@@ -1187,8 +1186,7 @@ l1_get_brick_spd_record(int16_t nasid, i
return 0;
 
 fail:
-   if (spd != NULL)
-   free(spd, M_DEVBUF, spdlen);
+   free(spd, M_DEVBUF, spdlen);
return rc;
 }
 
Index: b/sys/arch/sparc/sparc/clock.c
===
--- a/sys/arch/sparc/sparc/clock.c  2015-09-12 10:50:55.095612714 +0200
+++ b/sys/arch/sparc/sparc/clock.c  2015-09-12 10:51:49.125685092 +0200
@@ -1125,8 +1125,7 @@ eeprom_uio(uio)
error = eeprom_update(buf, (size_t)off, cnt);
 
  out:
-   if (buf)
-   free(buf, M_DEVBUF, 0);
+   free(buf, M_DEVBUF, EEPROM_SIZE);
eeprom_give();
return (error);
 #else /* ! SUN4 */
Index: b/sys/arch/sparc/sparc/openprom.c
===
--- a/sys/arch/sparc/sparc/openprom.c   2015-09-12 10:50:55.005612242 +0200
+++ b/sys/arch/sparc/sparc/openprom.c   2015-09-12 10:51:49.115685124 +0200
@@ -245,10 +245,8 @@ openpromioctl(dev, cmd, data, flags, p)
return (ENOTTY);
}
 
-   if (name)
-   free(name, M_TEMP, 0);
-   if (value)
-   free(value, M_TEMP, 0);
+   free(name, M_TEMP, 0);
+   free(value, M_TEMP, 0);
 
return (error);
 }
Index: b/sys/arch/sparc/stand/common/dvma.c
===
--- a/sys/arch/sparc/stand/common/dvma.c2014-07-19 17:34:30.0 
+0200
+++ b/sys/arch/sparc/stand/common/dvma.c2015-09-12 11:03:34.546042656 
+0200
@@ -142,6 +142,5 @@ dvma_free(char *dvma, int len)
char *mem;
 
mem = dvma_mapout(dvma, len);
-   if (mem)
-   free(mem, len);
+   free(mem, len);
 }
Index: b/sys/arch/sparc64/dev/fhc.c
===
--- a/sys/arch/sparc64/dev/fhc.c2015-09-12 11:08:22.576274652 +0200
+++ b/sys/arch/sparc64/dev/fhc.c2015-09-12 11:08:32.186276302 +0200
@@ -117,14 +117,10 @@ fhc_attach(struct fhc_softc *sc)