[PATCH 1/3] powerpc: bare minimum checkpoint/restart implementation
Serge E. Hallyn
serue at us.ibm.com
Fri Jan 30 15:01:30 EST 2009
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
More information about the Linuxppc-dev
mailing list