csh: blkfree() usage

Hello, In csh(1) the function blkfree() behaves like free(3) and performs no action if its argument is NULL, so the caller can avoid checking. I lightly tested the following patch on i386 and amd64. In two places the pointer was copied to a temporary variable (v) before being passed to blkfree()

smtpd: smtp_client_state() error message

Hello, smtp_client_state() and smtp_client_response() both do switch (proto->state) ... but smtp_client_state() doesn't print the id of the bad state in the default case. This patch makes the fatalx() message the same for both. Does this look OK? - Michael Index: smtp_client.c

smtpd: malloc+strlcpy -> strndup

Hello, Replace a malloc+strlcpy with strndup in cmdline_symset(). Parameter s is a "keyname=value" string and sym is the "keyname" part. If s is "=value", sym will be an empty string. The patch doesn't change this behaviour although it might be undesirable to call symset() with an empty string.

icmp6_mtudisc_clone() tweak

Hello, The code executed on failure in icmp6_mtudisc_clone() is always the same so it can be declared in one place. Is anyone happy to OK this? - Michael Index: icmp6.c === RCS file: /cvs/src/sys/netinet6/icmp6.c,v retrieving

umidi(4) jack count

Hello, The umidi(4) driver has three different endpoint allocation functions, which are called by alloc_all_endpoints(). The initial jack count can be moved here because it is zero no matter which allocation function is used. Also when we eventually free the jacks the count can be reset. Does

umidi(4) alloc_all_endpoints_fixed_ep()

Hello, In the umidi(4) driver the function alloc_all_endpoints_fixed_ep() returns USBD_NOMEM if mallocarray() fails, but the return value is always USBD_INVAL when 'goto error' happens. OK? - Michael Index: umidi.c === RCS file:

Re: csh: blkfree() usage

ping. On Thu, Aug 30, 2018 at 12:20:37AM +0800, Michael Mikonos wrote: > Hello, > > In csh(1) the function blkfree() behaves like free(3) and > performs no action if its argument is NULL, so the caller > can avoid checking. > > I lightly tested the following patch on i38

Re: pfctl: introduce copy_sa()

On Mon, Sep 10, 2018 at 12:16:57PM +0200, Klemens Nanni wrote: > On Mon, Sep 10, 2018 at 11:59:55AM +0200, Klemens Nanni wrote: > > Small helper to put the dance around `af' into one single location. > Wrong/bad diff, please disregard. > I was just about to reply that the size in the memcpy()

Re: update tradcpp to 0.5.2

On Fri, Aug 31, 2018 at 12:57:14PM +1000, Jonathan Gray wrote: > update tradcpp to 0.5.2 > > release 0.5.2 (20160904) >- Fix typo in -U usage message, noticed by Joerg. >- Add a -debuglog option to send an execution trace to a file. > Intended to be used when debugging imake

Re: [PATCH] qcow2 disk format

On Thu, Aug 30, 2018 at 11:19:08PM -0700, Ori Bernstein wrote: > On Wed, 15 Aug 2018 23:02:16 -0700, Ori Bernstein wrote: > > > Updated style from feedback from off-list. Also added checks and > > erroring for incompatible extensions. > > One more update. This revision adds explicit definitions

csh: simplify strsave()

Hello, The function strsave() in csh(1) is practically strdup(3). The only difference is memory allocation failure results in calling the stderror() error handler, which will later exit. This patch makes the code (IMO) clearer by removing two loops. xmalloc() behaves the same as xreallocarray()

vmd: add some NULL checks after {c,m}alloc()

Hello, Sometimes vmd doesn't seem to check the result of malloc/calloc. I tried to preserve the existing behavour w.r.t. return values for the functions modified; some functions returned 1 on error while others return -1. Does this look correct? - Michael Index: loadfile_elf.c

Re: vmd: add some NULL checks after {c,m}alloc()

On Tue, Sep 11, 2018 at 03:36:49PM +0800, Michael Mikonos wrote: > Hello, > > Sometimes vmd doesn't seem to check the result of malloc/calloc. > I tried to preserve the existing behavour w.r.t. return values > for the functions modified; some functions returned 1 on error > whil

Re: vmd: add some NULL checks after {c,m}alloc()

On Tue, Sep 11, 2018 at 11:25:52AM -0700, Ori Bernstein wrote: > On Tue, 11 Sep 2018 15:36:49 +0800 > Michael Mikonos wrote: > > > Hello, > > > > Sometimes vmd doesn't seem to check the result of malloc/calloc. > > I tried to preserve the exist

Re: vmd: add some NULL checks after {c,m}alloc()

On Wed, Sep 12, 2018 at 12:13:35AM -0700, Ori Bernstein wrote: > On Tue, 11 Sep 2018 23:29:53 -0700, Ori Bernstein wrote: > > > static ssize_t > > @@ -362,8 +377,9 @@ qc2_close(void *p) > > struct qcdisk *disk; > > > > disk = p; > > - pwrite(disk->fd, disk->l1, disk->l1sz,

smtpd.conf manpage typo

OK? Index: smtpd.conf.5 === RCS file: /cvs/src/usr.sbin/smtpd/smtpd.conf.5,v retrieving revision 1.205 diff -u -p -u -r1.205 smtpd.conf.5 --- smtpd.conf.524 Sep 2018 16:14:34 - 1.205 +++ smtpd.conf.58 Oct

Re: yacc + unveil

Hello, Forwarding a newer patch that I came up with. This time unveil()s are done before pledge() so no subsequent pledge() is needed to remove the unveil promise. * temporary files are created & unlinked in /tmp, so unveil the directory * output_file_name is either -o PATH or the default of

parse.y: strndup() in cmdline_symset()

Hello, When I updated cmdline_symset() in parse.y in the following commit I missed src/sbin/{iked,ipsecctl,pfctl}/parse.y. OK to update them? https://marc.info/?l=openbsd-cvs=153631079505256=2 Index: iked/parse.y === RCS file:

Re: Qcow2: Clean up logging/error handling

On Tue, Oct 23, 2018 at 09:44:24PM -0700, Ori Bernstein wrote: > This patch turns most warnings into errors, and uses the > appropriate fatal/fatalx so that we don't print bogus error > strings. It also adds checks for unsupported refcount sizes > and writes that clobber the header. > > Ok?

Re: parse.y: strndup() in cmdline_symset()

ping? On Thu, Nov 01, 2018 at 04:14:53PM +0800, Michael Mikonos wrote: > Hello, > > When I updated cmdline_symset() in parse.y in the following commit > I missed src/sbin/{iked,ipsecctl,pfctl}/parse.y. OK to update them? > > https://marc.info/?l=openbsd-cvs=153631079505256=2 &

Re: installboot: double free

On Tue, Nov 06, 2018 at 10:20:34AM +0100, Otto Moerbeek wrote: > On Tue, Nov 06, 2018 at 04:35:05PM +0800, Michael Mikonos wrote: > > > Hello, > > > > In installboot's fileprefix() function r is the return value > > of realpath(). If snprintf() fails free(r) happen

installboot: double free

Hello, In installboot's fileprefix() function r is the return value of realpath(). If snprintf() fails free(r) happens twice--- the second time is at label "err". From what I see the behavior was introduced in util.c revision 1.12. Does this fix look OK? - Michael Index: util.c

installboot: explicit free() in bootstrap()

Hello, On hppa and landisk, bootstrap() is called from md_installboot(). md_installboot() is the last action before returning from main(), but bootstrap() can explicitly free the buffer it calloc'd (boot). I don't have access to hppa or landisk. Is someone able to check that this doesn't break

Re: csh: memory leak in setDolp()

BTW the leak happens when a pattern in variable modifier s/// is not found. $ set a="test" $ echo $a:s/badpattern// test Any objections if I commit this? On Thu, Sep 20, 2018 at 12:30:05PM +0800, Michael Mikonos wrote: > Hello, > > In setDolp() pointers cp and dp initiall

Re: smtpd: malloc+strlcpy -> strndup

On Sat, Sep 01, 2018 at 11:31:49PM +0200, Gilles Chehade wrote: > On Sat, Sep 01, 2018 at 09:20:59PM +0800, Michael Mikonos wrote: > > Hello, > > > > Replace a malloc+strlcpy with strndup in cmdline_symset(). > > Parameter s is a "keyname=value" str

Re: csh: blkfree() usage

I also tested this patch on landisk, but this was mainly as an exercise to try a recent snapshot under the gxemul emulator (v0.6.0). On Thu, Aug 30, 2018 at 12:20:37AM +0800, Michael Mikonos wrote: > Hello, > > In csh(1) the function blkfree() behaves like free(3) and > performs no a

Re: smtpd: malloc+strlcpy -> strndup

On Mon, Sep 03, 2018 at 02:24:49PM +0800, Michael Mikonos wrote: > On Sat, Sep 01, 2018 at 11:31:49PM +0200, Gilles Chehade wrote: > > On Sat, Sep 01, 2018 at 09:20:59PM +0800, Michael Mikonos wrote: > > > Hello, > > > > > > Replace a malloc+st

Re: csh: avoid using uninitialized stat buffer

On Wed, Sep 19, 2018 at 09:19:22AM -0600, Todd C. Miller wrote: > When getcwd() fails, the stat buffer 'swd' is used uninitialized > by the else clause. Since it is used in both clauses we should > perform the stat before the if(). > > However, fixing this causes 'cp' to be unitialized in some

csh: memory leak in setDolp()

Hello, In setDolp() pointers cp and dp initially point to the same copied string, but later dp can become NULL if Strstr() finds no match. The copied string is not freed in this case. NetBSD added this fix in their dol.c revision 1.23 (2006). OK? - Michael Index: dol.c

yacc + unveil

Hello, I haven't tried using unveil() before but yacc cleanly annotates all the files it needs in open_files(). The options -d -r -v each cause an extra file to be written. unveil() is only needed for the input file if not reading from stdin. Temporary files are always under /tmp because TMPDIR

unveil DPRINTF()

Hello, As done in other parts of the kernel, introduce DPRINTF() macro to unveil. I think this is worth doing because the code is slightly more readable. OK? - Michael Index: kern_unveil.c === RCS file:

unveil manpage tweak

This patch aligns unveil.2 with pledge.2, so the unveil manual will explicitly mention errno is set. OK? Index: unveil.2 === RCS file: /cvs/src/lib/libc/sys/unveil.2,v retrieving revision 1.9 diff -u -p -u -r1.9 unveil.2 ---

Re: yacc + unveil

On Mon, Sep 24, 2018 at 10:53:47PM -0600, Theo de Raadt wrote: > Ugh. A diff which doens't check error returns. Averting my gaze > is similar to "no way". Hope you have another quarter, because you > need to try again Oops... new coin inserted. I decided to create a fatal_perror() function

Re: yacc + unveil

On Tue, Sep 25, 2018 at 03:22:38PM +0100, Ricardo Mestre wrote: > This is an example of better to start at just hoisting the code that > opens the many fds and put them all inside open_files(). After that it's > just a matter of calling pledge("stdio") and we are done. > > Of course that after

unveil manpage tweak 2

Re-reading the unveil manual I found a typo which isn't flagged by a spell checker. Does anyone prefer just doing s/paths// though? Index: unveil.2 === RCS file: /cvs/src/lib/libc/sys/unveil.2,v retrieving revision 1.10 diff -u -p

csh: quoted string treated as a glob in switch

Hello, As an exercise I tried writing a csh script and I found strange behaviour in its parsing of switch statements. The following snippet causes a syntax error (switch: Missing ].): set c="[" switch ("$c") case "[": echo match breaksw endsw The call tree is main() -> process() ->

Re: yacc + unveil

On Tue, Sep 25, 2018 at 05:25:54PM +0200, Sebastien Marie wrote: > On Tue, Sep 25, 2018 at 11:15:43PM +0800, Michael Mikonos wrote: > > On Tue, Sep 25, 2018 at 03:22:38PM +0100, Ricardo Mestre wrote: > > > This is an example of better to start at just hoisting the code that &g

Re: yacc + unveil

On Tue, Sep 25, 2018 at 11:42:26PM +0800, Michael Mikonos wrote: > On Tue, Sep 25, 2018 at 05:25:54PM +0200, Sebastien Marie wrote: > > On Tue, Sep 25, 2018 at 11:15:43PM +0800, Michael Mikonos wrote: > > > On Tue, Sep 25, 2018 at 03:22:38PM +0100, Ricardo Mestre wrote: > >

Re: vmd: add some NULL checks after {c,m}alloc()

On Wed, Sep 12, 2018 at 11:33:11AM -0700, Ori Bernstein wrote: > On Wed, 12 Sep 2018 15:36:32 +0800 > Michael Mikonos wrote: > > > On Wed, Sep 12, 2018 at 12:13:35AM -0700, Ori Bernstein wrote: > > > On Tue, 11 Sep 2018 23:29:53 -0700, Ori Bernstein > > > wrot

Re: csh: simplify strsave()

On Sat, Sep 08, 2018 at 10:13:35AM +0200, Martijn van Duren wrote: > On 09/08/18 04:57, Michael Mikonos wrote: > > Hello, > > > > The function strsave() in csh(1) is practically strdup(3). > > The only difference is memory allocation failure results in > > cal

Re: csh: simplify strsave()

On Sat, Sep 15, 2018 at 06:16:42AM -0600, Todd C. Miller wrote: > On Sat, 15 Sep 2018 12:42:22 +0200, Martijn van Duren wrote: > > > While here, should we also remove any in favour of strchr? Only > > difference seems to be the return type (bool vs pointer). > > Note that any(NULL, ch) is safe

Re: csh: simplify strsave()

On Mon, Sep 17, 2018 at 09:06:16AM -0600, Todd C. Miller wrote: > On Mon, 17 Sep 2018 15:53:06 +0200, Martijn van Duren wrote: > > > It should be safe. There are 5 instances where any() isn't called with > > a string literal: > > Thanks for checking. OK millert@ for the diff. > > - todd Also

csh: remove macros xmalloc,xcalloc,xreallocarray

Hello, In csh(1) the functions Malloc(), Calloc() & Reallocarray() are always called via the macros (which don't really do anything). This patch renames the functions and deletes the macros. Does this look OK? - Michael Index: csh.h

Re: vmd: add some NULL checks after {c,m}alloc()

On Thu, Sep 13, 2018 at 10:08:16PM -0700, Ori Bernstein wrote: > On Thu, 13 Sep 2018 10:30:54 +0800, Michael Mikonos wrote: > > > Thanks for the explanation. Overall it seems better having the > > clean-up code localised in qc2_close() so I'm happy to OK the > > patch w

Re: vmd: add some NULL checks after {c,m}alloc()

On Thu, Sep 13, 2018 at 11:10:50PM -0700, Ori Bernstein wrote: > On Fri, 14 Sep 2018 13:50:40 +0800, Michael Mikonos wrote: > > > On Thu, Sep 13, 2018 at 10:08:16PM -0700, Ori Bernstein wrote: > > > On Thu, 13 Sep 2018 10:30:54 +0800, Michael Mikonos wrote: > > >

smtpctl.8 typo

Hello, My spell-checker found the following typo this morning. OK? - Michael Index: smtpctl.8 === RCS file: /cvs/src/usr.sbin/smtpd/smtpctl.8,v retrieving revision 1.63 diff -u -p -u -r1.63 smtpctl.8 --- smtpctl.8 14 May 2018

rip_usrreq() style tweak

Hello, Is it OK to merge the PRU_CONNECT2 case into the generic "not supported" case in {rip,rip6}_usrreq()? If anything this allows the reader to see all the unsupported requests in one place. - Michael Index: netinet/raw_ip.c

Re: rip_usrreq() style tweak

On Thu, Feb 21, 2019 at 04:58:08PM +0100, Jeremie Courreges-Anglas wrote: > On Thu, Feb 21 2019, Michael Mikonos wrote: > > Hello, > > > > Is it OK to merge the PRU_CONNECT2 case into the generic > > "not supported" case in {rip,rip6}_usrreq()? > > If

ansify flex

Hello, Upstream flex already updated function declarations to ANSI. The following patch applies this change to the in-tree version. Does it look OK? - Michael Index: ccl.c === RCS file: /cvs/src/usr.bin/lex/ccl.c,v retrieving

flex {c,m}alloc() checks

Hello, I noticed that flex is too trusting and assumes calloc/malloc will always succeed. Hopefully I caught all of them. I tried to follow the existing idiom of calling flexerror() and passing strings via the _() macro. OK? - Michael Index: dfa.c

Re: flex {c,m}alloc() checks

On Sun, Aug 25, 2019 at 02:58:47PM +0200, Otto Moerbeek wrote: > On Sun, Aug 25, 2019 at 08:32:04PM +0800, Michael Mikonos wrote: > > > Hello, > > > > I noticed that flex is too trusting and assumes > > calloc/malloc will always succeed. Hopefully I > > caugh

Re: flex {c,m}alloc() checks

> I'd say go for the x* solution, > > -Otto Sure. When I looked at this again there are also realloc() return value checks missing, so I added xcalloc(), xmalloc() and xrealloc(). An old (unused) function yy_flex_xmalloc() gets removed. When building this I checked the resulting .o files

assert() triggered in flex

Hello, I was wondering why fuzz input was sometimes crashing flex(1). Now that assert() is enabled in the build I am able to trigger it with the following input. $ cat test.l %{ hello %} %% ))) world $ /usr/bin/lex -d - < test.l assertion "_sf_top_ix > 0" failed: file "scanflags.c", line 55,

assert() missing in flex

Hello, When building flex on clang 8.0.1 (i386) I noticed that assert() expands to nothing. This happens because of a fallback declaration of assert() in flexdef.h when HAVE_ASSERT_H is not set. Instead of changing flexdef.h the following patch follows the existing pattern in Makefile of adding

Re: assert() missing in flex

On Mon, Sep 16, 2019 at 11:38:30AM +0200, Marc Espie wrote: > On Mon, Sep 16, 2019 at 05:07:21PM +0800, Michael Mikonos wrote: > > Hello, > > > > When building flex on clang 8.0.1 (i386) I noticed that assert() > > expands to nothing. This happens because of a fallbac

Re: assert() triggered in flex

On Tue, Sep 17, 2019 at 11:56:26PM +0800, Michael Mikonos wrote: > Hello, > > I was wondering why fuzz input was sometimes crashing flex(1). > Now that assert() is enabled in the build I am able to > trigger it with the following input. > > $ cat

Re: flex: dead code around flex_die()

Ping, in case anyone would like to comment. On Thu, Oct 31, 2019 at 08:55:46AM +0800, Michael Mikonos wrote: > Hello, > > The macro flex_die(), defined in flexdef.h, never returns. > Instead it does FLEX_EXIT() which longjmp()s back to flex_main() > before exiting. > The follo

flex: dead code around flex_die()

Hello, The macro flex_die(), defined in flexdef.h, never returns. Instead it does FLEX_EXIT() which longjmp()s back to flex_main() before exiting. The following patch removes some dead code, as statements after flex_die() are never reached. Does this look OK? - Michael Index: tables.c

flex HAVE_DECL___FUNC__

Hello, When reviewing flex's config.h I think we could enable HAVE_DECL___FUNC__. This allows the flex_die() macro to include __func__ in its output. For example: flex: fatal internal error at main.c:146 (flex_main): spooky error Does this look OK? - Michael Index: config.h