On Fri, Oct 30, 2009 at 06:00:26PM -0500, se...@us.ibm.com wrote: > From: Serge E. Hallyn <se...@us.ibm.com> > > Signed-off-by: Serge E. Hallyn <se...@us.ibm.com> > ---
<snip> > - * 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". > + * The special flags are surrounded by %() to help them visually stand > + * out. For instance, %(O) means an objref. The following special > + * flags are recognized: > + * E: error > + * O: objref > + * P: pointer > + * T: task > + * S: string > + * V: variable > * Something for the future: It might be good to have "F: File" as well. It may sometimes be useful to print out a file name instead of just the struct file pointer. It'd be useful for epoll, file checkpoint ops in general, and file-backed VMAs. <snip> > -char *ckpt_generate_fmt(struct ckpt_ctx *ctx, char *fmt0, char *fmt) > +void ckpt_generate_fmt(struct ckpt_ctx *ctx, char *fmt) > { > - char *format = ctx->fmt_buf; > - 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" }, > - }; > - > - format[len++] = '['; > - > - if (fmt0[0] == 'T') { > - if (ctx->tsk) > - len = sprintf(format, "pid %d tsk %s ", > + char *s = ctx->fmt_buf; > + int len = 0; > + int first = 1; > + > + for (; *fmt && len < CKPT_MSG_BUFSZ; fmt++) { > + if (*fmt != '%' || fmt[1] != '(' || fmt[2] == '\0' || > + fmt[3] != ')') { > + s[len++] = *fmt; > + continue; > + } > + if (!first) > + s[len++] = ' '; > + else > + first = 0; > + switch (fmt[2]) { > + case 'E': > + len += sprintf(s+len, "[%s]", "err %d"); Why not use snprintf ? Maybe I'm misunderstanding the length check in the for-loop but it doesn't look sufficient to avoid overrunning the fmt_buf. Suppose len == CKPT_MSG_BUFSZ - 1 and fmt[2] is non-NUL. Then we're writing 8-9 more non-NUL bytes to fmt_buf ("[err %d]" in this case). You could avoid snprintf() by doing: for (; *fmt && len < (CKPT_MSG_BUFSZ - 9); fmt++) { which won't allow you to completely fill fmt_buf but will ensure you never run off the end. But I still think snprintf may be necessary to handle the default case (below). > + break; > + case 'O': > + len += sprintf(s+len, "[%s]", "obj %d"); > + break; > + case 'P': > + len += sprintf(s+len, "[%s]", "ptr %p"); > + break; > + case 'V': > + len += sprintf(s+len, "[%s]", "sym %pS"); > + break; > + case 'S': > + len += sprintf(s+len, "[%s]", "str %s"); > + break; > + case 'T': > + if (ctx->tsk) > + len += sprintf(s+len, "[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++; > - } > + else > + len += sprintf(s+len, "[pid -1 tsk NULL]"); > + break; > + default: > + printk(KERN_ERR "c/r: bad format specifier %c\n", > + fmt[2]); > + BUG(); Perhaps BUG() isn't such a good idea since this will be used in error-recovery paths. The printk looks fine. Perhaps just skip to the ')': while (*fmt && *fmt != ')') fmt++; The problem is this breaks walking the valist because an argument has been provided which won't be consumed. To fix this seems necessary to insert strings indicating which arguments to use with each subsequent conversion specifier: arg_num = 1; /* valist index starts at one. see man snprintf */ for (...) { ... case 'S': len += snprintf(s+len, CKPT_MSG_BUFSZ - len, "[str %%*%d$s]", arg_num); /* sample output:[str %*3$s]" */ arg_num++; break; ... default: printk(KERN_ERR "c/r: bad format specifier %c\n", fmt[2]); while (*fmt && *fmt != ')') fmt++; arg_num++; break; which all but requires the use snprintf instead of sprintf. Cheers, -Matt Helsley _______________________________________________ Containers mailing list contain...@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers _______________________________________________ Devel mailing list Devel@openvz.org https://openvz.org/mailman/listinfo/devel