From: Serge E. Hallyn <[email protected]>

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 <[email protected]>
---
 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);
 }
 
-/*
- * __ckpt_generate_fmt - generate standard checkpoint error message
- * @ctx: checkpoint context
- * @fmt0: c/r-format string
- * @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 @fmt0 by subtituting format snippets
- * according to the contents of @fmt0.  The format characters in
- * @fmt0 can be E (error), O (objref), P (pointer), S (string) and
- * V (variable/symbol). For example, E will generate a "err %d" in
- * PREFMT (see prefmt_array below).
- *
- * If @fmt0 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
- * 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
- * the T key), e.g.:
- *
- *   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.
- */
-static char *__ckpt_generate_fmt(struct ckpt_ctx *ctx, char *fmt0, char *fmt)
-{
-       static int warn_notask = 0;
-       static int warn_prefmt = 0;
-       char *format;
-       int i, j, len = 0;
-
-       static struct {
-               char key;
-               char *fmt;
-       } prefmt_array[] = {
-               { 'E', "err %d" },
-               { 'O', "obj %d" },
-               { 'P', "ptr %p" },
-               { 'V', "sym %pS" },
-               { 'S', "str %s" },
-               { 0, "??? %pS" },
-       };
-
-       /*
-        * 17 for "pid %d" (plus space)
-        * 21 for "tsk %s" (tsk->comm)
-        * up to 8 per varfmt entry
-        */
-       format = kzalloc(37 + 8 * strlen(fmt0) + strlen(fmt), GFP_KERNEL);
-       if (!format)
-               return NULL;
-
-       format[len++] = '[';
-
-       if (fmt0[0] == 'T') {
-               if (ctx->tsk)
-                       len = sprintf(format, "pid %d tsk %s ",
-                                     task_pid_vnr(ctx->tsk), ctx->tsk->comm);
-               else if (warn_notask++ < 5)
-                       printk(KERN_ERR "c/r: no target task set\n");
-               fmt0++;
-       }
-
-       for (i = 0; i < strlen(fmt0); i++) {
-               for (j = 0; prefmt_array[j].key; j++)
-                       if (prefmt_array[j].key == fmt0[i])
-                               break;
-               if (!prefmt_array[j].key && warn_prefmt++ < 5)
-                       printk(KERN_ERR "c/r: unknown prefmt %c\n", fmt0[i]);
-               len += sprintf(&format[len], "%s ", prefmt_array[j].fmt);
-       }
-
-       if (len > 1)
-               sprintf(&format[len-1], "]: %s", fmt);  /* erase last space */
-       else
-               sprintf(format, "%s", fmt);
-
-       return format;
-}
-
-/* see _ckpt_generate_fmt for information on @fmt0 */
-static void __ckpt_generate_err(struct ckpt_ctx *ctx, char *fmt0,
-                               char *fmt, va_list ap)
+/* see ckpt_generate_fmt for information on @fmt extensions */
+static void __ckpt_generate_err(struct ckpt_ctx *ctx, char *fmt, va_list ap)
 {
        va_list aq;
        char *format;
        char *str;
        int len;
 
-       format = __ckpt_generate_fmt(ctx, fmt0, fmt);
+       format = ckpt_generate_fmt(ctx, fmt);
        va_copy(aq, ap);
 
        /*
@@ -223,15 +134,14 @@ static void __ckpt_generate_err(struct ckpt_ctx *ctx, 
char *fmt0,
  * @fmt: message format
  * @...: arguments
  *
- * See _ckpt_generate_fmt for information on @fmt0.
  * Use this during checkpoint to report while holding a spinlock
  */
-void __ckpt_write_err(struct ckpt_ctx *ctx, char *fmt0, char *fmt, ...)
+void __ckpt_write_err(struct ckpt_ctx *ctx, char *fmt, ...)
 {
        va_list ap;
 
        va_start(ap, fmt);
-       __ckpt_generate_err(ctx, fmt0, fmt, ap);
+       __ckpt_generate_err(ctx, fmt, ap);
        va_end(ap);
 }
 
@@ -242,10 +152,9 @@ void __ckpt_write_err(struct ckpt_ctx *ctx, char *fmt0, 
char *fmt, ...)
  * @fmt: error string format
  * @...: error string arguments
  *
- * See _ckpt_generate_fmt for information on @fmt0.
  * If @fmt is null, the string in the ctx->err_string will be used (and freed)
  */
-int ckpt_write_err(struct ckpt_ctx *ctx, char *fmt0, char *fmt, ...)
+int ckpt_write_err(struct ckpt_ctx *ctx, char *fmt, ...)
 {
        va_list ap;
        char *str;
@@ -253,7 +162,7 @@ int ckpt_write_err(struct ckpt_ctx *ctx, char *fmt0, char 
*fmt, ...)
 
        if (fmt) {
                va_start(ap, fmt);
-               __ckpt_generate_err(ctx, fmt0, fmt, ap);
+               __ckpt_generate_err(ctx,  fmt, ap);
                va_end(ap);
        }
 
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);
 }
 
+/*
+ * 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
+ * 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
+ * the T key), e.g.:
+ *
+ *   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.
+ *
+ * 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);
+       format = kzalloc(alloclen, GFP_KERNEL);
+       if (!format)
+               return NULL;
+
+       for (; *fmt; fmt++) {
+               BUG_ON(len > alloclen);
+               if (*fmt != '%' || fmt[1] != '(' || fmt[3] != ')') {
+                       format[len++] = *fmt;
+                       continue;
+               }
+               if (!first)
+                       format[len++] = ' ';
+               else
+                       first = 0;
+               switch (fmt[2]) {
+               case 'E':
+                       len += sprintf(format+len, "[%s]", "err %d");
+                       break;
+               case 'O':
+                       len += sprintf(format+len, "[%s]", "obj %d");
+                       break;
+               case 'P':
+                       len += sprintf(format+len, "[%s]", "ptr %p");
+                       break;
+               case 'V':
+                       len += sprintf(format+len, "[%s]", "sym %pS");
+                       break;
+               case 'S':
+                       len += sprintf(format+len, "[%s]", "str %s");
+                       break;
+               case 'T':
+                       if (ctx->tsk)
+                               len += sprintf(format+len, "[pid %d tsk %s]",
+                                     task_pid_vnr(ctx->tsk), ctx->tsk->comm);
+                       else
+                               len += sprintf(format+len, "[pid -1 tsk NULL]");
+                       break;
+               default:
+                       printk(KERN_ERR "c/r: bad format specifier %c\n",
+                                       fmt[2]);
+                       BUG();
+               }
+
+               fmt += 3;
+       }
+       format[len] = '\0';
+
+       return format;
+}
 
 /**
  * sys_checkpoint - checkpoint a container
diff --git a/include/linux/checkpoint.h b/include/linux/checkpoint.h
index dfcb59b..8a1eaa7 100644
--- a/include/linux/checkpoint.h
+++ b/include/linux/checkpoint.h
@@ -70,12 +70,13 @@ extern int ckpt_write_buffer(struct ckpt_ctx *ctx, void 
*ptr, int len);
 extern int ckpt_write_string(struct ckpt_ctx *ctx, char *str, int len);
 
 /*
- * Generate a checkpoint error message with unified format, of the
- * form: "[PREFMT]: @fmt", where PREFMT is constructed from @fmt0. See
- * checkpoint/checkpoint.c:__ckpt_generate_fmt() for details.
+ * Generate a checkpoint error message with unified format.  Format
+ * can include tokens like %(T) for checkpoint-specific arguments,
+ * which must come before non-checkpoint-specific ones.
+ * See checkpoint/checkpoint.c:__ckpt_generate_fmt() for details.
  */
-extern void __ckpt_write_err(struct ckpt_ctx *ctx, char *fmt0, char *fmt, ...);
-extern int ckpt_write_err(struct ckpt_ctx *ctx, char *fmt0, char *fmt, ...);
+extern void __ckpt_write_err(struct ckpt_ctx *ctx, char *fmt, ...);
+extern int ckpt_write_err(struct ckpt_ctx *ctx, char *fmt, ...);
 
 extern int _ckpt_read_obj_type(struct ckpt_ctx *ctx,
                               void *ptr, int len, int type);
@@ -370,6 +371,8 @@ static inline void restore_debug_free(struct ckpt_ctx *ctx) 
{}
 
 #endif /* CONFIG_CHECKPOINT_DEBUG */
 
+extern char *ckpt_generate_fmt(struct ckpt_ctx *ctx, char *fmt);
+
 #endif /* CONFIG_CHECKPOINT */
 #endif /* __KERNEL__ */
 
-- 
1.6.1

_______________________________________________
Containers mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/containers

_______________________________________________
Devel mailing list
[email protected]
https://openvz.org/mailman/listinfo/devel

Reply via email to