FSTAB usage in quotacheck

2014-07-06 Thread Martin Natano
FSTAB has been deprecated in favor of _PATH_FSTAB in 4.4BSD. It's time to let go of it. Below the diff I committed to Bitrig. cheers, natano Replace deprecated FSTAB with _PATH_FSTAB; no binary change. ok oga@ diff --git a/sbin/quotacheck/quotacheck.c b/sbin/quotacheck/quotacheck.c index

fusefs_quotactl return value

2014-10-03 Thread Martin Natano
fusefs_quotactl() unconditionally returns zero, despite fusefs not supporting quotas. I think it should return EOPNOTSUPP. At least that is what cd9660, udf, fuse, msdosfs, tmpfs and nfs do. cheers, natano --- fuse_vfsops.c.orig Thu Sep 11 14:53:36 2014 +++ fuse_vfsops.c Thu Sep 11

ksh garbage printing

2014-10-04 Thread Martin Natano
The following patch fixes a bug in ksh I reported some time ago (http://marc.info/?l=openbsd-bugsm=137292039914229w=2). I committed this patch to Bitrig in December 2013; it seems to work fine so far. Here the bug report inline for reference. Description: When using a keybinding that

ksh emacs mode cleanup

2014-10-04 Thread Martin Natano
Below some code cleanup for ksh's emacs mode. cheers, natano --- Get rid of left over null elements in x_ftab as NELEM() is used instead. Index: emacs.c === RCS file: /cvs/src/bin/ksh/emacs.c,v retrieving revision 1.48 diff -u

tree.h style nit-pick

2014-10-05 Thread Martin Natano
Let's make tree.h conform to style(9). Index: tree.h === RCS file: /cvs/src/sys/sys/tree.h,v retrieving revision 1.13 diff -u -r1.13 tree.h --- tree.h 9 Jul 2011 00:19:45 - 1.13 +++ tree.h 5 Oct 2014 17:25:24

Re: another ancient bug in head(1)

2014-10-08 Thread Martin Natano
Index: head.c === RCS file: /cvs/src/usr.bin/head/head.c,v retrieving revision 1.17 diff -u -p -r1.17 head.c --- head.c7 Oct 2014 19:38:57 - 1.17 +++ head.c7 Oct 2014 21:16:43 - @@ -48,6 +48,7 @@ static

Re: tree.h style nit-pick

2014-10-10 Thread Martin Natano
On Fri, Oct 10, 2014 at 01:02:17PM -0400, Ted Unangst wrote: On Sun, Oct 05, 2014 at 19:42, Martin Natano wrote: Let's make tree.h conform to style(9). Index: tree.h === RCS file: /cvs/src/sys/sys/tree.h,v retrieving

ps: remove redundant prototype

2014-10-12 Thread Martin Natano
The findvar() function in keyword.c contains a prototype of the vcmp() function, which is already declared further up in the same file. I'm not even sure that prototype is correct, as it fails to include the 'static' classifier (vcmp() is a static function). cheers, natano Index: keyword.c

vi: remove bitstring.h copy

2014-10-13 Thread Martin Natano
The vi editor includes its own (outdated) copy of bitstring.h. It can safely be removed; the binary doesn't change. cheers, natano --- usr.bin/vi/LAYOUT 29 Jan 2001 01:58:24 - 1.4 +++ usr.bin/vi/LAYOUT 13 Oct 2014 19:12:53 - @@ -103,7 +103,6 @@ include/ Replacement

vi: remove obsolete headers, documentation cleanup

2014-10-13 Thread Martin Natano
There are some left-over (unused) header files from removed components in vi. The following patch deletes them and cleans up the documentation to remove references to these components. No binary change. cheers, natano --- Index: LAYOUT

Re: apmd: remove useless defines

2014-10-27 Thread Martin Natano
Remove now useless defines. ok? Index: apmd.c === RCS file: /cvs/src/usr.sbin/apmd/apmd.c,v retrieving revision 1.72 diff -u -p -u -p -r1.72 apmd.c --- apmd.c26 Oct 2014 22:16:16 - 1.72 +++ apmd.c27 Oct

mg: exit code cleanup

2014-11-09 Thread Martin Natano
mg(1) calls 'exit(1)' on failure, but 'exit(GOOD)' on success. In my opinion it would be more readable to just use 'exit(0)' for a normal exit. (If there really is the need for a define, EXIT_SUCCESS would be a better fit anyways, and EXIT_* should be applied consistently.) Also, the MALLOCROUND()

vi: config.h cleanup

2014-11-10 Thread Martin Natano
Let's have a look at build/config.h: The comments at the top are misleading: the file is not automatically generated by configure, and config.h.in doesn't exist. The following constants are used to prevent conditional declaration of some standard libc functions:

vi: remove ipc left-overs

2014-11-10 Thread Martin Natano
There is some left-over code from the ipc component, which has been removed. See diff below; no binary change. cheers, natano Index: cl/cl_main.c === RCS file: /cvs/src/usr.bin/vi/cl/cl_main.c,v retrieving revision 1.20 diff -u

vi: remove workarounds for other systems

2014-11-10 Thread Martin Natano
There are two workarounds for a broken libc on linux, one for a missing MAXHOSTNAMELEN and one portable reimplementation of strsignal (with another name though). None of those are necessary for OpenBSD. See diff below. cheers, natano Index: common/exf.c

vi: remove portability goo

2014-11-11 Thread Martin Natano
The more I dig into vi, the more portability goo I find: The TRUE and FALSE constants are defined by curses.h, thus no need to check for them. The mvchgat macro, VWERASE and tons of other macros are present in OpenBSDs curses implementation, this means a lot of the #ifdef maze can be removed.

vi: global state cleanup

2014-11-12 Thread Martin Natano
The 'struct _gs' type (aka. GS) contains pointers to screen interface functions, which are set by cl_func_std(), but this mechanism isn't necessary anymore, because the other frontends (tk, motif, ipc) have been removed. The only remaining frontend is curses and can be used directly. The only

Re: vi: revise memory allocation handling

2014-11-12 Thread Martin Natano
--- common/screen.c 12 Nov 2014 04:28:41 - 1.11 +++ common/screen.c 12 Nov 2014 07:20:51 - @@ -84,16 +84,16 @@ goto mem; sp-subre_len = orig-subre_len; if (orig-repl != NULL (sp-repl = - v_strdup(sp, orig-repl,

mandoc: preconv endian issue

2014-11-13 Thread Martin Natano
While reading preconv.c two peculiarities catched my eye: 1. The preconv_encode() function does manual byteswapping where none is necessary (and harmful). While the bit-shifting used to construct the value in 'accum' might seem endian specific on first sight, it is not. The bug manifests itself

Re: mandoc: preconv endian issue

2014-11-14 Thread Martin Natano
The correct unicode codepoint would be U+00FC, not U+FC00. 'accum' already contained the correct value _before_ byteswapping. Code review and testing confirmed that this analysis is completely correct. So i have committed this part of the patch. Thanks! 2. The code in

vi: file locking

2014-11-14 Thread Martin Natano
The vi editor contains code for two different file locking methods - one using flock(), the other using fcntl(). The fcntl method is unused and has severe limitations (as described in a code comment). Let's remove it for sake of readibility. See the diff below; no binary change. cheers, natano

vi: stale prototype

2014-11-15 Thread Martin Natano
The sscr_pty() function has been removed in r1.18. Let's kill the remaining prototype. cheers, natano Index: ex/ex_script.c === RCS file: /cvs/src/usr.bin/vi/ex/ex_script.c,v retrieving revision 1.21 diff -u -r1.21 ex_script.c ---

vi: perl api

2014-11-15 Thread Martin Natano
When building vi with PERLINTERP=yes the build errors out with the following message: /usr/src/usr.bin/vi/build/../ex/ex_perl.c: In function 'ex_perl': /usr/src/usr.bin/vi/build/../ex/ex_perl.c:59: error: implicit declaration of function 'perl_ex_perl'

vi: the LIBRARY is a lie

2014-11-15 Thread Martin Natano
The LIBRARY define is undocumenteed and triggers the same conditional inclusions as PURIFY does. See the diff below, no binary change. cheers, natano Index: cl/cl_main.c === RCS file: /cvs/src/usr.bin/vi/cl/cl_main.c,v retrieving

vi: mvchgat

2014-11-15 Thread Martin Natano
I missed this part of the workaround for a missing mvchgat(); no binary change. Index: cl/cl_funcs.c === RCS file: /cvs/src/usr.bin/vi/cl/cl_funcs.c,v retrieving revision 1.17 diff -u -r1.17 cl_funcs.c --- cl/cl_funcs.c 12 Nov

vi: filemodes

2014-11-15 Thread Martin Natano
The S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH mode can be replaced by DEFFILEMODE. See the diff below; no binary change. cheers, natano Index: common/exf.c === RCS file: /cvs/src/usr.bin/vi/common/exf.c,v retrieving revision

vi: cl_putchar

2014-11-15 Thread Martin Natano
The cl_putchar() function does nothing, but call putchar(). stacklevel-- cheers, natano Index: cl/cl_funcs.c === RCS file: /cvs/src/usr.bin/vi/cl/cl_funcs.c,v retrieving revision 1.17 diff -u -r1.17 cl_funcs.c --- cl/cl_funcs.c

vi: #include workarounds

2014-11-15 Thread Martin Natano
Remove #include workarounds; no binary change. cheers, natano Index: common/exf.c === RCS file: /cvs/src/usr.bin/vi/common/exf.c,v retrieving revision 1.32 diff -u -r1.32 exf.c --- common/exf.c14 Nov 2014 20:23:56 -

Re: vi: global state cleanup

2014-11-20 Thread Martin Natano
On Wed, Nov 12, 2014 at 07:34:28PM +0100, Martin Natano wrote: The 'struct _gs' type (aka. GS) contains pointers to screen interface functions, which are set by cl_func_std(), but this mechanism isn't necessary anymore, because the other frontends (tk, motif, ipc) have been removed. The only

Re: ntpd: prefer %z when formatting size_t

2015-02-10 Thread Martin Natano
Obviously not that trivial: The return type of sizeof() is size_t, which is unsigned, thus the second conversion specification should be %zu. (The printf manpage notes that the %zd conversion indicates that the argument is of a signed type equivalent in size to a size_t.). natano On Mon, Feb 09,

fusefs_readdir() should set eofflag

2015-02-18 Thread Martin Natano
fuse_readdir() fails to set the eofflag correctly. The consequence of this is, that callers of VOP_READDIR, that examine the value of the eofflag after the call, might be mislead about the eof status, as the flag hasn't been modified (and my even be uninitialized). The manual page for

Re: fusefs_readdir() should set eofflag

2015-02-19 Thread Martin Natano
@@ -733,6 +734,9 @@ fb_delete(fbuf); } + + if (!error ap-a_eofflag != NULL) + *ap-a_eofflag = eofflag; Is the null check here necessary? Other file systems don't do this, so I'm wondering if you encountered a null pointer here. I didn't encounter a

Re: report thread scheduling setting properly

2015-04-23 Thread Martin Natano
On Tue, Apr 21, 2015 at 08:03:43PM -0700, Philip Guenther wrote: While we don't actually make use of thread scheduling attributes, we do support setting and retrieving them. However, it doesn't make any sense to let you set them in a pthread_attr_t...but then ignore that in

grep -R without directory argument

2015-04-29 Thread Martin Natano
grep reads from standard input when no files are specified. It also does so when -R is used, which doesn't really make sense. I think using the current working directory as a fallback when no directories are specified would make sense. POSIX says If no file operands are specified, the standard

prevent busy loop in rnd.c

2015-04-08 Thread Martin Natano
Due to an involuntary integer overflow, the randomread() and randomwrite() functions can be tricked into entering an endless loop. That loop can be triggered by performing a read() of UINT_MAX + 1 bytes from /dev/random on a platform where sizeof(size_t) sizeof(u_int). This holds true for e.g.

Re: prevent busy loop in rnd.c

2015-04-08 Thread Martin Natano
Your diff looks correct, though I don't think the explicit casts are needed since the prototypes are in scope. Perhaps it quiets a warning with some compiler flags? The casts are not strictly necessary and you are correct about the warnings; both gcc (I believe since version 4.3 with the

prevent short transfer in lptwrite()

2015-04-09 Thread Martin Natano
uio_resid may overflow when coerced to u_int, causing lptwrite() to return early. Also, uiomovei() can be replaced with uiomove(), the size argument is of type size_t anyway. Index: dev/ic/lpt.c === RCS file:

prevent short transfer in nvramread()

2015-04-09 Thread Martin Natano
Below another min() - ulmin() conversion to prevent integer overflow. The size (tmp - buf) passed to uiomovei() is essentially bound by the 'count' variable, thus convert to uiomove(). Index: arch/amd64/amd64/nvram.c === RCS file:

Kill emulsw co.

2015-07-05 Thread Martin Natano
There are some left-over declarations for emulsw, nemuls, emul_elf32 and emul_elf64, all of which have been removed. Ok? Index: kern/exec_conf.c === RCS file: /cvs/src/sys/kern/exec_conf.c,v retrieving revision 1.32 diff -u -p -u

Re: [patch 3/3] ksh: add overflow checking for memory allocations

2015-05-24 Thread Martin Natano
I highly doubt any part of those three diffs is authored by yourself. (Merely renaming functions doesn't count as original work.) As pointed out before, the efficient overflow checking code is copied from Otto's code and the rest has been written by me. In case you don't remember the location

vattr_null() doesn't initialize va_filerev

2015-10-08 Thread Martin Natano
Filesystem implementations depend on vattr_null() to initialize the fields in struct vattr, which is true for all the fields except va_filerev. It therefore is not set to VNOVAL as expected by the file system, but contains whatever was there on the stack. This causes VOP_GETATTR() on cd9660 and

Add APM_IOC_HIBERNATE to apm.4

2015-10-13 Thread Martin Natano
The following patch adds documentation for the APM_IOC_HIBERNATE ioctl to the manual pages. cheers, natano Index: share/man/man4/man4.amd64/apm.4 === RCS file: /cvs/src/share/man/man4/man4.amd64/apm.4,v retrieving revision 1.5 diff

Re: Using tame() in userland

2015-08-30 Thread Martin Natano
Inline comments below. --- tsort.c 29 Jul 2015 10:42:37 - 1.26 +++ tsort.c 28 Aug 2015 08:03:59 - @@ -798,6 +799,43 @@ find_longest_cycle(struct array *h, stru #define plural(n) ((n) 1 ? s : ) +static void +TAME(int flags, const char *wl[]) +{ + if

pppd: remove unused function

2015-09-03 Thread Martin Natano
The get_host_seed() function in pppd is unused and can be removed. Index: pppd.h === RCS file: /cvs/src/usr.sbin/pppd/pppd.h,v retrieving revision 1.19 diff -u -p -u -r1.19 pppd.h --- pppd.h 12 Jun 2015 14:18:25 - 1.19

cd9660 uiomove() conversion

2016-01-04 Thread Martin Natano
Below the uiomove conversion for isofs/cd9660/cd9660_vnops.c. cheers, natano Index: isofs/cd9660/cd9660_vnops.c === RCS file: /cvs/src/sys/isofs/cd9660/cd9660_vnops.c,v retrieving revision 1.73 diff -u -p -r1.73 cd9660_vnops.c ---

i386/nvram.c uiomove() conversion

2016-01-04 Thread Martin Natano
Below a diff to convert i386/nvram.c to use uiomove(). A similar diff for amd64 has already been committed. (See https://marc.info/?l=openbsd-tech=142860367111291) Also, pos is converted to off_t for both amd64 and i386 to prevent truncation and signedness mismatch. Calling nvramread() with a

tmpfs uiomove() conversion

2016-01-02 Thread Martin Natano
Below a diff to convert tmpfs/tmpfs_{subr,vnops}.c. That's an easy one. Nearly all the relevant size variables already are a size_t, or a smaller unsigned type. In the last hunk tn_size (an off_t) might be passed to uiomove(), which is not problematic, because tn_size is constrained to only hold

dev/rnd.c uiovmove() conversion

2016-01-02 Thread Martin Natano
Below, a diff to convert dev/rnd.c to use uiomove(). cheers, natano Index: dev/rnd.c === RCS file: /cvs/src/sys/dev/rnd.c,v retrieving revision 1.177 diff -u -p -u -r1.177 rnd.c --- dev/rnd.c 28 Dec 2015 05:21:53 - 1.177

integer truncation in soreceive()

2015-12-31 Thread Martin Natano
Another integer overflow: A recv() call with a size of 2^32 bytes causes soreceive() to spin in an endless loop, resulting in a system freeze. The diff below prevents this behaviour by establishing an upper bound for uio_resid before assigning the value to an integer variable with smaller width.

bpf uiomove() conversion

2016-01-09 Thread Martin Natano
Below the uiomove() conversion for bpf. bd_hlen is a signed integer, but can't be negative, because it contains the size of a buffer. Thus, the conversion to size_t is ok. Index: net/bpf.c === RCS file: /cvs/src/sys/net/bpf.c,v

uvm_io.c uiomove() conversion

2016-01-09 Thread Martin Natano
Below the conversion for uvm_io.c. The sz variable is already a size_t. Index: uvm/uvm_io.c === RCS file: /cvs/src/sys/uvm/uvm_io.c,v retrieving revision 1.25 diff -u -p -u -r1.25 uvm_io.c --- uvm/uvm_io.c14 Mar 2015 03:38:53

drm_drv.c uiomove() conversion

2016-01-09 Thread Martin Natano
Below the uiomove() conversion for drm_drv.c. ev->event->length is a u_int32_t, thus the implicit conversion to size_t should be non-problematic. Index: ./dev/pci/drm/drm_drv.c === RCS file: /cvs/src/sys/dev/pci/drm/drm_drv.c,v

klog uiomove() conversion

2016-01-09 Thread Martin Natano
Below the uiomove() conversion for kern/subr_log.c. msg_buf[rsx] are all of type long, but are always positive. This diff prevents truncation of uio_resid (and l) due to min() usage. Index: kern/subr_log.c === RCS file:

sys_pipe.c uiomove() conversion

2016-01-09 Thread Martin Natano
Below the uiomove() conversion for kern/sys_pipe.c. This diff eliminates a potential overflow of the nread variable. I think it would benefit readability to sprinkle some ulmin()/MIN() into that code and wrap lines to <80 chars, but that's probably a matter for another diff. Including those

0 is UIO_USERSPACE

2016-01-09 Thread Martin Natano
0 is the same as UIO_USERSPACE, but more readable. No binary change. Index: ./arch/octeon/dev/amdcf.c === RCS file: /cvs/src/sys/arch/octeon/dev/amdcf.c,v retrieving revision 1.1 diff -u -p -u -r1.1 amdcf.c ---

fusefs uiomove() conversion

2016-01-08 Thread Martin Natano
Below the conversion from uiovmovei() to uiomove() for miscfs/fusefs/. All size parameters already are size_t, so the diff is straightforward. Index: miscfs/fuse/fuse_device.c === RCS file: /cvs/src/sys/miscfs/fuse/fuse_device.c,v

ksyms uiomove() conversion

2016-01-09 Thread Martin Natano
Below the uiomove() conversion for dev/ksyms.c. 'len' is a size_t. Index: dev/ksyms.c === RCS file: /cvs/src/sys/dev/ksyms.c,v retrieving revision 1.30 diff -u -p -u -r1.30 ksyms.c --- dev/ksyms.c 29 Aug 2015 01:58:39 - 1.30

wsevent.c uiomove() conversion

2016-01-09 Thread Martin Natano
Below the uiomove() conversion for dev/wscons/wsevent.c. 'cnt' could as well be a size_t, but using u_int makes clear, that it will never exceed UINT_MAX, and that 'ev->get = cnt;' won't overflow. Index: dev/wscons/wsevent.c === RCS

infinite loop in randomread()/randomwrite()

2015-12-26 Thread Martin Natano
A read of 2^32 bytes can trigger an endless loop in randomread(), due to integer truncation when passing a size_t argument to min(). There is a similar issue in randomwrite(). The diff included below is a minimal version of a similar diff I've sent to tech@ some months ago, but with peripheral

Re: hashfree, a counterpart to hashinit

2015-12-24 Thread Martin Natano
Hi, the diff reads fine to me, except for following nit-picks: It would be nice if the hashfree() function would check for negative/zero element counts, like hashinit() does. Also, new functionality should be documented in the manual. Please find below a regenerated diff including both changes.

Re: replace bpf open loops

2016-05-28 Thread Martin Natano
On Fri, May 27, 2016 at 10:03:37PM +0200, Jeremie Courreges-Anglas wrote: > Martin Natano <nat...@natano.net> writes: > > > I think it's time to get rid of all the bpf open() loops in base. > > dhclient and libpcap do a plain open("/dev/bpf0", ...) since a coupl

Re: ptrace PT_IO write bug

2016-05-28 Thread Martin Natano
On Fri, May 27, 2016 at 10:15:39PM +0200, Jeremie Courreges-Anglas wrote: > Mathieu - writes: > > > Hello everyone, > > > > While playing a bit with ptrace to do some debugging I stumbled upon > > something that looks like a bug. > > While trying to write to the ptrace'd

lockmgr() api removal

2016-05-29 Thread Martin Natano
It is time for the lockmgr() api to die. The api is only used by filesystems, where it is a trivial change to use rrw locks instead. All it needs is LK_* defines for the RW_* flags. (See the sys/lock.h hunk in the diff below.) The ffs regress tests display the same number of fail/ok results

Re: prefer AF_* over PF_*

2016-05-29 Thread Martin Natano
On Sat, May 28, 2016 at 07:55:00PM -0700, Philip Guenther wrote: > > About the only place userland code should use PF_* socket constants is > with sysctl(3)'s CTL_NET hierarchy. All the standardized functions are > defined as taking AF_* values. Let's use the preferred names in the >

Re: libc: delete unused hash algorithms

2016-05-29 Thread Martin Natano
On Sat, May 28, 2016 at 07:47:50PM -0700, Philip Guenther wrote: > > Overriding the hash algorithm used by the Berkeley DB bits isn't support > (it would break getpw* if nothing else) and hasn't been possible since the > symbol hiding effort last fall. So eliminate the redirection through a >

remove doforce from kern/vfs_subr.c

2016-05-26 Thread Martin Natano
The doforce variable isn't modified anywhere. Also, the only filesystem left using it is fuse. It has been removed from all other filesystems. Ok to remove? natano Index: kern/vfs_subr.c === RCS file:

replace bpf open loops

2016-05-26 Thread Martin Natano
I think it's time to get rid of all the bpf open() loops in base. dhclient and libpcap do a plain open("/dev/bpf0", ...) since a couple of weeks now and the upgrade issue (/dev/bpf vs. /dev/bpf0) has been fixed. I didn't hear any other complaints in the meantime. Ok? Too soon? natano Index:

Re: use ${CC} -E rather than ${CPP}

2016-05-26 Thread Martin Natano
See one comment inline. On Thu, May 26, 2016 at 09:59:24AM -0600, Todd C. Miller wrote: > Since /usr/bin/cpp still specified -traditional, use ${CC} -E > instead. I've had this rotting in my tree for years now. > > - todd > > Index: lib/libcurses/Makefile >

Re: use ${CC} -E rather than ${CPP}

2016-05-26 Thread Martin Natano
On Thu, May 26, 2016 at 02:14:29PM -0600, Todd C. Miller wrote: > On Thu, 26 May 2016 22:07:14 +0200, Martin Natano wrote: > > > > diff -u -p -u -r1.11 Makefile > > > --- share/locale/ctype/Makefile 20 Mar 2016 15:45:40 - 1.11 > > > +++ share/locale/c

Re: continue in empty loops

2016-06-01 Thread Martin Natano
On Wed, Jun 01, 2016 at 01:08:05PM -0400, Ted Unangst wrote: > I find this easier to read. My old eyes don't focus on the semicolon, which > makes me wonder what's supposed to be happening. Yes, please! ok natano@ > > > Index: pax/ar_io.c >

Re: ancient relics in newsyslog

2016-06-01 Thread Martin Natano
On Wed, Jun 01, 2016 at 01:43:06AM -0400, Ted Unangst wrote: > Let's make the defaults be the defaults. Reads fine. ok natano@ > > Index: Makefile > === > RCS file: /cvs/src/usr.bin/newsyslog/Makefile,v > retrieving revision 1.6 >

ntfs mkdir errno value

2016-06-01 Thread Martin Natano
The following patch addresses an issue found by landry. mkdir on an ntfs mountpoint returns ENOENT, while the error should be EROFS. We don't support write access on ntfs. Ok? natano Index: ntfs/ntfs_subr.c === RCS file:

ppp_tty uiomove() conversion

2016-01-13 Thread Martin Natano
Below the uiomove() conversion for net/ppp_tty.c. M_TRAILINGSPACE() returns int, but the result can't be negative, so using u_int for the return value should be fine. Index: net/ppp_tty.c === RCS file: /cvs/src/sys/net/ppp_tty.c,v

spec_vnops.c uiomove() conversion

2016-01-13 Thread Martin Natano
Below the conversion to uiomove() for kern/spec_vnops.c. This diff prevents truncation of uio_resid when passed to min(). Index: kern/spec_vnops.c === RCS file: /cvs/src/sys/kern/spec_vnops.c,v retrieving revision 1.84 diff -u -p -u

tty uiomove() conversion

2016-01-13 Thread Martin Natano
Below the conversion from uiomovei() to uiomove() for kern/tty.c and kern/tty_pty.c. 'cc' consistently contains small, non-negative integer values, so leaving the type as int should be ok. It could as well be changed to size_t, but I don't see a benefit in doing so for that particular case, except

Re: [patch] ls + utf-8 support

2016-01-17 Thread Martin Natano
I agree with Ingo, ls(1) shouldn't generate unsafe output. Regardless of whether the output is to a terminal or some other file. cheers, natano

Re: [patch] kern/exec_script: return error when the shell name is not specified

2016-02-06 Thread Martin Natano
I think that 'cp += shellarglen + 1;' is a dead store. The 'cp' variable is not used anymore after that line. Either way your diff reads fine to me! natano > Index: sys/kern/exec_script.c > === > RCS file:

ufs/ffs/ext2fs uiomove() conversion

2016-02-07 Thread Martin Natano
Below the conversion to uiomovei() for ufs. While there I changed all instances of 'blkoffset', 'size' and 'xfersize' that where declared as long integers to be plain integers instead for consistency with the surrounding code. These variables are limited by fs_bsize and e2fs_bsize respectively,

Re: ufs/ffs/ext2fs uiomove() conversion

2016-02-08 Thread Martin Natano
> One unrelated thing noted while reviewing: > > ufs/ext2fs/ext2fs_readwrite.c: > static int > ext2_ind_read(struct vnode *vp, struct inode *ip, struct m_ext2fs *fs, > struct uio *uio) > { > ... > > if (vp->v_type == VLNK) { > if ((int)ext2fs_size(ip) <

diff -e: mishandling of bare dots

2016-02-13 Thread Martin Natano
When diff encounters a line that consists of a single dot, it emits two dots instead, stops the current command and emits a substitute command to replace the double dot with a single one. Then it restarts the (original) command if necessary and inserts further lines. This is done because a single

Re: udf uiomove() conversion

2016-01-27 Thread Martin Natano
On Wed, Jan 27, 2016 at 06:26:00AM +0100, Stefan Kempf wrote: > Martin Natano wrote: > > Below the conversion to uiomove() for isofs/udf/. Note that converting > > size to size_t is not possible in udf_read(), as udf_readatoffset() > > requires a pointer to an integer variable

ntfs uiomove() conversion

2016-01-31 Thread Martin Natano
Below the conversion to uiomove() for ntfs. In the first three hunks the size passed to uiomove(i) already was of type size_t. I also converted the 'left' variable in ntfs_readattr() to size_t, because it tracks the remainder of 'rsize', which also is size_t. Index: ntfs/ntfs_subr.c

Re: Add APM_IOC_HIBERNATE to apm.4

2016-02-02 Thread Martin Natano
> The following patch adds documentation for the APM_IOC_HIBERNATE ioctl > to the manual pages. There was a typo in the zaurus hunk. Updated diff below. Index: share/man/man4/man4.amd64/apm.4 === RCS file:

nfs uiomove() conversion

2016-01-27 Thread Martin Natano
Below the uiomove() conversion for nfs. I didn't change the type of 'n' to be size_t, because it never exceeds the maximum rpc size (nm_rsize), which is an integer too. (Also, to avoid unnecessary code churn.) Index: nfs/nfs_bio.c

Re: diff -e: mishandling of bare dots

2016-02-27 Thread Martin Natano
Last chance to voice objections. I plan to commit this in the next days. natano > Index: diffreg.c > === > RCS file: /cvs/src/usr.bin/diff/diffreg.c,v > retrieving revision 1.90 > diff -u -p -r1.90 diffreg.c > --- diffreg.c 26 Oct

read(2) shouldn't return EFBIG

2016-02-28 Thread Martin Natano
The ext2fs_read() and ffs_read() functions return EFBIG when uio_offset if smaller than zero or larger than the maxium file size. However this doesn't seem to be in accordance with the POSIX read(2) specification, which requires EINVAL for an invalid offset (< 2) and a return of 0 (zero bytes

Re: tmpfs improvements

2016-02-28 Thread Martin Natano
On Sat, Feb 27, 2016 at 05:44:49PM -0500, Michael McConville wrote: > * There wasn't yet a list of possible errors for tmpfs in mount(2). >That said, I question the value of maintaining these lists. It's >almost impossible for them to be comprehensive - for example, some >*_mount()

Re: read(2) shouldn't return EFBIG

2016-02-28 Thread Martin Natano
On Sun, Feb 28, 2016 at 04:40:21PM +0100, Stefan Kempf wrote: > Stefan Kempf wrote: > > Martin Pieuchot wrote: > > > I'm also wondering when you say "an offset that's at or paste the > > > EOF" does that include ``uio_resid''? I mean shouldn't you check > > > for: > > > > > > if

stale entries for RAIDframe in chrtoblktbl

2016-02-25 Thread Martin Natano
For most architectures there still is an entry for the removed RAIDframe device lurking in chrtoblktbl. While there I truncated the tables to the minimum required size; chrtoblk() and blktochr() are designed to handle a table shorter than cdevsw. Ok? natano Index: arch/alpha/alpha/conf.c

Re: Switch to uiomove in usb

2016-02-29 Thread Martin Natano
On Sun, Feb 28, 2016 at 08:14:39PM +0100, Stefan Kempf wrote: > This changes uiomovei calls to uiomove in usb. It fixes > a few integer truncations due to use of min, and uses > unsigned types for count variables where it makes sense. > This also allows us to get rid of a couple of 'if (len < 0)'

Re: monitor_fdpass.c: print ssize_t with %zd

2016-02-29 Thread Martin Natano
On Mon, Feb 29, 2016 at 07:56:22PM +0100, Jeremie Courreges-Anglas wrote: > > ok? Looks good to me. > > Index: libexec/ftpd/monitor_fdpass.c > === > RCS file: /cvs/src/libexec/ftpd/monitor_fdpass.c,v > retrieving revision 1.6 >

Re: move mnt_maxsymlinklen to struct ufsmount

2016-02-26 Thread Martin Natano
Sorry, the second message was not supposed to go to the list...

move mnt_maxsymlinklen to struct ufsmount

2016-02-26 Thread Martin Natano
The concept of differentiating between "short" and "long" symlinks is specific to ufs/, but the relevant variable is contained in struct mount, which is used by all filesystems. I think it would be a worthwile effort to clear this up by moving maxsymlinklen to struct ufsmount instead. Filesystem

Re: move mnt_maxsymlinklen to struct ufsmount

2016-02-26 Thread Martin Natano
at this. natano On Fri, Feb 26, 2016 at 11:32:08PM +0100, Martin Natano wrote: > The concept of differentiating between "short" and "long" symlinks is > specific to ufs/, but the relevant variable is contained in struct > mount, which is used by all filesystems. I think it

Re: Harmful casts in ufs

2016-02-22 Thread Martin Natano
On Mon, Feb 22, 2016 at 06:31:00AM -0700, Todd C. Miller wrote: > On Sun, 21 Feb 2016 16:33:27 +0100, Stefan Kempf wrote: > > > Should we really mount the FS in that case? If the FS was of the > > "new" format, then short symlinks would store the destination path in the > > inode directly. I

Re: Use uiomove() in if_tun and if_pppx

2016-01-23 Thread Martin Natano
For what it's worth, your diff reads fine to me. It took me a while to realize, that tlen in pppxwrite() and tun_dev_write() can't overflow because of the range check beforehand, but on close look that code also turns out to be correct. cheers, natano On Fri, Jan 22, 2016 at 07:29:54PM +0100,

Re: Firefox, malloc(3) and threads

2016-01-23 Thread Martin Natano
Yes! This absolutely makes Youtube videos watchable for me (on a Thinkpad T520). There still is occassional stuttering, but _far_ less disruptive than before. Another usecase where I see improvements is reloading a resource-heavy web page while switching tabs. Before applying the patch, this

bktr uiomove() conversion

2016-01-24 Thread Martin Natano
Below the conversion from uiomovei() to uiomove() for bktr. The patch also replaces two occurrences of uio->uio_iov->iov_len with uio->uio_resid. I don't see a reason why bktr should inspect iov_len directly. Index: dev/pci/bktr/bktr_core.c

udf uiomove() conversion

2016-01-20 Thread Martin Natano
Below the conversion to uiomove() for isofs/udf/. Note that converting size to size_t is not possible in udf_read(), as udf_readatoffset() requires a pointer to an integer variable. Changing that would cause a lot of code churn, so i chose to truncate uio_resid to INT_MAX instead.

vmctl create nit-picks

2016-01-19 Thread Martin Natano
Hi, below some comments regarding the vmctl disk image creation code. Index: main.c === RCS file: /cvs/src/usr.sbin/vmctl/main.c,v retrieving revision 1.13 diff -u -p -u -r1.13 main.c --- main.c 5 Jan 2016 16:25:34 -

  1   2   3   >