Date: Fri, 19 Nov 2010 18:20:04 +0200 From: Andriy Gapon <avg@freebsd.org> To: John Baldwin <jhb@freebsd.org> Cc: freebsd-current@freebsd.org, Pawel Jakub Dawidek <pjd@freebsd.org> Subject: Re: taskqueue_create() name parameter lieftime Message-ID: <4CE6A3B4.2080604@freebsd.org> In-Reply-To: <201011160827.11628.jhb@freebsd.org> References: <4CE2771F.8020109@freebsd.org> <201011160827.11628.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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 @@ -53,7 +53,6 @@ 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; @@ -62,6 +61,7 @@ struct taskqueue { int tq_tcount; int tq_spin; int tq_flags; + char tq_name[MAXCOMLEN]; }; #define TQ_FLAGS_ACTIVE (1 << 0) @@ -106,7 +106,7 @@ _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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4CE6A3B4.2080604>