Re: [RFC][PATCH 04/20] pspace: Allow multiple instaces of the process id namespace

[ebiederm@xmission.com (Eric W. Biederman)]

William Lee Irwin III <wli@holomorphy.com> writes:

> On Mon, Feb 06, 2006 at 12:34:08PM -0700, Eric W. Biederman wrote:
>> -#define mk_pid(map, off)	(((map) - pidmap_array)*BITS_PER_PAGE + (off))
>> +#define mk_pid(map, off) (((map) - pspace->pidmap)*BITS_PER_PAGE + (off))
>>  #define find_next_offset(map, off)					\
>
> mk_pid() should be:
>
> #define mk_pid(pspace, map, off)	\
> 			(((map) - (pspace)->pidmap)*BITS_PER_PAGE + (off))
>
> or otherwise made an inline; pscape escaping like this shouldn't happen.

Agreed that is a bug.

> On Mon, Feb 06, 2006 at 12:34:08PM -0700, Eric W. Biederman wrote:
>> +static struct pspace *new_pspace(struct task_struct *leader)
>> +{
>> +	struct pspace *pspace, *parent;
>> +	int i;
>> +	size_t len;
>> +	parent = leader->pspace;
>> +	len = strlen(parent->name) + 10;
>> +	pspace = kzalloc(sizeof(struct pspace) + len, GFP_KERNEL);
>> +	if (!pspace)
>> +		return NULL;
>> +	atomic_set(&pspace->count, 1);
>> +	pspace->flags        = 0;
>> +	pspace->nr_threads   = 0;
>> +	pspace->nr_processes = 0;
>> +	pspace->last_pid     = 0;
>> +	pspace->min          = RESERVED_PIDS;
>> +	pspace->max          = PID_MAX_DEFAULT;
>> +	for (i = 0; i < PIDMAP_ENTRIES; i++) {
>> +		atomic_set(&pspace->pidmap[i].nr_free,  BITS_PER_PAGE);
>> +		pspace->pidmap[i].page = NULL;
>> +	}
>> +	attach_any_pid(&pspace->child_reaper, leader, PIDTYPE_PID, 
>> +			parent, leader->wid);
>> +	leader->pspace->nr_processes++;
>> +	snprintf(pspace->name, len + 1, "%s/%d", parent->name, leader->wid);
>> +
>> +	return pspace;
>> +}
>
> kzalloc() followed by zeroing ->flags et al by hand is redundant.

Totally agreed.  I was explicitly initializing all of the fields
then I missed one in an update and got bit rather badly.  So I went
for the shotgun approach.  I should probably just make it kmalloc
again at this point.   I think kmalloc plus explicitly initializing
the fields is going to be a more readable and more efficient because
it removes the redundancy.

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