Re: [PATCH] task name handling in proc fs

[Andrew Morton]

Mike Kravetz <kravetz@xxxxxxxxxx> wrote:
>
> On Thu, Jul 01, 2004 at 03:19:35PM -0700, Andrew Morton wrote:
> > Mike Kravetz <kravetz@xxxxxxxxxx> wrote:
> > >
> > > --- linux-2.6.7/fs/proc/array.c	Wed Jun 16 05:19:36 2004
> > > +++ linux-2.6.7.ptest/fs/proc/array.c	Thu Jul  1 17:44:14 2004
> > > @@ -97,14 +97,14 @@
> > >  		name++;
> > >  		i--;
> > >  		*buf = c;
> > > -		if (!c)
> > > +		if (!*buf)
> > >  			break;
> > > -		if (c == '\\') {
> > > -			buf[1] = c;
> > > +		if (*buf == '\\') {
> > > +			buf[1] = *buf;
> > >  			buf += 2;
> > >  			continue;
> > >  		}
> > > -		if (c == '\n') {
> > > +		if (*buf == '\n') {
> > >  			buf[0] = '\\';
> > >  			buf[1] = 'n';
> > >  			buf += 2;
> > 
> > What is this code for?
> 
> The code is copying the task name from 'c' to 'buf' one character
> at a time.  It is then 'post processing' the characters.  Currently,
> the post processing is based on the value of c which is part of the
> source string (task->curr).  However, it is possible for the source
> string to change during this copy (think exec).  In such a case I
> think it is better to base the 'post processing' on the character
> that already has been safely been copied to the target string rather
> than the character in the source string which might have changed.

But this just makes it a bit less racy than it used to be.

If we need locking around task->comm (and it seems that we do) then
let's do it.

void get_task_comm(char *buf, struct task_struct *tsk);
void set_task_comm(struct task_struct *tsk, char *buf);

It would be a bit lame to add a new lock for this - probably
task_lock() could be coopted.
-
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/