[Devel] Re: [PATCH 04/22] Change to the new enhanced error string format

[Serge E. Hallyn]

Quoting Matt Helsley (matthltc at us.ibm.com):
> On Fri, Oct 30, 2009 at 06:00:26PM -0500, serue at us.ibm.com wrote:
> > From: Serge E. Hallyn <serue at us.ibm.com>
> > 
> > Signed-off-by: Serge E. Hallyn <serue at 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.

Sure...  As callers want it, we can add it - I also expect %A for
ctx->active_pid (which won't take an argument) to be added.

...

> > +	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 ?

Yup, thanks for keeping me honest - just switched my new patchset
to do that.

...

> > +		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

I disagree - this fmt is passed in by the kernel, so if we get a flag
we don't understand, then it is a bug in our c/r code.

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