Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 10 Feb 2015 10:15:38 +0200
From:      Konstantin Belousov <kostikbel@gmail.com>
To:        Rui Paulo <rpaulo@me.com>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Rui Paulo <rpaulo@FreeBSD.org>
Subject:   Re: svn commit: r278479 - in head: etc sys/kern
Message-ID:  <20150210081538.GK42409@kib.kiev.ua>
In-Reply-To: <A4743616-E491-4E08-AF75-564EB613AD86@me.com>
References:  <201502092313.t19NDpoS083043@svn.freebsd.org> <20150209232826.GJ42409@kib.kiev.ua> <A4743616-E491-4E08-AF75-564EB613AD86@me.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Feb 09, 2015 at 06:35:55PM -0800, Rui Paulo wrote:
> On Feb 9, 2015, at 15:28, Konstantin Belousov <kostikbel@gmail.com> wrote:
> > Arguably, there should be a knob, probably sysctl, to turn the
> > functionality off. I definitely do not want this on crash boxes used for
> > userspace debugging.  Even despite the example handler is inactive.
> 
> OK, I can provide a sysctl knob.
Seen that, thanks.

> 
> >> +	len = MAXPATHLEN * 2 + 5 /* comm= */ + 5 /* core= */ + 1;
> > It is much cleaner to use static const char arrays for the names,
> > and use sizeof() - 1 instead of hard-coding commented constants.
> 
> OK.  I was trying to avoid allocating >2k on the stack.
Probably I was not clear enough.  I do not suggest to change data allocation
from malloc to automatic.  I mean
static const char comm_name[] = "comm=";
and then use sizeof(comm_name) - 1 and comm_name instead of string literal.

> 
> >> +	data = malloc(len, M_TEMP, M_NOWAIT);
> > Why is this allocation M_NOWAIT ?
> 
> That should be M_WAITOK.
> 
> >> +		freepath = NULL;
> >> +	}
> >> +	if (vn_fullpath_global(td, vp, &fullpath, &freepath) != 0)
> >> +		goto out;
> >> +	snprintf(data, len, "%s core=%s", data, fullpath);
> > This is weird, and highly depends on the implementation details, supplying
> > the same string as target and source.  IMO strcat(9) is enough there.
> 
> OK, I'll change it to strcat.
> 
> --
> Rui Paulo
> 
> 



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150210081538.GK42409>