lkml.org 
[lkml]   [2019]   [May]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v2 03/17] kunit: test: add string_stream a std::stream like string builder
    From
    Date
    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 <brendanhiggins@google.com>
    > ---
    > 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 <brendanhiggins@google.com>
    > + */
    > +
    > +#ifndef _KUNIT_STRING_STREAM_H
    > +#define _KUNIT_STRING_STREAM_H
    > +
    > +#include <linux/types.h>
    > +#include <linux/spinlock.h>
    > +#include <linux/kref.h>
    > +#include <stdarg.h>
    > +
    > +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 <brendanhiggins@google.com>
    > + */
    > +
    > +#include <linux/list.h>
    > +#include <linux/slab.h>
    > +#include <kunit/string-stream.h>
    > +
    > +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

    \
     
     \ /
      Last update: 2019-05-03 03:27    [W:2.843 / U:0.092 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site