Re: [PATCH 01/11] perf tools: Introduce struct perf_maps_opts

From: David Ahern
Date: Mon Feb 13 2012 - 13:50:31 EST




On 02/13/2012 11:32 AM, Arnaldo Carvalho de Melo wrote:
> Em Mon, Feb 13, 2012 at 04:27:33PM +0900, Namhyung Kim escreveu:
>> The perf_maps_opts struct will be used for taking care of cpu/thread
>> maps based on user's input. Since it is used on various subcommands
>> it'd be better factoring it out.
>
> I think 'struct perf_target' is a better name than 'struct
> perf_maps_opts'.
>
> Then you can remove the 'target_' prefix from pid and tid:
>
> struct perf_target {
> pid_t pid;
> pid_t tid;
> uid_t uid;
> const char *cpu_list;
> const char *uid_str;
> bool system_wide;
> };
>
> Also bear in mind that this patch will clash with David Ahern's patch
> for supporting pid and tid lists, I'll try to integrate it today and
> then you can work on top of it in my perf/core branch, ok?
>
> Now to the other patches...
>
> - Arnaldo
>

The cleanup might make my multiple tid/pid patch easier: e.g.,

struct perf_target{
...
char errmsg[128];
};

Then if the tid/pid string parsing fails in perf_evlist__create_maps and
friends the errmsg can be put into the buffer for the callers to get a
more useful message to the user as to what happened.

Today's perf if you give it an invalid pid, scandir fails and the
command spits out the usage statement. Which is completely confusing --
ie., not clear that the command failed b/c the pid does not exist.

David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/