From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from lindbergh.monkeyblade.net (lindbergh.monkeyblade.net [23.128.96.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5FA53381DC; Thu, 26 Oct 2023 20:20:27 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="BnCE+LEO" Received: from mgamail.intel.com (mgamail.intel.com [192.55.52.93]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id ECC5618A; Thu, 26 Oct 2023 13:20:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1698351625; x=1729887625; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=d9QFfqFoHIzmszUDwJhsesLvCTOCbL+5vMd7tK5kcmY=; b=BnCE+LEOLOcSwoBeKNmQubx5+Qm9mwakWzBQ25+QvERtD3kRQq0Yy0oE lQU6TaD8rmRiisarnIhTEoEznHth899zl9y7nqpN31b86emmaZGIaqM+D uCTr272F5fzr7R8q1YxKu5bW06AfC1EFRBzVPPPwE0JcSADkytoRB4j8E b/DJpBNhrZ5Z36tYpJgOBcWanfypyDSg5d/HsfsBGCd+kwSOEGsntEvjc XmVsZt2mV+zrrwl7CgQQrjKs4q9kr8m7qF51Y439Sgb3aRpFhCHGNWUe4 ek30wExogg0TP1nheLVq/ZL0wetYnlX3edmMisvUqmofL/3x6Vql4EU1s Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10875"; a="384845878" X-IronPort-AV: E=Sophos;i="6.03,254,1694761200"; d="scan'208";a="384845878" Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Oct 2023 13:20:24 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10875"; a="762964629" X-IronPort-AV: E=Sophos;i="6.03,254,1694761200"; d="scan'208";a="762964629" Received: from smile.fi.intel.com ([10.237.72.54]) by fmsmga007.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Oct 2023 13:20:19 -0700 Received: from andy by smile.fi.intel.com with local (Exim 4.97-RC3) (envelope-from ) id 1qw6pz-00000008xA2-2pRF; Thu, 26 Oct 2023 23:20:15 +0300 Date: Thu, 26 Oct 2023 23:20:15 +0300 From: Andy Shevchenko To: Kees Cook Cc: Steven Rostedt , "Matthew Wilcox (Oracle)" , Christoph Hellwig , Justin Stitt , Kent Overstreet , Petr Mladek , Rasmus Villemoes , Sergey Senozhatsky , Masami Hiramatsu , Greg Kroah-Hartman , Arnd Bergmann , Jonathan Corbet , Yun Zhou , Jacob Keller , Zhen Lei , linux-trace-kernel@vger.kernel.org, Yosry Ahmed , linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org Subject: Re: [PATCH v2] seq_buf: Introduce DECLARE_SEQ_BUF and seq_buf_str() Message-ID: References: <20231026194033.it.702-kees@kernel.org> Precedence: bulk X-Mailing-List: linux-trace-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20231026194033.it.702-kees@kernel.org> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo On Thu, Oct 26, 2023 at 12:40:37PM -0700, Kees Cook wrote: > Solve two ergonomic issues with struct seq_buf; > > 1) Too much boilerplate is required to initialize: > > struct seq_buf s; > char buf[32]; > > seq_buf_init(s, buf, sizeof(buf)); > > Instead, we can build this directly on the stack. Provide > DECLARE_SEQ_BUF() macro to do this: > > DECLARE_SEQ_BUF(s, 32); > > 2) %NUL termination is fragile and requires 2 steps to get a valid > C String (and is a layering violation exposing the "internals" of > seq_buf): > > seq_buf_terminate(s); > do_something(s->buffer); > > Instead, we can just return s->buffer direction after terminating it > in refactored seq_buf_terminate(), now known as seq_buf_str(): > > do_soemthing(seq_buf_str(s)); ... > +#define DECLARE_SEQ_BUF(NAME, SIZE) \ > + char __ ## NAME ## _buffer[SIZE] = ""; \ > + struct seq_buf NAME = { .buffer = &__ ## NAME ## _buffer, \ > + .size = SIZE } Hmm... Wouldn't be more readable to have it as #define DECLARE_SEQ_BUF(NAME, SIZE) \ char __ ## NAME ## _buffer[SIZE] = ""; \ struct seq_buf NAME = { \ .buffer = &__ ## NAME ## _buffer, \ .size = SIZE, \ } ? ... > +static inline char *seq_buf_str(struct seq_buf *s) > { > if (WARN_ON(s->size == 0)) > - return; > + return ""; I'm wondering why it's a problem to have an empty string? > if (seq_buf_buffer_left(s)) > s->buffer[s->len] = 0; > else > s->buffer[s->size - 1] = 0; > + > + return s->buffer; > } -- With Best Regards, Andy Shevchenko