[Devel] Re: [PATCH 1/3] powerpc: bare minimum checkpoint/restart implementation

[Serge E. Hallyn]

Quoting Oren Laadan (orenl at cs.columbia.edu):
> > +static void cr_hdr_init(struct cr_hdr *hdr, __s16 type, __s16 len, __u32 parent)
> > +{
> > +	hdr->type = type;
> > +	hdr->len = len;
> > +	hdr->parent = parent;
> > +}
> > +
> 
> This function is rather generic and useful to non-arch-dependent and other
> architectures code. Perhaps put in a separate patch ?

BTW I disagree here - looking through the patch I found
the use of this fn to be very distracting.  To replace 3
simple in-line assignments, I have to verify the order of
4 weird-looking arguments, and actually first try to
remember what 'cr_hdr_init' is supposed to be and do?

That's not meant to be a complaint, just explanation :)

I prefer dropping its use altogether.  Since I believe
most of the functions calling it in this patch shouldn't
exist anyway (just writing 0xdeadbeef into the file), it
all the more should just go away.

-serge
_______________________________________________
Containers mailing list
Containers at lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers