svn commit: r278479 - in head: etc sys/kern

[Konstantin Belousov]

On Mon, Feb 09, 2015 at 06:35:55PM -0800, Rui Paulo wrote:
> On Feb 9, 2015, at 15:28, Konstantin Belousov <kostikbel at 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
> 
>