Re: [PATCH 2/2] c/r: define s390-specific checkpoint-restart code (v5)

[Dave Hansen <dave-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>]

On Tue, 2009-02-24 at 13:37 -0600, Serge E. Hallyn wrote:
> 
> > +/* dump the cpu state and registers of a given task */
> > +int cr_write_cpu(struct cr_ctx *ctx, struct task_struct *t)
> > +{
> > +     struct cr_hdr h;
> > +     struct cr_hdr_cpu *hh = cr_hbuf_get(ctx, sizeof(*hh));
> > +     int ret;
> > +
> > +     h.type = CR_HDR_CPU;
> > +     h.len = sizeof(*hh);
> > +     h.parent = task_pid_vnr(t);
> 
> BTW - I know Dave mentioened using a generic helper for this
> often-used stanza above, but I continue to be against that
> bc the helper ends up having to take a bunch of eye-numbing
> arguments and I think the code ends up hard to read.  But
> maybe you can think of a way to make that clearer...

Yeah, that's true.  The plethora of types also makes it hard to do with
a function.  For plain readability you can't beat what we already have
there.

But, I do still wonder why we need the .parent member in the cr_hdr
itself.  Shouldn't that be something that gets pushed down to where we
can actually describe it, like in the cr_hdr_foo structures?

-- Dave