Re: RFC [patch 00/34] PID Virtualization Overview

[Hubertus Franke]

Serge E. Hallyn wrote:
> Quoting Eric W. Biederman (ebiederm@xxxxxxxxxxxx):
>
> > Arjan van de Ven <arjan@xxxxxxxxxxxxx> writes:
> >
> >
> > > On Wed, 2006-01-18 at 22:54 +0000, Alan Cox wrote:
> > >
> > > > On Mer, 2006-01-18 at 11:38 -0800, Dave Hansen wrote:
> > > >
> > > > > But, it seems that many drivers like to print out pids as a unique
> > > > > identifier for the task.  Should we just let them print those
> > > > > potentially non-unique identifiers, deprecate and kill them, or provide
> > > > > a replacement with something else which is truly unique?
> > > >
> > > > Pick a format for container number + pid and document/stick with it -
> > > > something like container::pid (eg 0::114) or 114[0] whatever so long as
> > > > it is consistent
> > >
> > > having a pid_to_string(<task struct>) or maybe task_to_string() thing
> > > for convenient printing of pids/tasks.. I'm all for that. Means you can
> > > even configure how verbose you want it to be (include ->comm or not,
> > > ->state maybe etc)
> >
> > The only way I can see to sanely do this is to pass it the temporary
> > buffer it writes it's contents into.
> > Something like:
> > printk(KERN_XXX "%s\n", task_to_string(buf, tsk)); ?
>
>
> That's kind of neat :)
>
> The only other thing I can think of is to do something like
>
> #define task_str(tsk) tsk->container_id, tsk->pid
> or
> #define task_str(tsk) tsk->container_id, ":", tsk->pid
>
> and have it be used as
>
> printk(KERN_XXX "%s::%s\n", task_str(tsk));
> or
> printk(KERN_XXX "%s%s%s\n", task_str(tsk));
>
> The only reason I point it out is that we don't risk memory corruption
> if the printk caller forgets to give the extra '%s's, like we do if
> the caller forgets they need char buf[PID_CONTAINER_MAXLENGTH] instead
> of 'char *buf;' or 'char buf;'.
>
> -serge

As odd as this looks .. it does have the benefits and anything that avoids
potential problems.

On the other hand you might run into problems with the following.

		char *str = task_str(tsk);

Eitherway .. I don't think these are the big fish to fry now :-)

-- Hubertus

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/