[Devel] Re: [RFC v14][PATCH 04/54] General infrastructure for checkpoint restart

[Serge E. Hallyn]

Quoting Oren Laadan (orenl at cs.columbia.edu):
> 
> 
> Serge E. Hallyn wrote:
> > Quoting Oren Laadan (orenl at cs.columbia.edu):
> > ...
> >> +static int checkpoint_write_header(struct ckpt_ctx *ctx)
> >> +{
> >> +	struct ckpt_hdr_header *h;
> >> +	struct new_utsname *uts;
> >> +	struct timeval ktv;
> >> +	int ret;
> >> +
> >> +	h = ckpt_hdr_get_type(ctx, sizeof(*h), CKPT_HDR_HEADER);
> > 
> > ...
> >> +	struct ckpt_hdr_tail *h;
> >> +	int ret;
> >> +
> >> +	h = ckpt_hdr_get_type(ctx, sizeof(*h), CKPT_HDR_TAIL);
> > 
> > ...
> >> +	struct ckpt_hdr_task *h;
> >> +	int ret;
> >> +
> >> +	h = ckpt_hdr_get_type(ctx, sizeof(*h), CKPT_HDR_TASK);
> > 
> > ...
> >> +/**
> >> + * ckpt_hdr_get_type - get a hdr of certain size
> >> + * @ctx: checkpoint context
> >> + * @len: number of bytes to reserve
> >> + *
> >> + * Returns pointer to reserved space on hbuf
> >> + */
> >> +void *ckpt_hdr_get_type(struct ckpt_ctx *ctx, int len, int type)
> >> +{
> > 
> > Observation (based on all callers in later patches as well): the second
> > argument appears to be superfluous?  You should be able to determine
> > based on type.
> 
> Not always: for instance, ckpt_hdr_file_xxxx all use CKPT_HDR_FILE,
> but have different sizes (all share a common 'struct ckpt_hdr_file',
> but actual payload differs).
> 
> (Besides, that would require adding a big table to decide the length
> based on a type... which I don't really like).

But only in one place, instead of having 'sizeof(*h)' at every
caller.

But ok, if they differ in the file case then nm.

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