[PATCH v1] libtraceevent: Avoid a simple asprintf case

[PATCH v1] libtraceevent: Avoid a simple asprintf case

[Ian Rogers]

Avoid an asprintf for the single character TEP_EVENT_NEWLINE and
TEP_EVENT_DELIM case.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 src/event-parse.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/event-parse.c b/src/event-parse.c
index b6ae67e..16fdb46 100644
--- a/src/event-parse.c
+++ b/src/event-parse.c
@@ -1232,9 +1232,11 @@ static enum tep_event_type __read_token(struct tep_handle *tep, char **tok)
 	switch (type) {
 	case TEP_EVENT_NEWLINE:
 	case TEP_EVENT_DELIM:
-		if (asprintf(tok, "%c", ch) < 0)
+		*tok = malloc(2);
+		if (!*tok)
 			return TEP_EVENT_ERROR;
-
+		(*tok)[0] = ch;
+		(*tok)[1] = '\0';
 		return type;
 
 	case TEP_EVENT_OP:
-- 
2.45.0.rc1.225.g2a3ae87e7f-goog

Re: [PATCH v1] libtraceevent: Avoid a simple asprintf case

[Steven Rostedt]

On Wed,  8 May 2024 22:13:17 -0700
Ian Rogers <irogers@google.com> wrote:

> Avoid an asprintf for the single character TEP_EVENT_NEWLINE and
> TEP_EVENT_DELIM case.

This states what the patch does, but omits why. What's wrong with
asprintf() here?

-- Steve

Re: [PATCH v1] libtraceevent: Avoid a simple asprintf case

[Ian Rogers]

On Thu, May 16, 2024 at 3:03 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Wed,  8 May 2024 22:13:17 -0700
> Ian Rogers <irogers@google.com> wrote:
>
> > Avoid an asprintf for the single character TEP_EVENT_NEWLINE and
> > TEP_EVENT_DELIM case.
>
> This states what the patch does, but omits why. What's wrong with
> asprintf() here?

Nothing except overhead. I was tracking down an asprintf issue [1],
asprintf breaking the sanitizer stack traces on my debian based linux,
and using asprintf for the sake of copying a character was creating a
huge amount of noise for me. 2 lines-of-code extra means this change
is by one measure more complex but it removes special knowledge of
asprintf return values and the meaning of %c, so on the other hand it
is less complex. I won't be offended if it's not accepted.

Thanks,
Ian

[1] https://lore.kernel.org/lkml/20240509052015.1914670-1-irogers@google.com/

> -- Steve

Re: [PATCH v1] libtraceevent: Avoid a simple asprintf case

[Steven Rostedt]

On Thu, 16 May 2024 15:09:40 -0700
Ian Rogers <irogers@google.com> wrote:

> > This states what the patch does, but omits why. What's wrong with
> > asprintf() here?  
> 
> Nothing except overhead. I was tracking down an asprintf issue [1],
> asprintf breaking the sanitizer stack traces on my debian based linux,
> and using asprintf for the sake of copying a character was creating a
> huge amount of noise for me. 2 lines-of-code extra means this change
> is by one measure more complex but it removes special knowledge of
> asprintf return values and the meaning of %c, so on the other hand it
> is less complex. I won't be offended if it's not accepted.

I don't mind taking the patch, but the above description is needed in
the change log, as that explains the purpose of the patch.

You don't need to resend, I'll just cut and paste what you wrote above
to the commit message.

Thanks,

-- Steve

Re: [PATCH v1] libtraceevent: Avoid a simple asprintf case

[Ian Rogers]

On Thu, May 16, 2024 at 4:51 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Thu, 16 May 2024 15:09:40 -0700
> Ian Rogers <irogers@google.com> wrote:
>
> > > This states what the patch does, but omits why. What's wrong with
> > > asprintf() here?
> >
> > Nothing except overhead. I was tracking down an asprintf issue [1],
> > asprintf breaking the sanitizer stack traces on my debian based linux,
> > and using asprintf for the sake of copying a character was creating a
> > huge amount of noise for me. 2 lines-of-code extra means this change
> > is by one measure more complex but it removes special knowledge of
> > asprintf return values and the meaning of %c, so on the other hand it
> > is less complex. I won't be offended if it's not accepted.
>
> I don't mind taking the patch, but the above description is needed in
> the change log, as that explains the purpose of the patch.
>
> You don't need to resend, I'll just cut and paste what you wrote above
> to the commit message.

Thanks, the commit message LGTM.

Ian

> Thanks,
>
> -- Steve