Re: taskqueue_create() name parameter lieftime

on 16/11/2010 15:27 John Baldwin said the following:
> On Tuesday, November 16, 2010 7:20:47 am Andriy Gapon wrote:
>>
>> taskqueue_create() documentation never explicitly says this, but current
>> taskqueue_create() implementation just stores a 'name' pointer parameter
>> internally.  Thus it depends on the 'name' having a life time encompassing that of
>> the taskqueue.
>> I think that alternatively we could have copied the name (or a portion of it) into
>> an internal buffer.
>> I don't any argument for either approach, just curious which one looks more
>> preferable from general (FreeBSD, kernel) programming practices point of view.
> 
> Hmm, in many other places we store a separate copy (e.g. all the interrupt
> code uses separate MAXCOMLEN char arrays to hold names).  If that is easy to
> do, that is probably the best approach.

BTW, tq_name doesn't seem to be used anywhere at all.
Perhaps just drop it?  But still could be useful in a debugger, though.

Anyway, here is a possible change:
diff --git a/sys/kern/subr_taskqueue.c b/sys/kern/subr_taskqueue.c
index 49ddce2..9b8ae66 100644
--- a/sys/kern/subr_taskqueue.c
+++ b/sys/kern/subr_taskqueue.c
_at__at_ -53,7 +53,6 _at__at_ struct taskqueue_busy {

 struct taskqueue {
 	STAILQ_HEAD(, task)	tq_queue;
-	const char		*tq_name;
 	taskqueue_enqueue_fn	tq_enqueue;
 	void			*tq_context;
 	TAILQ_HEAD(, taskqueue_busy) tq_active;
_at__at_ -62,6 +61,7 _at__at_ struct taskqueue {
 	int			tq_tcount;
 	int			tq_spin;
 	int			tq_flags;
+	char			tq_name[MAXCOMLEN];
 };

 #define	TQ_FLAGS_ACTIVE		(1 << 0)
_at__at_ -106,7 +106,7 _at__at_ _taskqueue_create(const char *name, int mflags,

 	STAILQ_INIT(&queue->tq_queue);
 	TAILQ_INIT(&queue->tq_active);
-	queue->tq_name = name;
+	strlcpy(queue->tq_name, name, sizeof(queue->tq_name));
 	queue->tq_enqueue = enqueue;
 	queue->tq_context = context;
 	queue->tq_spin = (mtxflags & MTX_SPIN) != 0;


-- 
Andriy Gapon