On 22.07.2020 15:34, Arnaldo Carvalho de Melo wrote: > Em Wed, Jul 22, 2020 at 09:27:34AM -0300, Arnaldo Carvalho de Melo escreveu: >> Em Fri, Jul 17, 2020 at 10:01:33AM +0300, Alexey Budankov escreveu: >>> >>> 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 <[email protected]> >>> Acked-by: Jiri Olsa <[email protected]> >>> Acked-by: Namhyung Kim <[email protected]> >> >> I'm applying locally for testing, but can you answer what is the reason >> for that newline in the CMD_ACK_TAG? I tried looking at previous >> discussion but couldn't quickly find any reasoning for that >> >> - Arnaldo >> >>> +++ b/tools/perf/util/evlist.h >>> @@ -359,4 +359,22 @@ void perf_evlist__force_leader(struct evlist *evlist); >>> struct evsel *perf_evlist__reset_weak_group(struct evlist *evlist, >>> struct evsel *evsel, >>> bool close); >>> +#define EVLIST_CTL_CMD_ENABLE_TAG "enable" >>> +#define EVLIST_CTL_CMD_DISABLE_TAG "disable" >>> +#define EVLIST_CTL_CMD_ACK_TAG "ack\n" >> >> What for? That '\n'? > > Oh, I see, this is not a _command_, it is a _response_, and the sender > expects a newline, right?
Right. Shell commands can expect strings terminated by \n, e.g. 'read -u'. See sample bash script in 00/15 for example. > > So I think this needs to be marked clearly as: > > // Commands accepted by 'perf record' > > #define EVLIST_CTL_CMD_ENABLE_TAG "enable" > #define EVLIST_CTL_CMD_DISABLE_TAG "disable" > > > // Responses sent by 'perf record' > > #define EVLIST_CTL_RSP_ACK "ack" > > And then make the response add the newline, as conceivably we could have > something like "nack", "invalid", "enoperm", etc, no? That above makes sense. > > Anyway, this can be done on top of this series, so I'm continuing > processing it locally. > > - Arnaldo > Alexei
