This is the mail archive of the
systemtap@sourceware.org
mailing list for the systemtap project.
Re: [PATCH v4 2/3] perf/sdt : Support perf-list to print SDT events in a single file
- From: Masami Hiramatsu <masami dot hiramatsu dot pt at hitachi dot com>
- To: Hemant Kumar <hemant at linux dot vnet dot ibm dot com>
- Cc: linux-kernel at vger dot kernel dot org, srikar at linux dot vnet dot ibm dot com, peterz at infradead dot org, oleg at redhat dot com, hegdevasant at linux dot vnet dot ibm dot com, mingo at redhat dot com, anton at redhat dot com, systemtap at sourceware dot org, namhyung at kernel dot org, aravinda at linux dot vnet dot ibm dot com, penberg at iki dot fi
- Date: Thu, 28 Aug 2014 19:54:07 +0900
- Subject: Re: [PATCH v4 2/3] perf/sdt : Support perf-list to print SDT events in a single file
- Authentication-results: sourceware.org; auth=none
- References: <20140827213745 dot 13454 dot 46266 dot stgit at hemant-fedora> <20140827214426 dot 13454 dot 76532 dot stgit at hemant-fedora>
(2014/08/28 6:50), Hemant Kumar wrote:
[...]
> +/*
> + * get_sdt_note_info(): flush the SDT notes onto stdout
> + */
> +static void get_sdt_note_info(struct list_head *start, const char *target)
> +{
> + struct sdt_note *pos;
> +
> + if (list_empty(start))
> + return;
> +
> + printf("%s :\n", target);
> + list_for_each_entry(pos, start, note_list) {
> + printf("%%%s : %s\n", pos->provider, pos->name);
Hmm, this will show
%app : marker
instead of
%app:marker
(blanks are placed around ":")
I think it should be the latter format.
> + }
> +}
> +
> +/*
> + * Error displayed in case of query of a
> + * single file for SDT markers
> + */
> +static int sdt_err(int val, const char *target)
> +{
> + switch (-val) {
> + case 0:
> + break;
> + case ENOENT:
> + /* Absence of SDT markers */
> + printf("%s : No SDT events found\n", target);
Please use pr_err or pr_warning for error messages.
> + break;
> + case EBADF:
> + printf("%s : Bad file name\n", target);
> + break;
> + default:
> + printf("%s\n", strerror(val));
> + }
And strerror_r instead of strerror (see https://lkml.org/lkml/2014/8/13/828)
> +
> + return val;
> +}
> +
> +/*
> + * cleanup_sdt_note_list() : Free the sdt note list
> + */
> +static void cleanup_sdt_note_list(struct list_head *sdt_notes)
> +{
> + struct sdt_note *tmp, *pos;
> +
> + if (list_empty(sdt_notes))
> + return;
You don't need to check this. If the list is empty list_for_each...
just skips loops.
> +
> + list_for_each_entry_safe(pos, tmp, sdt_notes, note_list) {
> + list_del(&pos->note_list);
> + free(pos->name);
> + free(pos->provider);
> + free(pos);
> + }
> +}
Thank you,
> +
> +/*
> + * filename__find_sdt() : looks for sdt markers and the list is
> + * stored in sdt_notes
> + */
> +static int filename__find_sdt(const char *target)
> +{
> + int ret;
> +
> + LIST_HEAD(sdt_notes);
> +
> + ret = get_sdt_note_list(&sdt_notes, target);
> + if (!ret)
> + get_sdt_note_info(&sdt_notes, target);
> + else
> + sdt_err(ret, target);
> +
> + cleanup_sdt_note_list(&sdt_notes);
> +
> + return ret;
> +}
> +
> +/*
> + * print_sdt_notes() : wrapper function
> + */
> +void print_sdt_events(const char *arg)
> +{
> + if (arg) {
> + filename__find_sdt(arg);
> + return;
> + }
> + pr_err("Error : File Name must be specified with \"sdt\" option!\n"
> + "Usage :\n perf list sdt <file-name>\n");
> +
> + return;
> +}
>
>
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com