Messages in this thread | | | Date | Wed, 10 Jan 2024 16:52:53 -0500 | From | Rafael Aquini <> | Subject | Re: [PATCH v2] workqueue.c: Increase workqueue name length |
| |
On Wed, Jan 10, 2024 at 09:47:56PM +0100, Rasmus Villemoes wrote: > On 10/01/2024 21.29, Audra Mitchell wrote: > > > @@ -4663,9 +4663,10 @@ struct workqueue_struct *alloc_workqueue(const char *fmt, > > unsigned int flags, > > int max_active, ...) > > { > > - va_list args; > > + va_list args, args_copy; > > struct workqueue_struct *wq; > > struct pool_workqueue *pwq; > > + int len; > > > > /* > > * Unbound && max_active == 1 used to imply ordered, which is no longer > > @@ -4692,6 +4693,13 @@ struct workqueue_struct *alloc_workqueue(const char *fmt, > > } > > > > va_start(args, max_active); > > + va_copy(args_copy, args); > > + len = vsnprintf(NULL, 0, fmt, args_copy); > > + WARN(len > WQ_NAME_LEN, > > + "workqueue: wq->name too long (%d). Truncated to WQ_NAME_LEN (%d)\n", > > + len, WQ_NAME_LEN); > > + > > + va_end(args_copy); > > vsnprintf(wq->name, sizeof(wq->name), fmt, args); > > Eh, why not just _not_ throw away the return value from the existing > vsnprintf() and do "len >= sizeof(wq->name)" to know if truncation > happened? There's really no need need to do vsnprintf() twice. (And yes, > you want >=, not >). >
The extra vsnprintf call is required because the return of the existing vsnprintf() is going to be already capped by sizeof(wq->name).
> Oh, and definitely not WARN, pr_warn() or pr_warn_once() please. >
Then you lose the ability to figure out what was trying to create the wq with the inflated name. Also, the _once variants don't seem to do good here, because alloc_workqueue() can be called by different drivers.
-- Rafael
| |