On Tue, Jul 05, 2011 at 12:08:26PM -0400, Ted Unangst wrote:
> On Tue, Jul 05, 2011, Otto Moerbeek wrote:
> > Hi,
> >
> > this adds decoding of some interesting structs to kdump. Mostly from
> > FreeBSD, but adapted to our tree.
>
> very cool, just a one comment below.
>
> > error = copyout(mtod(m, caddr_t), SCARG(uap, asa), len);
> > - if (error == 0)
> > + if (error == 0) {
> > +#ifdef KTRACE
> > + if (KTRPOINT(p, KTR_STRUCT))
> > + ktrsockaddr(p, mtod(m, caddr_t), len);
> > +#endif
> > error = copyout(&len, SCARG(uap, alen), sizeof (len));
>
> > error = copyout(&sb, SCARG(uap, ub), sizeof(sb));
> > +#ifdef KTRACE
> > + if (error == 0 && KTRPOINT(p, KTR_STRUCT))
> > + ktrstat(p, &sb);
> > +#endif
>
> the new ktrace points are being added before, after, and between
> existing copyout calls. Can we make that consistent? Probably best to
> do it only if copyout succeeds.
My original idea was to add the trace point after the copyout that
copies the struct. But indeed, it might be better to do it only if
both (if there are two) succeed.
-Otto
