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 <zanu...@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
