Messages in this thread | | | From | Namhyung Kim <> | Date | Mon, 13 Jul 2020 12:20:59 +0900 | Subject | Re: [PATCH v10 05/15] perf evlist: implement control command handling functions |
| |
On Wed, Jul 8, 2020 at 4:50 PM Alexey Budankov <alexey.budankov@linux.intel.com> wrote: > > > Implement functions of initialization, finalization and processing > of control command messages coming from control file descriptors. > Allocate control file descriptor as descriptor at struct pollfd > object of evsel_list for atomic poll() operation. > > Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com> > --- [SNIP] > +static int evlist__ctlfd_recv(struct evlist *evlist, enum evlist_ctl_cmd *cmd, > + char *cmd_data, size_t data_size) > +{ > + int err; > + char c; > + size_t bytes_read = 0; > + > + memset(cmd_data, 0, data_size--);
I overlooked the '--' at the end and thought there might be buffer overflow.. Care to add a comment?
> + > + do { > + err = read(evlist->ctl_fd.fd, &c, 1);
Maybe I missed earlier discussion, but do we really want this 1 byte read in a loop?
Thanks Namhyung
> + if (err > 0) { > + if (c == '\n' || c == '\0') > + break; > + cmd_data[bytes_read++] = c; > + if (bytes_read == data_size) > + break; > + } else { > + if (err == -1) > + pr_err("Failed to read from ctlfd %d: %m\n", evlist->ctl_fd.fd); > + break; > + } > + } while (1); > + > + pr_debug("Message from ctl_fd: \"%s%s\"\n", cmd_data, > + bytes_read == data_size ? "" : c == '\n' ? "\\n" : "\\0"); > + > + if (err > 0) { > + if (!strncmp(cmd_data, EVLIST_CTL_CMD_ENABLE_TAG, > + (sizeof(EVLIST_CTL_CMD_ENABLE_TAG)-1))) { > + *cmd = EVLIST_CTL_CMD_ENABLE; > + } else if (!strncmp(cmd_data, EVLIST_CTL_CMD_DISABLE_TAG, > + (sizeof(EVLIST_CTL_CMD_DISABLE_TAG)-1))) { > + *cmd = EVLIST_CTL_CMD_DISABLE; > + } > + } > + > + return err; > +}
| |