From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.1 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS, T_DKIMWL_WL_HIGH autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id AB7F4C43219 for ; Fri, 3 May 2019 01:26:41 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 6DE1C2133F for ; Fri, 3 May 2019 01:26:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1556846801; bh=ezIzjSzXH0XYlnG8MDnPY/5HL3hONlOSfYSFuod7JP4=; h=Subject:To:Cc:References:From:Date:In-Reply-To:List-ID:From; b=FqBFq6gUHKDnKNm/Eg2gpwWItx82v2e7L0S+5uj2eyAFVwwEoe8N6RjoOj8+kEUFZ yyXI5Xf0eCLvuWrxNLNeS4HC3yo7EU9JDuDRS8fX45v8l0+Nuzapsfn31u7aXVm5qe /R1Fs5xx4pUSLEqFENu5IxL7E+QZD+baUScaZzXI= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726616AbfECB0k (ORCPT ); Thu, 2 May 2019 21:26:40 -0400 Received: from mail.kernel.org ([198.145.29.99]:37554 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726327AbfECB0k (ORCPT ); Thu, 2 May 2019 21:26:40 -0400 Received: from [192.168.1.112] (c-24-9-64-241.hsd1.co.comcast.net [24.9.64.241]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id BD7F22075E; Fri, 3 May 2019 01:26:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1556846798; bh=ezIzjSzXH0XYlnG8MDnPY/5HL3hONlOSfYSFuod7JP4=; h=Subject:To:Cc:References:From:Date:In-Reply-To:From; b=dSKrZ0M3+KMtPQkDdumpJNz+AO6E8QIThgPYAO13XHWWwflaoRbIz/oNqlv7qoGTe VXl92EIa/rUgQudCogxHlCVDc61J3iy5Cvqxwo79kJh2ROhBpjJ7BSMqTUevaRDp6y lAlz7wW77WAmzYXCoP5yBl+76exX90pmOHxKFBS8= Subject: Re: [PATCH v2 03/17] kunit: test: add string_stream a std::stream like string builder To: Brendan Higgins , frowand.list@gmail.com, gregkh@linuxfoundation.org, keescook@google.com, kieran.bingham@ideasonboard.com, mcgrof@kernel.org, robh@kernel.org, sboyd@kernel.org Cc: devicetree@vger.kernel.org, dri-devel@lists.freedesktop.org, kunit-dev@googlegroups.com, linux-doc@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-nvdimm@lists.01.org, linux-um@lists.infradead.org, Alexander.Levin@microsoft.com, Tim.Bird@sony.com, amir73il@gmail.com, dan.carpenter@oracle.com, dan.j.williams@intel.com, daniel@ffwll.ch, jdike@addtoit.com, joel@jms.id.au, julia.lawall@lip6.fr, khilman@baylibre.com, knut.omang@oracle.com, logang@deltatee.com, mpe@ellerman.id.au, pmladek@suse.com, richard@nod.at, rientjes@google.com, rostedt@goodmis.org, wfg@linux.intel.com, skhan@linuxfoundation.org References: <20190501230126.229218-1-brendanhiggins@google.com> <20190501230126.229218-4-brendanhiggins@google.com> From: shuah Message-ID: <1befe456-d981-d726-44f9-ebe3702ee51d@kernel.org> Date: Thu, 2 May 2019 19:26:35 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <20190501230126.229218-4-brendanhiggins@google.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org On 5/1/19 5:01 PM, Brendan Higgins wrote: > A number of test features need to do pretty complicated string printing > where it may not be possible to rely on a single preallocated string > with parameters. > > So provide a library for constructing the string as you go similar to > C++'s std::string. > > Signed-off-by: Brendan Higgins > --- > include/kunit/string-stream.h | 51 ++++++++++++ > kunit/Makefile | 3 +- > kunit/string-stream.c | 144 ++++++++++++++++++++++++++++++++++ > 3 files changed, 197 insertions(+), 1 deletion(-) > create mode 100644 include/kunit/string-stream.h > create mode 100644 kunit/string-stream.c > > diff --git a/include/kunit/string-stream.h b/include/kunit/string-stream.h > new file mode 100644 > index 0000000000000..567a4629406da > --- /dev/null > +++ b/include/kunit/string-stream.h > @@ -0,0 +1,51 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * C++ stream style string builder used in KUnit for building messages. > + * > + * Copyright (C) 2019, Google LLC. > + * Author: Brendan Higgins > + */ > + > +#ifndef _KUNIT_STRING_STREAM_H > +#define _KUNIT_STRING_STREAM_H > + > +#include > +#include > +#include > +#include > + > +struct string_stream_fragment { > + struct list_head node; > + char *fragment; > +}; > + > +struct string_stream { > + size_t length; > + struct list_head fragments; > + > + /* length and fragments are protected by this lock */ > + spinlock_t lock; > + struct kref refcount; > +}; > + > +struct string_stream *new_string_stream(void); > + > +void destroy_string_stream(struct string_stream *stream); > + > +void string_stream_get(struct string_stream *stream); > + > +int string_stream_put(struct string_stream *stream); > + > +int string_stream_add(struct string_stream *this, const char *fmt, ...); > + > +int string_stream_vadd(struct string_stream *this, > + const char *fmt, > + va_list args); > + > +char *string_stream_get_string(struct string_stream *this); > + > +void string_stream_clear(struct string_stream *this); > + > +bool string_stream_is_empty(struct string_stream *this); > + > +#endif /* _KUNIT_STRING_STREAM_H */ > diff --git a/kunit/Makefile b/kunit/Makefile > index 5efdc4dea2c08..275b565a0e81f 100644 > --- a/kunit/Makefile > +++ b/kunit/Makefile > @@ -1 +1,2 @@ > -obj-$(CONFIG_KUNIT) += test.o > +obj-$(CONFIG_KUNIT) += test.o \ > + string-stream.o > diff --git a/kunit/string-stream.c b/kunit/string-stream.c > new file mode 100644 > index 0000000000000..7018194ecf2fa > --- /dev/null > +++ b/kunit/string-stream.c > @@ -0,0 +1,144 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * C++ stream style string builder used in KUnit for building messages. > + * > + * Copyright (C) 2019, Google LLC. > + * Author: Brendan Higgins > + */ > + > +#include > +#include > +#include > + > +int string_stream_vadd(struct string_stream *this, > + const char *fmt, > + va_list args) > +{ > + struct string_stream_fragment *fragment; Since there is field with the same name, please use a different name. Using the same name for the struct which contains a field of the same name get very confusing and will hard to maintain the code. > + int len; > + va_list args_for_counting; > + unsigned long flags; > + > + /* Make a copy because `vsnprintf` could change it */ > + va_copy(args_for_counting, args); > + > + /* Need space for null byte. */ > + len = vsnprintf(NULL, 0, fmt, args_for_counting) + 1; > + > + va_end(args_for_counting); > + > + fragment = kmalloc(sizeof(*fragment), GFP_KERNEL); > + if (!fragment) > + return -ENOMEM; > + > + fragment->fragment = kmalloc(len, GFP_KERNEL); This is confusing. See above comment. > + if (!fragment->fragment) { > + kfree(fragment); > + return -ENOMEM; > + } > + > + len = vsnprintf(fragment->fragment, len, fmt, args); > + spin_lock_irqsave(&this->lock, flags); > + this->length += len; > + list_add_tail(&fragment->node, &this->fragments); > + spin_unlock_irqrestore(&this->lock, flags); > + return 0; > +} > + > +int string_stream_add(struct string_stream *this, const char *fmt, ...) > +{ > + va_list args; > + int result; > + > + va_start(args, fmt); > + result = string_stream_vadd(this, fmt, args); > + va_end(args); > + return result; > +} > + > +void string_stream_clear(struct string_stream *this) > +{ > + struct string_stream_fragment *fragment, *fragment_safe; > + unsigned long flags; > + > + spin_lock_irqsave(&this->lock, flags); > + list_for_each_entry_safe(fragment, > + fragment_safe, > + &this->fragments, > + node) { > + list_del(&fragment->node); > + kfree(fragment->fragment); > + kfree(fragment); This is what git me down the road of checking the structure name to begin with. :) > + } > + this->length = 0; > + spin_unlock_irqrestore(&this->lock, flags); > +} > + > +char *string_stream_get_string(struct string_stream *this) > +{ > + struct string_stream_fragment *fragment; > + size_t buf_len = this->length + 1; /* +1 for null byte. */ > + char *buf; > + unsigned long flags; > + > + buf = kzalloc(buf_len, GFP_KERNEL); > + if (!buf) > + return NULL; > + > + spin_lock_irqsave(&this->lock, flags); > + list_for_each_entry(fragment, &this->fragments, node) > + strlcat(buf, fragment->fragment, buf_len); > + spin_unlock_irqrestore(&this->lock, flags); > + > + return buf; > +} > + > +bool string_stream_is_empty(struct string_stream *this) > +{ > + bool is_empty; > + unsigned long flags; > + > + spin_lock_irqsave(&this->lock, flags); > + is_empty = list_empty(&this->fragments); > + spin_unlock_irqrestore(&this->lock, flags); > + > + return is_empty; > +} > + > +void destroy_string_stream(struct string_stream *stream) > +{ > + string_stream_clear(stream); > + kfree(stream); > +} > + > +static void string_stream_destroy(struct kref *kref) > +{ > + struct string_stream *stream = container_of(kref, > + struct string_stream, > + refcount); > + destroy_string_stream(stream); > +} > + > +struct string_stream *new_string_stream(void) > +{ > + struct string_stream *stream = kzalloc(sizeof(*stream), GFP_KERNEL); > + > + if (!stream) > + return NULL; > + > + INIT_LIST_HEAD(&stream->fragments); > + spin_lock_init(&stream->lock); > + kref_init(&stream->refcount); > + return stream; > +} > + > +void string_stream_get(struct string_stream *stream) > +{ > + kref_get(&stream->refcount); > +} > + > +int string_stream_put(struct string_stream *stream) > +{ > + return kref_put(&stream->refcount, &string_stream_destroy); > +} > + > thanks, -- Shuah