serue@xxxxxxxxxx wrote:
> From: Serge E. Hallyn <serue@xxxxxxxxxx>
>
> Matt Helsley originally suggested this to avoid having two
> format strings. This is not bisect-safe and therefore not
> even compile-tested. Every call to ckpt_write_err must be
> updated to use a single format.
>
> Changelog:
> Oct 29: merge with the next patch, moving ckpt_generate_fmt()
> into checkpoing/sys.c and making it non-static so that
> we can use it in ckpt_error().
> Oct 28: also fix comment above ckpt_generate_fmt()
> Oct 27: ensure %(T) has a closing paren
>
> Signed-off-by: Serge E. Hallyn <serue@xxxxxxxxxx>
> ---
> checkpoint/checkpoint.c | 105 +++-----------------------------------------
> checkpoint/sys.c | 95 +++++++++++++++++++++++++++++++++++++++
> include/linux/checkpoint.h | 13 +++--
> 3 files changed, 110 insertions(+), 103 deletions(-)
>
> diff --git a/checkpoint/checkpoint.c b/checkpoint/checkpoint.c
> index 6eb8f3b..c6be4f9 100644
> --- a/checkpoint/checkpoint.c
> +++ b/checkpoint/checkpoint.c
> @@ -96,104 +96,15 @@ int ckpt_write_string(struct ckpt_ctx *ctx, char *str, int len)
> return ckpt_write_obj_type(ctx, str, len, CKPT_HDR_STRING);
> }
>
[...]
> diff --git a/checkpoint/sys.c b/checkpoint/sys.c
> index 260a1ee..9b2de18 100644
> --- a/checkpoint/sys.c
> +++ b/checkpoint/sys.c
> @@ -339,6 +339,101 @@ int walk_task_subtree(struct task_struct *root,
> return (ret < 0 ? ret : total);
> }
>
Some pieces of the comment below are obsolete...
> +/*
> + * ckpt_generate_fmt - generate standard checkpoint error message
> + * @ctx: checkpoint context
> + * @fmt: message format
> + *
> + * This generates a unified format of checkpoint error messages, to
> + * ease (after the failure) inspection by userspace tools. It converts
> + * the (printf) message @fmt into a new format: "[PREFMT]: fmt".
> + *
> + * PREFMT is constructed from @fmt by subtituting format snippets
> + * according to the contents of @fmt. The format characters in
> + * @fmt can be %(E) (error), %(O) (objref), %(P) (pointer), %(S) (string),
> + * %(C) (bytes read/written out of checkpoint image so far), * and %(V)
^^^^^
> + * (variable/symbol). For example, %(E) will generate a "err %d"
> + * in PREFMT.
^^^^^^^^^
> + *
> + * If @fmt begins with %(T), PREFMT will begin with "pid %d tsk %s"
^^^^^^^^^
> + * with the pid and the tsk->comm of the currently checkpointed task.
> + * The latter is taken from ctx->tsk, and is it the responsbilility of
typo ^^^^^^^^^^^^^^^^^
> + * the caller to have a valid pointer there (in particular, functions
> + * that iterate on the processes: collect_objects, checkpoint_task,
> + * and tree_count_tasks).
> + *
> + * The caller of ckpt_write_err() and _ckpt_write_err() must provide
^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^
> + * the additional variabes, in order, to match the @fmt0 (except for
typo ^^^^^^^^ ^^^^^^^
> + * the T key), e.g.:
^^^^^^^^^^ (obsolete)
> + *
> + * ckpt_writ_err(ctx, "TEO", "FILE flags %d", err, objref, flags)
^^^^^^^^
> + *
> + * Here, T is simply passed, E expects an integer (err), O expects an
> + * integer (objref), and the last argument matches the format string.
Rewrite example...
> + *
> + * XXX Do we want 'T' and 'C' to simply always be prepended?
> + */
> +char *ckpt_generate_fmt(struct ckpt_ctx *ctx, char *fmt)
> +{
> + char *format;
> + int alloclen, len = 0;
> + int first = 1;
> +
> + /*
> + * 17 for "pid %d" (plus space)
> + * 21 for "tsk %s" (tsk->comm)
> + * up to 8 per varfmt entry
^^^^^^^^
> + */
> + alloclen = 37 + 8 * strlen(fmt);
This calculation assumed that @fmt had only format string...
At the very minimum you could take strlen(fmt)/3 (+1 to round up)
I thought you were going to use a @ctx->buffer or something ?
> + format = kzalloc(alloclen, GFP_KERNEL);
> + if (!format)
> + return NULL;
> +
> + for (; *fmt; fmt++) {
> + BUG_ON(len > alloclen);
> + if (*fmt != '%' || fmt[1] != '(' || fmt[3] != ')') {
This is still a bit risky .. how about adding
|| fmt[2] == '\0'
between the 2nd and 3rd test ?
> + format[len++] = *fmt;
> + continue;
> + }
> + if (!first)
> + format[len++] = ' ';
> + else
> + first = 0;
[...]
Oren.
_______________________________________________
Containers mailing list
Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/containers
