From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751384AbbD2Wsk (ORCPT ); Wed, 29 Apr 2015 18:48:40 -0400 Received: from mail.linuxfoundation.org ([140.211.169.12]:59424 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751140AbbD2Wsj (ORCPT ); Wed, 29 Apr 2015 18:48:39 -0400 Date: Wed, 29 Apr 2015 15:48:37 -0700 From: Andrew Morton To: Sudeep Holla Cc: linux-kernel@vger.kernel.org, Tejun Heo , Pawel Moll , "Peter Zijlstra (Intel)" Subject: Re: [PATCH v2] bitmap: remove explicit newline handling using scnprintf format string Message-Id: <20150429154837.d9bb4415f05418496da22b19@linux-foundation.org> In-Reply-To: <1430235401-30403-1-git-send-email-sudeep.holla@arm.com> References: <1430128018-14667-1-git-send-email-sudeep.holla@arm.com> <1430235401-30403-1-git-send-email-sudeep.holla@arm.com> X-Mailer: Sylpheed 3.4.1 (GTK+ 2.24.23; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 28 Apr 2015 16:36:41 +0100 Sudeep Holla wrote: > bitmap_print_to_pagebuf uses scnprintf to copy the cpumask/list to page > buffer. It handles the newline and trailing null character explicitly. bitmap_print_to_pagebuf() is horrid :( That automagic "assume caller passed us an offset into a PAGE_SIZE area". > It's unnecessary and also partially duplicated as scnprintf already adds > trailing null character. The newline can be passed through format string > to scnprintf. This patch does that simplification. > > However theoritically there's one behavior difference: when the buffer > is too small, the original code would still output '\n' at the end while > the new code(with this patch) would just continue to print the formatted > string. Since this function is dealing with only page buffers, it's > highly unlikely to hit that corner case. > > This patch will help in auditing the users of bitmap_print_to_pagebuf > to verify that the buffer passed is large enough and get rid of it > completely by replacing them with direct scnprintf() Yes, bitmap_print_to_pagebuf() should be eliminated. Make the callers keep track of how much room they have in their buffer. > --- a/lib/bitmap.c > +++ b/lib/bitmap.c > @@ -462,19 +462,18 @@ EXPORT_SYMBOL(bitmap_parse_user); > * Output format is a comma-separated list of decimal numbers and > * ranges if list is specified or hex digits grouped into comma-separated > * sets of 8 digits/set. Returns the number of characters written to buf. > + * It is assumed that the buffer passed is atleast PAGE_SIZE and easily > + * accommodate nmaskbits. Well kind-of. How does this look? --- a/lib/bitmap.c~bitmap-remove-explicit-newline-handling-using-scnprintf-format-string-fix +++ a/lib/bitmap.c @@ -462,8 +462,10 @@ EXPORT_SYMBOL(bitmap_parse_user); * Output format is a comma-separated list of decimal numbers and * ranges if list is specified or hex digits grouped into comma-separated * sets of 8 digits/set. Returns the number of characters written to buf. - * It is assumed that the buffer passed is atleast PAGE_SIZE and easily - * accommodate nmaskbits. + * + * It is assumed that @buf is a pointer into a PAGE_SIZE area and that + * sufficient storage remains at @buf to accommodate the + * bitmap_print_to_pagebuf() output. */ int bitmap_print_to_pagebuf(bool list, char *buf, const unsigned long *maskp, int nmaskbits)