Re: [PATCH v2 4/7] tracing: Add synthetic event error logging

[Tom Zanussi <zanussi@kernel.org>]

Hi Steve,

On Mon, 2020-10-12 at 18:04 -0400, Steven Rostedt wrote:
> On Mon, 12 Oct 2020 15:18:06 -0500
> Tom Zanussi <zanussi@kernel.org> wrote:
> 
> > +static int cmdstr_append(char *buf, const char *str, int
> > *remaining)
> > +{
> > +	int len = strlen(str);
> > +
> > +	if (len + 1 >= *remaining) {
> > +		synth_err(SYNTH_ERR_CMD_TOO_LONG, 0);
> > +		return -EINVAL;
> > +	}
> > +
> > +	strcat(buf, str);
> > +	strcat(buf, " ");
> > +	*remaining -= len + 1;
> > +
> > +	return 0;
> > +}
> > +
> > +static int save_cmdstr(int argc, const char *name, const char
> > **argv)
> > +{
> > +	int i, ret, remaining = MAX_DYNEVENT_CMD_LEN;
> > +	char *buf;
> > +
> > +	synth_err_clear();
> > +
> > +	buf = kzalloc(MAX_DYNEVENT_CMD_LEN, GFP_KERNEL);
> > +	if (!buf)
> > +		return -ENOMEM;
> > +
> > +	ret = cmdstr_append(buf, name, &remaining);
> > +	if (ret) {
> > +		kfree(buf);
> > +		return ret;
> > +	}
> > +
> > +	for (i = 0; i < argc; i++) {
> > +		ret = cmdstr_append(buf, argv[i], &remaining);
> > +		if (ret) {
> > +			kfree(buf);
> > +			return ret;
> > +		}
> > +	}
> > +
> > +	last_cmd_set(buf);
> > +
> > +	kfree(buf);
> > +
> > +	return ret;
> > +}
> > +
> 
> Hmm, the above could easily be replaced with:
> 
> 	struct seq_buf s;
> 
> 	buf = kzalloc(MAX_DYNEVENT_CMD_LEN, GFP_KERNEL);
> 	if (!buf)
> 		return -ENOMEM;
> 
> 	seq_buf_init(&s, buf, MAX_DYNEVENT_CMD_LEN);
> 
> 	seq_buf_puts(&s, name);
> 
> 	for (i = 0; i < argc; i++) {
> 		seq_buf_putc(&s, ' ');
> 		seq_buf_puts(&s, argv[i]);
> 	}
> 
> 	if (!seq_buf_buffer_left(&s)) {
> 		synth_err(SYNTH_ERR_CMD_TOO_LONG, 0);
> 		kfree(buf);
> 		return -EINVAL;
> 	}
> 	buf[s.len] = 0;
> 	last_cmd_set(buf);
> 
> 	kfree(buf);
> 	return 0;
> 

Yeah, that makes sense, will change it.

Thanks,

Tom

> 
> -- Steve