On Tue, Jul 05, 2011 at 06:17:53PM +0200, Otto Moerbeek wrote:
> 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
Played a bit with that, but it becomes messy, because it is not always
obvious the mbuf is initalized. Putting the ktrace call immediately
after the corresponding copyout at least guarantees that the mbuf is
filled.
For things like connect you want the struct even if the conect fails.
See e.g. the example I posted.
-Otto
