From owner-freebsd-current@FreeBSD.ORG Fri Nov 19 16:20:07 2010 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id D2D831065670; Fri, 19 Nov 2010 16:20:07 +0000 (UTC) (envelope-from avg@freebsd.org) Received: from citadel.icyb.net.ua (citadel.icyb.net.ua [212.40.38.140]) by mx1.freebsd.org (Postfix) with ESMTP id 7999A8FC20; Fri, 19 Nov 2010 16:20:06 +0000 (UTC) Received: from odyssey.starpoint.kiev.ua (alpha-e.starpoint.kiev.ua [212.40.38.101]) by citadel.icyb.net.ua (8.8.8p3/ICyb-2.3exp) with ESMTP id SAA15507; Fri, 19 Nov 2010 18:20:04 +0200 (EET) (envelope-from avg@freebsd.org) Message-ID: <4CE6A3B4.2080604@freebsd.org> Date: Fri, 19 Nov 2010 18:20:04 +0200 From: Andriy Gapon User-Agent: Mozilla/5.0 (X11; U; FreeBSD amd64; en-US; rv:1.9.2.12) Gecko/20101029 Lightning/1.0b2 Thunderbird/3.1.6 MIME-Version: 1.0 To: John Baldwin References: <4CE2771F.8020109@freebsd.org> <201011160827.11628.jhb@freebsd.org> In-Reply-To: <201011160827.11628.jhb@freebsd.org> X-Enigmail-Version: 1.1.2 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: freebsd-current@freebsd.org, Pawel Jakub Dawidek Subject: Re: taskqueue_create() name parameter lieftime X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 19 Nov 2010 16:20:07 -0000 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