tmux leaving zombie behind

2013-07-04 Thread patrick keshishian
Hi,

Back in 2010 (Apri-ish according to google) I reported that tmux left
a zombie process behind on initial launch. If user detached and
reattached, the zombie process cleared. I even provided a patch, which
I believe was accepted, but then replaced with a slightly different
version. However, since then I have noticed this behavior (or a
similar issue) return. On initial launch of tmux, there is a zombie
process left around until a detach. I have not investigated this
further, so it may or may not be a related issue.

Hope this helps,
--patrick

$ ps auxw | grep tmux
me  12001  0.0  0.1  1136  1920 ??  Ss 9:00PM0:00.04 tmux:
server (/tmp/tmux-1000/default) (tmux)
me  32546  0.0  0.0 0 0 p1  Z+-  0:00.00 (tmux)
me  22096  0.0  0.0   976  1616 p1  S+ 9:00PM0:00.02 tmux:
client (/tmp/tmux-1000/default) (tmux)
me  14412  0.0  0.0   420   952 p3  S+ 9:01PM0:00.00 grep tmux



smtpd: if?

2013-07-04 Thread Maxime Villard
Hi,

- - - src/usr.sbin/smtpd/smtpd.c l.1326

if (! bsnprintf(key, sizeof key,
profiling.imsg.%s.%s.%s,
proc_name(smtpd_process),
proc_name(p-proc),
imsg_to_str(imsg-hdr.type)));-- ';'
stat_set(key, stat_timespec(dt));


What's the goal of the 'if' right there?



Re: smtpd: if?

2013-07-04 Thread Gilles Chehade
On Thu, Jul 04, 2013 at 08:48:16AM +0200, Maxime Villard wrote:
 Hi,
 
 - - - src/usr.sbin/smtpd/smtpd.c l.1326
 
   if (! bsnprintf(key, sizeof key,
   profiling.imsg.%s.%s.%s,
   proc_name(smtpd_process),
   proc_name(p-proc),
   imsg_to_str(imsg-hdr.type)));-- ';'
   stat_set(key, stat_timespec(dt));
 
 
 What's the goal of the 'if' right there?
 

I just commited a fix, it was reported by someone else in private too ;)

And for the record:

No need to worry much about it: it is a bug in profiling code which gets
bypassed unless you start smtpd in profiling mode. If you did, you would
still not hit it because the values we currently have do not exceed that
key buffer. And if they did, it wouldn't really be an issue anyway, this
is a key intended to carry informative value to the developer, it is not
clear if it's preferable to store a truncated key with its value, or not
to store any value at all :-/

-- 
Gilles Chehade

https://www.poolp.org  @poolpOrg



old unused leftover

2013-07-04 Thread Artturi Alm

Hi,

old has been old and unused for some time already.


- Artturi


Index: kern_timeout.c
===
RCS file: /cvs/src/sys/kern/kern_timeout.c,v
retrieving revision 1.35
diff -u -p -u -p -r1.35 kern_timeout.c
--- kern_timeout.c2 Jun 2012 00:11:16 -1.35
+++ kern_timeout.c3 Jul 2013 22:17:17 -
@@ -350,7 +350,7 @@ timeout_adjust_ticks(int adj)
 {
 struct timeout *to;
 struct circq *p;
-int new_ticks, b, old;
+int new_ticks, b;

 /* adjusting the monotonic clock backwards would be a Bad Thing */
 if (adj = 0)
@@ -363,8 +363,6 @@ timeout_adjust_ticks(int adj)
 while (p != timeout_wheel[b]) {
 to = (struct timeout *)p; /* XXX */
 p = CIRCQ_FIRST(p);
-
-old = to-to_time;

 /* when moving a timeout forward need to reinsert it */
 if (to-to_time - ticks  adj)



Re: libc malloc poison

2013-07-04 Thread Theo de Raadt
 On Wed, Jul 03, 2013 at 17:21, Theo de Raadt wrote:
  +   int pval = 0xd0d0caca;
  
  Can you explain the choice of this?
 
 I thought it sounded clever.

Ok, because there's more to the picture.

Inside the kernel, we tend to use 0xdeadbeef, or the DEADBEEF0/DEADBEEF1 values.

Reason for the latter is that we can try to put this value into memory
which the kernel does not manage.  Basically on half of our kernel
architectures if that is loaded into a pointer, it will hit unmanaged
kernel memory, exposing the bug.

Furthermore, the idea is that the 0xdeadbeef value should have a high
mix of set bits versus clear bits, for when it lands in a flag
variable.  A lot of bits are set; some paper I read years ago
discussed that on average flag bits set tends to traverse.

Furthermore, each of the sub-fields tend to be odd, which in other
use after cases regarding offsets/indexes can lead to more unaligned
access, once again triggering and exposing a bug.

Those are all theoretical ideas to try to expose the bugs as early
as possible.

I think the use of 0xd0d0caca in userland might not be the most
suitable choice, especially in a MI fashion.

  There are arguments to make this MI; other arguments to make it MD;
  and other arguments to introduce a bit of randomness.
  
  I'd like to know which arguments you have
 
 Since libc doesn't do free list integrity checking, I'm currently
 leaning towards a random value. (even with random, we could still
 check that all words of a free chunk are the same.)
 
 Somebody also noticed that we don't have separate values for allocated
 and freed memory. I suppose this makes debugging harder since you
 can't obviously identify freed memory? I lean towards prioritizing
 finding more bugs, which implies we need more variability, since any
 one value may allow a program to work where a different value would
 not.

I agree with this last sentence.

I suspect the best approach would be a hybrid value.  The upper half
of the address should try to land in an unmapped zone, or into the zero
page, or into some address space hole, ir into super high memory above
the stack which is gauranteed unmapped.

The 64-bit machines require a bit more consideration as well; we want
two 32-bit values to combine into a nice trashy address.

Should we use a kernel sysctl to recommend the high word?, and a mask
against random?  Of course, that could also be done using MD includes.

 



Re: libc malloc poison

2013-07-04 Thread Mark Kettenis
 From: Theo de Raadt dera...@cvs.openbsd.org
 Date: Thu, 04 Jul 2013 09:04:54 -0600
 
 I suspect the best approach would be a hybrid value.  The upper half
 of the address should try to land in an unmapped zone, or into the zero
 page, or into some address space hole, ir into super high memory above
 the stack which is gauranteed unmapped.

Don't forget strict alignment architectures, where it is beneficial
to have the lowest bit set to trigger alignment traps.



Re: libc malloc poison

2013-07-04 Thread Theo de Raadt
  From: Theo de Raadt dera...@cvs.openbsd.org
  Date: Thu, 04 Jul 2013 09:04:54 -0600
  
  I suspect the best approach would be a hybrid value.  The upper half
  of the address should try to land in an unmapped zone, or into the zero
  page, or into some address space hole, ir into super high memory above
  the stack which is gauranteed unmapped.
 
 Don't forget strict alignment architectures, where it is beneficial
 to have the lowest bit set to trigger alignment traps.

That's why I vaguely mentioned the idea of a sysctl or MD defines, which
would declare a fixed component, plus a mask on top of random.

That fixed component need not just be high bits, it can also cover the
lowest bit (or two) for instance.



Removing -Wno-format from kernel makefiles, 06/16

2013-07-04 Thread Stefan Fritsch
 Use %z* for size_t

while there, fix a few %d into %u
---
 sys/arch/i386/pci/pcibios.c |2 +-
 sys/dev/ic/bwi.c|4 ++--
 sys/dev/ic/pgt.c|6 +++---
 sys/dev/ic/ti.c |4 ++--
 sys/dev/pci/amdiic.c|4 ++--
 sys/dev/pci/amdpm.c |2 +-
 sys/dev/pci/if_iwi.c|4 ++--
 sys/dev/pci/if_iwn.c|   10 +-
 sys/dev/pci/if_lge.c|4 ++--
 sys/dev/pci/if_nge.c|4 ++--
 sys/dev/pci/if_tl.c |2 +-
 sys/dev/pci/if_wb.c |2 +-
 sys/dev/pci/if_wpi.c|4 ++--
 sys/dev/pci/piixpm.c|2 +-
 sys/dev/pci/yds.c   |2 +-
 sys/dev/usb/if_urndis.c |   12 ++--
 sys/netinet/ip_output.c |2 +-
 sys/netinet6/ip6_forward.c  |2 +-
 sys/netinet6/ip6_output.c   |2 +-
 sys/nfs/nfs_socket.c|4 ++--
 20 files changed, 39 insertions(+), 39 deletions(-)

diff --git sys/arch/i386/pci/pcibios.c sys/arch/i386/pci/pcibios.c
index 1b34391..372ac51 100644
--- sys/arch/i386/pci/pcibios.c
+++ sys/arch/i386/pci/pcibios.c
@@ -260,7 +260,7 @@ pcibios_pir_init(struct pcibios_softc *sc)
cksum += p[i];
 
printf(%s: PCI IRQ Routing Table rev %d.%d @ 0x%lx/%d 
-   (%d entries)\n, sc-sc_dev.dv_xname,
+   (%zd entries)\n, sc-sc_dev.dv_xname,
pirh-version  8, pirh-version  0xff, pa,
pirh-tablesize, (pirh-tablesize - sizeof(*pirh)) / 16);
 
diff --git sys/dev/ic/bwi.c sys/dev/ic/bwi.c
index 6295172..934a6ac 100644
--- sys/dev/ic/bwi.c
+++ sys/dev/ic/bwi.c
@@ -1673,7 +1673,7 @@ bwi_fwimage_is_valid(struct bwi_softc *sc, uint8_t *fw, 
size_t fw_len,
const struct bwi_fwhdr *hdr;
 
if (fw_len  sizeof(*hdr)) {
-   printf(%s: invalid firmware (%s): invalid size %u\n,
+   printf(%s: invalid firmware (%s): invalid size %zu\n,
sc-sc_dev.dv_xname, fw_name, fw_len);
return (1);
}
@@ -1686,7 +1686,7 @@ bwi_fwimage_is_valid(struct bwi_softc *sc, uint8_t *fw, 
size_t fw_len,
 */
if (betoh32(hdr-fw_size) != fw_len - sizeof(*hdr)) {
printf(%s: invalid firmware (%s): size mismatch, 
-   fw %u, real %u\n,
+   fw %u, real %zu\n,
sc-sc_dev.dv_xname,
fw_name,
betoh32(hdr-fw_size),
diff --git sys/dev/ic/pgt.c sys/dev/ic/pgt.c
index 692b72c..6f6a5d6 100644
--- sys/dev/ic/pgt.c
+++ sys/dev/ic/pgt.c
@@ -3193,7 +3193,7 @@ pgt_dma_alloc_queue(struct pgt_softc *sc, enum pgt_queue 
pq)
error = bus_dmamem_alloc(sc-sc_dmat, PGT_FRAG_SIZE, PAGE_SIZE,
0, pd-pd_dmas, 1, nsegs, BUS_DMA_WAITOK);
if (error != 0) {
-   printf(%s: error alloc frag %u on queue %u\n,
+   printf(%s: error alloc frag %zu on queue %u\n,
sc-sc_dev.dv_xname, i, pq);
free(pd, M_DEVBUF);
break;
@@ -3202,7 +3202,7 @@ pgt_dma_alloc_queue(struct pgt_softc *sc, enum pgt_queue 
pq)
error = bus_dmamem_map(sc-sc_dmat, pd-pd_dmas, nsegs,
PGT_FRAG_SIZE, (caddr_t *)pd-pd_mem, BUS_DMA_WAITOK);
if (error != 0) {
-   printf(%s: error map frag %u on queue %u\n,
+   printf(%s: error map frag %zu on queue %u\n,
sc-sc_dev.dv_xname, i, pq);
free(pd, M_DEVBUF);
break;
@@ -3212,7 +3212,7 @@ pgt_dma_alloc_queue(struct pgt_softc *sc, enum pgt_queue 
pq)
error = bus_dmamap_load(sc-sc_dmat, pd-pd_dmam,
pd-pd_mem, PGT_FRAG_SIZE, NULL, BUS_DMA_NOWAIT);
if (error != 0) {
-   printf(%s: error load frag %u on queue %u\n,
+   printf(%s: error load frag %zu on queue %u\n,
sc-sc_dev.dv_xname, i, pq);
bus_dmamem_free(sc-sc_dmat, pd-pd_dmas,
nsegs);
diff --git sys/dev/ic/ti.c sys/dev/ic/ti.c
index 85e3a2a..902591f 100644
--- sys/dev/ic/ti.c
+++ sys/dev/ic/ti.c
@@ -612,7 +612,7 @@ ti_alloc_jumbo_mem(struct ti_softc *sc)
state = 1;
if (bus_dmamem_map(sc-sc_dmatag, seg, rseg, TI_JMEM, kva,
BUS_DMA_NOWAIT)) {
-   printf(%s: can't map dma buffers (%d bytes)\n,
+   printf(%s: can't map dma buffers (%zu bytes)\n,
sc-sc_dv.dv_xname, TI_JMEM);
error = ENOBUFS;
goto out;
@@ -1600,7 +1600,7 @@ ti_attach(struct ti_softc *sc)
}
if (bus_dmamem_map(sc-sc_dmatag, seg, rseg,
sizeof(struct ti_ring_data), kva, 

Re: Removing -Wno-format from kernel makefiles, 3/16

2013-07-04 Thread Stefan Fritsch
On Wed, 3 Jul 2013, Mark Kettenis wrote:
  diff --git sys/arch/i386/i386/db_interface.c 
  sys/arch/i386/i386/db_interface.c
  index 85c1ff5..c75fd89 100644
  --- sys/arch/i386/i386/db_interface.c
  +++ sys/arch/i386/i386/db_interface.c
  @@ -197,11 +197,11 @@ db_sysregs_cmd(db_expr_t addr, int have_addr, 
  db_expr_t count, char *modif)
  uint16_t ldtr, tr;
   
  __asm__ __volatile__(sidt %0 : =m (idtr));
  -   db_printf(idtr:   0x%08x/%04x\n,
  +   db_printf(idtr:   0x%08x/%04llx\n,
  (unsigned int)(idtr  16), idtr  0x);
   
  __asm__ __volatile__(sgdt %0 : =m (gdtr));
  -   db_printf(gdtr:   0x%08x/%04x\n,
  +   db_printf(gdtr:   0x%08x/%04llx\n,
  (unsigned int)(gdtr  16), gdtr  0x);
 
 This is a tad bit inconsistent.  I'd either use %llx for both values
 and get rid of the cast, or use %x and use a cast in both cases.

Like this?

--- sys/arch/i386/i386/db_interface.c
+++ sys/arch/i386/i386/db_interface.c
@@ -197,12 +197,10 @@ db_sysregs_cmd(db_expr_t addr, int have_addr, db_expr_t 
count, char *modif)
uint16_t ldtr, tr;
 
__asm__ __volatile__(sidt %0 : =m (idtr));
-   db_printf(idtr:   0x%08x/%04x\n,
-   (unsigned int)(idtr  16), idtr  0x);
+   db_printf(idtr:   0x%08llx/%04llx\n, idtr  16, idtr  0x);
 
__asm__ __volatile__(sgdt %0 : =m (gdtr));
-   db_printf(gdtr:   0x%08x/%04x\n,
-   (unsigned int)(gdtr  16), gdtr  0x);
+   db_printf(gdtr:   0x%08llx/%04llx\n, gdtr  16, gdtr  0x);
 
__asm__ __volatile__(sldt %0 : =g (ldtr));
db_printf(ldtr:   0x%04x\n, ldtr);



Re: Removing -Wno-format from kernel makefiles, 4/16

2013-07-04 Thread Stefan Fritsch
On Wed, 3 Jul 2013, Mark Kettenis wrote:
  diff --git sys/arch/i386/i386/esm.c sys/arch/i386/i386/esm.c
  index c90b2c4..3dff69e 100644
  --- sys/arch/i386/i386/esm.c
  +++ sys/arch/i386/i386/esm.c
  @@ -880,7 +880,7 @@ esm_make_sensors(struct esm_softc *sc, struct 
  esm_devmap *devmap,
  }
   
  for (j = 0; j  nsensors; j++) {
  -   snprintf(s[j].desc, sizeof(s[j].desc), %s %d,
  +   snprintf(s[j].desc, sizeof(s[j].desc), %s %ld,
  sensor_map[i].name, sensor_map[i].arg + j);
  }
  break;
 
 Looking at this one, it makes more sense to make the arg member of
 struct esm_sensor_map an int.  That will result in some space
 savings if we'd ever bring this driver to amd64.


--- sys/arch/i386/i386/esm.c
+++ sys/arch/i386/i386/esm.c
@@ -87,7 +87,7 @@ enum sensor_type esm_typemap[] = {
 
 struct esm_sensor_map {
enum esm_sensor_typetype;
-   longarg;
+   int arg;
const char  *name;
 };
 




Removing -Wno-format from kernel makefiles, 07/16

2013-07-04 Thread Stefan Fritsch
 fix: %x instead of %p for int

---
 sys/dev/pci/musycc_obsd.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git sys/dev/pci/musycc_obsd.c sys/dev/pci/musycc_obsd.c
index 25a58d8..0844136 100644
--- sys/dev/pci/musycc_obsd.c
+++ sys/dev/pci/musycc_obsd.c
@@ -215,7 +215,7 @@ musycc_ebus_attach(struct device *parent, struct 
musycc_softc *esc,
 #endif
 
if (ebus_attach_device(rom, sc, 0, 0x400) != 0) {
-   printf(: failed to map rom @ %05p\n, 0);
+   printf(: failed to map rom @ %05x\n, 0);
goto failed;
}
 
-- 
1.7.6



Removing -Wno-format from kernel makefiles, 10/16

2013-07-04 Thread Stefan Fritsch
 fix: pointer to long

---
 sys/arch/i386/pci/pci_addr_fixup.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git sys/arch/i386/pci/pci_addr_fixup.c sys/arch/i386/pci/pci_addr_fixup.c
index 59880eb..40f7918 100644
--- sys/arch/i386/pci/pci_addr_fixup.c
+++ sys/arch/i386/pci/pci_addr_fixup.c
@@ -354,7 +354,7 @@ pciaddr_do_resource_reserve_disabled(struct pcibios_softc 
*sc,
(val  PCI_COMMAND_IO_ENABLE) == PCI_COMMAND_IO_ENABLE)
return (0);
 
-   PCIBIOS_PRINTV((disabled %s space at addr 0x%x size 0x%x\n,
+   PCIBIOS_PRINTV((disabled %s space at addr 0x%lx size 0x%x\n,
type == PCI_MAPREG_TYPE_MEM ? mem : io, *addr, size));
 
error = extent_alloc_region(ex, *addr, size, EX_NOWAIT | EX_MALLOCOK);
-- 
1.7.6



Removing -Wno-format from kernel makefiles, 09/16

2013-07-04 Thread Stefan Fritsch
 paddr_t is long

---
 sys/arch/i386/i386/machdep.c   |4 ++--
 sys/arch/i386/pci/pci_addr_fixup.c |2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git sys/arch/i386/i386/machdep.c sys/arch/i386/i386/machdep.c
index 7924386..1a26e7d 100644
--- sys/arch/i386/i386/machdep.c
+++ sys/arch/i386/i386/machdep.c
@@ -2774,7 +2774,7 @@ dumpsys()
 
/* Print out how many MBs we have more to go. */
if (dbtob(blkno - dumplo) % (1024 * 1024)  NBPG)
-   printf(%d ,
+   printf(%ld ,
(ptoa(dumpsize) - maddr) / (1024 * 1024));
 #if 0
printf((%x %lld) , maddr, blkno);
@@ -3224,7 +3224,7 @@ init386(paddr_t first_avail)
 
if (extent_alloc_region(iomem_ex, a, e - a, EX_NOWAIT))
/* XXX What should we do? */
-   printf(\nWARNING: CAN'T ALLOCATE RAM (%x-%x)
+   printf(\nWARNING: CAN'T ALLOCATE RAM (%lx-%lx)
 FROM IOMEM EXTENT MAP!\n, a, e);
 
physmem += atop(e - a);
diff --git sys/arch/i386/pci/pci_addr_fixup.c sys/arch/i386/pci/pci_addr_fixup.c
index e4eb1b4..59880eb 100644
--- sys/arch/i386/pci/pci_addr_fixup.c
+++ sys/arch/i386/pci/pci_addr_fixup.c
@@ -136,7 +136,7 @@ pci_addr_fixup(struct pcibios_softc *sc, pci_chipset_tag_t 
pc, int maxbus)
start = PCIADDR_ISAMEM_RESERVE;
sc-mem_alloc_start = (start + 0x10 + 1)  ~(0x10 - 1);
sc-port_alloc_start = PCIADDR_ISAPORT_RESERVE;
-   PCIBIOS_PRINTV(( Physical memory end: 0x%08x\n PCI memory mapped I/O 
+   PCIBIOS_PRINTV(( Physical memory end: 0x%08lx\n PCI memory mapped I/O 
space start: 0x%08x\n, avail_end, sc-mem_alloc_start));
 
/* 
-- 
1.7.6



Removing -Wno-format from kernel makefiles, 08/16

2013-07-04 Thread Stefan Fritsch
 %hu/%hd for uint16_t, %u/%d/%x for uint32_t

despite the name, ntohl returns uint32_t
also fix some %d into %u
---
 sys/arch/i386/i386/bios.c|4 ++--
 sys/arch/i386/i386/ioapic.c  |2 +-
 sys/arch/i386/pci/pcibios.c  |4 ++--
 sys/kern/vfs_cluster.c   |2 +-
 sys/msdosfs/msdosfs_conv.c   |2 +-
 sys/msdosfs/msdosfs_denode.c |2 +-
 sys/msdosfs/msdosfs_vnops.c  |2 +-
 sys/net/if_spppsubr.c|2 +-
 8 files changed, 10 insertions(+), 10 deletions(-)

diff --git sys/arch/i386/i386/bios.c sys/arch/i386/i386/bios.c
index 6e46e6a..75dab38 100644
--- sys/arch/i386/i386/bios.c
+++ sys/arch/i386/i386/bios.c
@@ -238,7 +238,7 @@ biosattach(struct device *parent, struct device *self, void 
*aux)
 
bios32_entry.segment = GSEL(GCODE_SEL, SEL_KPL);
bios32_entry.offset = 
(u_int32_t)ISA_HOLE_VADDR(h-entry);
-   printf(, BIOS32 rev. %d @ 0x%lx, h-rev, h-entry);
+   printf(, BIOS32 rev. %d @ 0x%x, h-rev, h-entry);
break;
}
}
@@ -285,7 +285,7 @@ biosattach(struct device *parent, struct device *self, void 
*aux)
for (; pa  end; pa+= NBPG, eva+= NBPG)
pmap_kenter_pa(eva, pa, VM_PROT_READ);
 
-   printf(, SMBIOS rev. %d.%d @ 0x%lx (%d entries),
+   printf(, SMBIOS rev. %d.%d @ 0x%x (%hd entries),
sh-majrev, sh-minrev, sh-addr, sh-count);
/*
 * Unbelievably the SMBIOS version number
diff --git sys/arch/i386/i386/ioapic.c sys/arch/i386/i386/ioapic.c
index 0b0c6b8..4197553 100644
--- sys/arch/i386/i386/ioapic.c
+++ sys/arch/i386/i386/ioapic.c
@@ -300,7 +300,7 @@ ioapic_attach(struct device *parent, struct device *self, 
void *aux)
 
ioapic_add(sc);
 
-   printf( pa 0x%lx, aaa-apic_address);
+   printf( pa 0x%x, aaa-apic_address);
 
if (bus_mem_add_mapping(aaa-apic_address, PAGE_SIZE, 0, bh) != 0) {
printf(, map failed\n);
diff --git sys/arch/i386/pci/pcibios.c sys/arch/i386/pci/pcibios.c
index 372ac51..bc8a848 100644
--- sys/arch/i386/pci/pcibios.c
+++ sys/arch/i386/pci/pcibios.c
@@ -151,7 +151,7 @@ pcibiosprobe(struct device *parent, void *match, void *aux)
rv = bios32_service(PCIBIOS_SIGNATURE, pcibios_entry,
pcibios_entry_info);
 
-   PCIBIOS_PRINTV((pcibiosprobe: 0x%lx:0x%lx at 0x%lx[0x%lx]\n,
+   PCIBIOS_PRINTV((pcibiosprobe: 0x%hx:0x%x at 0x%x[0x%x]\n,
pcibios_entry.segment, pcibios_entry.offset,
pcibios_entry_info.bei_base, pcibios_entry_info.bei_size));
 
@@ -172,7 +172,7 @@ pcibiosattach(struct device *parent, struct device *self, 
void *aux)
rev_min, mech1, mech2,
scmech1, scmech2, sc-max_bus);
 
-   printf(: rev %d.%d @ 0x%lx/0x%lx\n,
+   printf(: rev %d.%d @ 0x%x/0x%x\n,
rev_maj, rev_min  4, pcibios_entry_info.bei_base,
pcibios_entry_info.bei_size);
 
diff --git sys/kern/vfs_cluster.c sys/kern/vfs_cluster.c
index f9df40e..118a411 100644
--- sys/kern/vfs_cluster.c
+++ sys/kern/vfs_cluster.c
@@ -175,7 +175,7 @@ cluster_wbuild(struct vnode *vp, struct buf *last_bp, long 
size,
 
 #ifdef DIAGNOSTIC
if (size != vp-v_mount-mnt_stat.f_iosize)
-   panic(cluster_wbuild: size %ld != filesize %ld,
+   panic(cluster_wbuild: size %ld != filesize %u,
size, vp-v_mount-mnt_stat.f_iosize);
 #endif
 redo:
diff --git sys/msdosfs/msdosfs_conv.c sys/msdosfs/msdosfs_conv.c
index 727acd2..538aa1c 100644
--- sys/msdosfs/msdosfs_conv.c
+++ sys/msdosfs/msdosfs_conv.c
@@ -209,7 +209,7 @@ dos2unixtime(u_int dd, u_int dt, u_int dh, struct timespec 
*tsp)
 */
month = (dd  DD_MONTH_MASK)  DD_MONTH_SHIFT;
if (month == 0) {
-   printf(dos2unixtime(): month value out of range 
(%ld)\n,
+   printf(dos2unixtime(): month value out of range 
(%u)\n,
month);
month = 1;
}
diff --git sys/msdosfs/msdosfs_denode.c sys/msdosfs/msdosfs_denode.c
index d384a8b..29f2e57 100644
--- sys/msdosfs/msdosfs_denode.c
+++ sys/msdosfs/msdosfs_denode.c
@@ -387,7 +387,7 @@ detrunc(struct denode *dep, uint32_t length, int flags, 
struct ucred *cred,
 * directory's life.
 */
if ((DETOV(dep)-v_flag  VROOT)  !FAT32(pmp)) {
-   printf(detrunc(): can't truncate root directory, clust %ld, 
offset %ld\n,
+   printf(detrunc(): can't truncate root directory, clust %u, 
offset %u\n,
dep-de_dirclust, dep-de_diroffset);
return (EINVAL);
}
diff --git sys/msdosfs/msdosfs_vnops.c sys/msdosfs/msdosfs_vnops.c
index 79f611c..25d7030 100644
--- sys/msdosfs/msdosfs_vnops.c
+++ sys/msdosfs/msdosfs_vnops.c
@@ 

Re: Removing -Wno-format from kernel makefiles, V2

2013-07-04 Thread Stefan Fritsch
On Wednesday 03 July 2013, Stefan Fritsch wrote:
 I haven't done any signedness fixes so far, only the size fixes.
 But I will look over the patches again and try to fix the
 signedness issues in the touched code lines, too. There are quite
 a few I guess.

For people prefering to review by source files, an updated patch is 
here:

http://www.sfritsch.de/~stf/openbsd-format-string-v3/combined.patch



Re: Removing -Wno-format from kernel makefiles, 07/16

2013-07-04 Thread Franco Fichtner
On Jul 4, 2013, at 6:43 PM, Stefan Fritsch s...@sfritsch.de wrote:

 fix: %x instead of %p for int
 
 ---
 sys/dev/pci/musycc_obsd.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
 
 diff --git sys/dev/pci/musycc_obsd.c sys/dev/pci/musycc_obsd.c
 index 25a58d8..0844136 100644
 --- sys/dev/pci/musycc_obsd.c
 +++ sys/dev/pci/musycc_obsd.c
 @@ -215,7 +215,7 @@ musycc_ebus_attach(struct device *parent, struct 
 musycc_softc *esc,
 #endif
 
   if (ebus_attach_device(rom, sc, 0, 0x400) != 0) {
 - printf(: failed to map rom @ %05p\n, 0);
 + printf(: failed to map rom @ %05x\n, 0);

Makes me wonder what benefit this has other than avoiding to type four
more zeros in the format string manually?  ;)

   goto failed;
   }
 
 -- 
 1.7.6
 




Re: Removing -Wno-format from kernel makefiles, 3/16

2013-07-04 Thread Mark Kettenis
 Date: Thu, 4 Jul 2013 18:41:30 +0200 (CEST)
 From: Stefan Fritsch s...@sfritsch.de
 
 On Wed, 3 Jul 2013, Mark Kettenis wrote:
   diff --git sys/arch/i386/i386/db_interface.c 
   sys/arch/i386/i386/db_interface.c
   index 85c1ff5..c75fd89 100644
   --- sys/arch/i386/i386/db_interface.c
   +++ sys/arch/i386/i386/db_interface.c
   @@ -197,11 +197,11 @@ db_sysregs_cmd(db_expr_t addr, int have_addr, 
   db_expr_t count, char *modif)
 uint16_t ldtr, tr;

 __asm__ __volatile__(sidt %0 : =m (idtr));
   - db_printf(idtr:   0x%08x/%04x\n,
   + db_printf(idtr:   0x%08x/%04llx\n,
 (unsigned int)(idtr  16), idtr  0x);

 __asm__ __volatile__(sgdt %0 : =m (gdtr));
   - db_printf(gdtr:   0x%08x/%04x\n,
   + db_printf(gdtr:   0x%08x/%04llx\n,
 (unsigned int)(gdtr  16), gdtr  0x);
  
  This is a tad bit inconsistent.  I'd either use %llx for both values
  and get rid of the cast, or use %x and use a cast in both cases.
 
 Like this?

ok kettenis@

 --- sys/arch/i386/i386/db_interface.c
 +++ sys/arch/i386/i386/db_interface.c
 @@ -197,12 +197,10 @@ db_sysregs_cmd(db_expr_t addr, int have_addr, db_expr_t 
 count, char *modif)
   uint16_t ldtr, tr;
  
   __asm__ __volatile__(sidt %0 : =m (idtr));
 - db_printf(idtr:   0x%08x/%04x\n,
 - (unsigned int)(idtr  16), idtr  0x);
 + db_printf(idtr:   0x%08llx/%04llx\n, idtr  16, idtr  0x);
  
   __asm__ __volatile__(sgdt %0 : =m (gdtr));
 - db_printf(gdtr:   0x%08x/%04x\n,
 - (unsigned int)(gdtr  16), gdtr  0x);
 + db_printf(gdtr:   0x%08llx/%04llx\n, gdtr  16, gdtr  0x);
  
   __asm__ __volatile__(sldt %0 : =g (ldtr));
   db_printf(ldtr:   0x%04x\n, ldtr);
 



Re: Removing -Wno-format from kernel makefiles, 4/16

2013-07-04 Thread Mark Kettenis
 Date: Thu, 4 Jul 2013 18:42:50 +0200 (CEST)
 From: Stefan Fritsch s...@sfritsch.de
 
 On Wed, 3 Jul 2013, Mark Kettenis wrote:
   diff --git sys/arch/i386/i386/esm.c sys/arch/i386/i386/esm.c
   index c90b2c4..3dff69e 100644
   --- sys/arch/i386/i386/esm.c
   +++ sys/arch/i386/i386/esm.c
   @@ -880,7 +880,7 @@ esm_make_sensors(struct esm_softc *sc, struct 
   esm_devmap *devmap,
 }

 for (j = 0; j  nsensors; j++) {
   - snprintf(s[j].desc, sizeof(s[j].desc), %s %d,
   + snprintf(s[j].desc, sizeof(s[j].desc), %s %ld,
 sensor_map[i].name, sensor_map[i].arg + j);
 }
 break;
  
  Looking at this one, it makes more sense to make the arg member of
  struct esm_sensor_map an int.  That will result in some space
  savings if we'd ever bring this driver to amd64.
 

go for it

 --- sys/arch/i386/i386/esm.c
 +++ sys/arch/i386/i386/esm.c
 @@ -87,7 +87,7 @@ enum sensor_type esm_typemap[] = {
  
  struct esm_sensor_map {
   enum esm_sensor_typetype;
 - longarg;
 + int arg;
   const char  *name;
  };
  
 
 
 



Re: Removing -Wno-format from kernel makefiles, 06/16

2013-07-04 Thread Alexander Bluhm
On Thu, Jul 04, 2013 at 06:39:03PM +0200, Stefan Fritsch wrote:
 diff --git sys/netinet/ip_output.c sys/netinet/ip_output.c
 index b59accf..43a0551 100644
 --- sys/netinet/ip_output.c
 +++ sys/netinet/ip_output.c
 @@ -267,7 +267,7 @@ reroute:
   if (mtag != NULL) {
  #ifdef DIAGNOSTIC
   if (mtag-m_tag_len != sizeof (struct tdb_ident))
 - panic(ip_output: tag of length %d (should be %d,
 + panic(ip_output: tag of length %hu (should be %zu,
   mtag-m_tag_len, sizeof (struct tdb_ident));
  #endif
   tdbi = (struct tdb_ident *)(mtag + 1);
 diff --git sys/netinet6/ip6_forward.c sys/netinet6/ip6_forward.c
 index 6d7f971..1dff149 100644
 --- sys/netinet6/ip6_forward.c
 +++ sys/netinet6/ip6_forward.c
 @@ -160,7 +160,7 @@ reroute:
   if (mtag != NULL) {
  #ifdef DIAGNOSTIC
   if (mtag-m_tag_len != sizeof (struct tdb_ident))
 - panic(ip6_forward: tag of length %d (should be %d,
 + panic(ip6_forward: tag of length %hu (should be %zu,
   mtag-m_tag_len, sizeof (struct tdb_ident));
  #endif
   tdbi = (struct tdb_ident *)(mtag + 1);
 diff --git sys/netinet6/ip6_output.c sys/netinet6/ip6_output.c
 index f405b31..baf4103 100644
 --- sys/netinet6/ip6_output.c
 +++ sys/netinet6/ip6_output.c
 @@ -232,7 +232,7 @@ ip6_output(struct mbuf *m0, struct ip6_pktopts *opt, 
 struct route_in6 *ro,
   if (mtag != NULL) {
  #ifdef DIAGNOSTIC
   if (mtag-m_tag_len != sizeof (struct tdb_ident))
 - panic(ip6_output: tag of length %d (should be %d,
 + panic(ip6_output: tag of length %hu (should be %zu,
   mtag-m_tag_len, sizeof (struct tdb_ident));
  #endif
   tdbi = (struct tdb_ident *)(mtag + 1);

netinet and netinet6 is OK bluhm@

 diff --git sys/nfs/nfs_socket.c sys/nfs/nfs_socket.c
 index e0f28e4..e1f7b3d 100644
 --- sys/nfs/nfs_socket.c
 +++ sys/nfs/nfs_socket.c
 @@ -600,7 +600,7 @@ tryagain:
   } while (error == EWOULDBLOCK);
   if (!error  auio.uio_resid  0) {
   log(LOG_INFO,
 -  short receive (%d/%d) from nfs server %s\n,
 +  short receive (%zd/%zd) from nfs server %s\n,
sizeof(u_int32_t) - auio.uio_resid,
sizeof(u_int32_t),
rep-r_nmp-nm_mountp-mnt_stat.f_mntfromname);

This should be %zu/%zu

 @@ -631,7 +631,7 @@ tryagain:
error == ERESTART);
   if (!error  auio.uio_resid  0) {
   log(LOG_INFO,
 - short receive (%d/%d) from nfs server %s\n,
 + short receive (%zu/%u) from nfs server %s\n,
   len - auio.uio_resid, len,
   rep-r_nmp-nm_mountp-mnt_stat.f_mntfromname);
   error = EPIPE;
 -- 
 1.7.6

Note that in nfs is another len that should be %u.

log(LOG_ERR, %s (%d) from nfs server %s\n,
impossible packet length,
len,
rep-r_nmp-nm_mountp-mnt_stat.f_mntfromname);

bluhm