| Message ID | 20230814132309.32641-5-rf@opensource.cirrus.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | a5abe7b201779b0000f1e8ab522e5c6fc0c413bf |
| Delegated to: | Brendan Higgins |
| Headers | show |
| Series | kunit: Add dynamically-extending log | expand |
On Mon, 14 Aug 2023 at 21:23, Richard Fitzgerald <rf@opensource.cirrus.com> wrote: > > Add an optional feature to string_stream that will append a newline to > any added string that does not already end with a newline. The purpose > of this is so that string_stream can be used to collect log lines. > > This is enabled/disabled by calling string_stream_set_append_newlines(). > > Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com> > --- Looks good. I don't mind the extra 'wasted' byte if a message already ends in a newline. Reviewed-by: David Gow <davidgow@google.com> > lib/kunit/string-stream.c | 28 +++++++++++++++++++++------- > lib/kunit/string-stream.h | 7 +++++++ > 2 files changed, 28 insertions(+), 7 deletions(-) > > diff --git a/lib/kunit/string-stream.c b/lib/kunit/string-stream.c > index ed24d86af9f5..1dcf6513b692 100644 > --- a/lib/kunit/string-stream.c > +++ b/lib/kunit/string-stream.c > @@ -44,32 +44,46 @@ int string_stream_vadd(struct string_stream *stream, > va_list args) > { > struct string_stream_fragment *frag_container; > - int len; > + int buf_len, result_len; > va_list args_for_counting; > > /* Make a copy because `vsnprintf` could change it */ > va_copy(args_for_counting, args); > > /* Evaluate length of formatted string */ > - len = vsnprintf(NULL, 0, fmt, args_for_counting); > + buf_len = vsnprintf(NULL, 0, fmt, args_for_counting); > > va_end(args_for_counting); > > - if (len == 0) > + if (buf_len == 0) > return 0; > > + /* Reserve one extra for possible appended newline. */ > + if (stream->append_newlines) > + buf_len++; > + > /* Need space for null byte. */ > - len++; > + buf_len++; > > frag_container = alloc_string_stream_fragment(stream->test, > - len, > + buf_len, > stream->gfp); > if (IS_ERR(frag_container)) > return PTR_ERR(frag_container); > > - len = vsnprintf(frag_container->fragment, len, fmt, args); > + if (stream->append_newlines) { > + /* Don't include reserved newline byte in writeable length. */ > + result_len = vsnprintf(frag_container->fragment, buf_len - 1, fmt, args); > + > + /* Append newline if necessary. */ > + if (frag_container->fragment[result_len - 1] != '\n') > + result_len = strlcat(frag_container->fragment, "\n", buf_len); > + } else { > + result_len = vsnprintf(frag_container->fragment, buf_len, fmt, args); > + } > + > spin_lock(&stream->lock); > - stream->length += len; > + stream->length += result_len; > list_add_tail(&frag_container->node, &stream->fragments); > spin_unlock(&stream->lock); > > diff --git a/lib/kunit/string-stream.h b/lib/kunit/string-stream.h > index b669f9a75a94..048930bf97f0 100644 > --- a/lib/kunit/string-stream.h > +++ b/lib/kunit/string-stream.h > @@ -25,6 +25,7 @@ struct string_stream { > spinlock_t lock; > struct kunit *test; > gfp_t gfp; > + bool append_newlines; > }; > > struct kunit; > @@ -47,4 +48,10 @@ bool string_stream_is_empty(struct string_stream *stream); > > void string_stream_destroy(struct string_stream *stream); > > +static inline void string_stream_set_append_newlines(struct string_stream *stream, > + bool append_newlines) > +{ > + stream->append_newlines = append_newlines; > +} > + > #endif /* _KUNIT_STRING_STREAM_H */ > -- > 2.30.2 >
On Mon, Aug 14, 2023 at 9:23 AM Richard Fitzgerald <rf@opensource.cirrus.com> wrote: > > Add an optional feature to string_stream that will append a newline to > any added string that does not already end with a newline. The purpose > of this is so that string_stream can be used to collect log lines. > > This is enabled/disabled by calling string_stream_set_append_newlines(). > > Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com> Hello! I just wanted to look over handling the newline and this looks good to me. I agree with David that I don't mind one extra allocated byte. Reviewed-by: Rae Moar <rmoar@google.com> Thanks! -Rae > --- > lib/kunit/string-stream.c | 28 +++++++++++++++++++++------- > lib/kunit/string-stream.h | 7 +++++++ > 2 files changed, 28 insertions(+), 7 deletions(-) > > diff --git a/lib/kunit/string-stream.c b/lib/kunit/string-stream.c > index ed24d86af9f5..1dcf6513b692 100644 > --- a/lib/kunit/string-stream.c > +++ b/lib/kunit/string-stream.c > @@ -44,32 +44,46 @@ int string_stream_vadd(struct string_stream *stream, > va_list args) > { > struct string_stream_fragment *frag_container; > - int len; > + int buf_len, result_len; > va_list args_for_counting; > > /* Make a copy because `vsnprintf` could change it */ > va_copy(args_for_counting, args); > > /* Evaluate length of formatted string */ > - len = vsnprintf(NULL, 0, fmt, args_for_counting); > + buf_len = vsnprintf(NULL, 0, fmt, args_for_counting); > > va_end(args_for_counting); > > - if (len == 0) > + if (buf_len == 0) > return 0; > > + /* Reserve one extra for possible appended newline. */ > + if (stream->append_newlines) > + buf_len++; > + > /* Need space for null byte. */ > - len++; > + buf_len++; > > frag_container = alloc_string_stream_fragment(stream->test, > - len, > + buf_len, > stream->gfp); > if (IS_ERR(frag_container)) > return PTR_ERR(frag_container); > > - len = vsnprintf(frag_container->fragment, len, fmt, args); > + if (stream->append_newlines) { > + /* Don't include reserved newline byte in writeable length. */ > + result_len = vsnprintf(frag_container->fragment, buf_len - 1, fmt, args); > + > + /* Append newline if necessary. */ > + if (frag_container->fragment[result_len - 1] != '\n') > + result_len = strlcat(frag_container->fragment, "\n", buf_len); > + } else { > + result_len = vsnprintf(frag_container->fragment, buf_len, fmt, args); > + } > + > spin_lock(&stream->lock); > - stream->length += len; > + stream->length += result_len; > list_add_tail(&frag_container->node, &stream->fragments); > spin_unlock(&stream->lock); > > diff --git a/lib/kunit/string-stream.h b/lib/kunit/string-stream.h > index b669f9a75a94..048930bf97f0 100644 > --- a/lib/kunit/string-stream.h > +++ b/lib/kunit/string-stream.h > @@ -25,6 +25,7 @@ struct string_stream { > spinlock_t lock; > struct kunit *test; > gfp_t gfp; > + bool append_newlines; > }; > > struct kunit; > @@ -47,4 +48,10 @@ bool string_stream_is_empty(struct string_stream *stream); > > void string_stream_destroy(struct string_stream *stream); > > +static inline void string_stream_set_append_newlines(struct string_stream *stream, > + bool append_newlines) > +{ > + stream->append_newlines = append_newlines; > +} > + > #endif /* _KUNIT_STRING_STREAM_H */ > -- > 2.30.2 >
diff --git a/lib/kunit/string-stream.c b/lib/kunit/string-stream.c index ed24d86af9f5..1dcf6513b692 100644 --- a/lib/kunit/string-stream.c +++ b/lib/kunit/string-stream.c @@ -44,32 +44,46 @@ int string_stream_vadd(struct string_stream *stream, va_list args) { struct string_stream_fragment *frag_container; - int len; + int buf_len, result_len; va_list args_for_counting; /* Make a copy because `vsnprintf` could change it */ va_copy(args_for_counting, args); /* Evaluate length of formatted string */ - len = vsnprintf(NULL, 0, fmt, args_for_counting); + buf_len = vsnprintf(NULL, 0, fmt, args_for_counting); va_end(args_for_counting); - if (len == 0) + if (buf_len == 0) return 0; + /* Reserve one extra for possible appended newline. */ + if (stream->append_newlines) + buf_len++; + /* Need space for null byte. */ - len++; + buf_len++; frag_container = alloc_string_stream_fragment(stream->test, - len, + buf_len, stream->gfp); if (IS_ERR(frag_container)) return PTR_ERR(frag_container); - len = vsnprintf(frag_container->fragment, len, fmt, args); + if (stream->append_newlines) { + /* Don't include reserved newline byte in writeable length. */ + result_len = vsnprintf(frag_container->fragment, buf_len - 1, fmt, args); + + /* Append newline if necessary. */ + if (frag_container->fragment[result_len - 1] != '\n') + result_len = strlcat(frag_container->fragment, "\n", buf_len); + } else { + result_len = vsnprintf(frag_container->fragment, buf_len, fmt, args); + } + spin_lock(&stream->lock); - stream->length += len; + stream->length += result_len; list_add_tail(&frag_container->node, &stream->fragments); spin_unlock(&stream->lock); diff --git a/lib/kunit/string-stream.h b/lib/kunit/string-stream.h index b669f9a75a94..048930bf97f0 100644 --- a/lib/kunit/string-stream.h +++ b/lib/kunit/string-stream.h @@ -25,6 +25,7 @@ struct string_stream { spinlock_t lock; struct kunit *test; gfp_t gfp; + bool append_newlines; }; struct kunit; @@ -47,4 +48,10 @@ bool string_stream_is_empty(struct string_stream *stream); void string_stream_destroy(struct string_stream *stream); +static inline void string_stream_set_append_newlines(struct string_stream *stream, + bool append_newlines) +{ + stream->append_newlines = append_newlines; +} + #endif /* _KUNIT_STRING_STREAM_H */
Add an optional feature to string_stream that will append a newline to any added string that does not already end with a newline. The purpose of this is so that string_stream can be used to collect log lines. This is enabled/disabled by calling string_stream_set_append_newlines(). Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com> --- lib/kunit/string-stream.c | 28 +++++++++++++++++++++------- lib/kunit/string-stream.h | 7 +++++++ 2 files changed, 28 insertions(+), 7 deletions(-)