Re: [PATCH] perf: Fix indentation errors by removing extra spaces

From: Anup Sharma
Date: Thu Mar 16 2023 - 12:06:26 EST


On Wed, Mar 15, 2023 at 05:10:52PM -0700, Ian Rogers wrote:
> On Wed, Mar 15, 2023 at 1:33 PM Anup Sharma <anupnewsmail@xxxxxxxxx> wrote:
> >
> > Cleaning up tabstop related warning reported by checkpatch.pl script
> >
> > WARNING: Statements should start on a tabstop
> >
> > Signed-off-by: Anup Sharma <anupnewsmail@xxxxxxxxx>
>
> Thanks Anup,
>
> Arnaldo, what's the policy on whitespace cleanup? Obviously new
> patches should try to avoid it, but patches like this are sometimes
> frowned upon due to losing blame history.>

Thank you Ian for informing, Just to get kickstarted I sent this patch.
Will keep in mind from next time.

> Anup, when I run checkpatch.pl on this I see other warnings in the same code:

Correct, there are few other warning also, however I tried cleaning tabstop related
warning first, then thought of going to the next.

> ```
> $ b4 am ZBIro7uDO/Mii3xF@yoga
> Grabbing thread from lore.kernel.org/all/ZBIro7uDO%2FMii3xF%40yoga/t.mbox.gz
> Analyzing 1 messages in the thread
> Checking attestation on all messages, may take a moment...
> ---
> ✓ [PATCH] perf: Fix indentation errors by removing extra spaces
> ---
> ✓ Signed: DKIM/gmail.com
> ---
> Total patches: 1
> ---
> Link: https://lore.kernel.org/r/ZBIro7uDO/Mii3xF@yoga
> Base: not specified
> git am ./20230316_anupnewsmail_perf_fix_indentation_errors_by_removing_extra_spaces.mbx
> $ scripts/checkpatch.pl
> ./20230316_anupnewsmail_perf_fix_indentation_errors_by_removing_extra_spaces.mbx
> WARNING: braces {} are not necessary for single statement blocks
> #78: FILE: tools/perf/jvmti/libjvmti.c:315:
> + if (line_file_names[nr_lines - 1]) {
> + free(line_file_names[nr_lines - 1]);
> + }
>
> WARNING: Missing a blank line after declarations
> #96: FILE: tools/perf/ui/browser.c:44:
> + int color = ui_browser__percent_color(browser, percent, current);
> + ui_browser__set_color(browser, color);
> ```
>
> These should probably be fixed at the same time.

I'd want to fix those as well, if you can guide me on the correct proceduce.

> Thanks,
> Ian
>
> > ---
> > tools/perf/builtin-lock.c | 2 +-
> > tools/perf/builtin-trace.c | 6 +++---
> > tools/perf/jvmti/libjvmti.c | 8 ++++----
> > tools/perf/ui/browser.c | 4 ++--
> > tools/perf/ui/browsers/hists.c | 10 +++++-----
> > tools/perf/util/symbol-elf.c | 2 +-
> > 6 files changed, 16 insertions(+), 16 deletions(-)
> >
> > diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
> > index 054997edd98b..4a1d5a08c8d6 100644
> > --- a/tools/perf/builtin-lock.c
> > +++ b/tools/perf/builtin-lock.c
> > @@ -2094,7 +2094,7 @@ static int parse_lock_type(const struct option *opt __maybe_unused, const char *
> > char buf[32];
> >
> > if (strchr(tok, ':'))
> > - continue;
> > + continue;
> >
> > /* try :R and :W suffixes for rwlock, rwsem, ... */
> > scnprintf(buf, sizeof(buf), "%s:R", tok);
> > diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> > index 610fb60b1c0d..155abfd07286 100644
> > --- a/tools/perf/builtin-trace.c
> > +++ b/tools/perf/builtin-trace.c
> > @@ -1942,7 +1942,7 @@ static __maybe_unused bool trace__syscall_enabled(struct trace *trace, int id)
> > trace->ev_qualifier_ids.nr, sizeof(int), intcmp) != NULL;
> >
> > if (in_ev_qualifier)
> > - return !trace->not_ev_qualifier;
> > + return !trace->not_ev_qualifier;
> >
> > return trace->not_ev_qualifier;
> > }
> > @@ -3363,10 +3363,10 @@ static struct bpf_program *trace__find_usable_bpf_prog_entry(struct trace *trace
> > candidate_is_pointer = candidate_field->flags & TEP_FIELD_IS_POINTER;
> >
> > if (is_pointer) {
> > - if (!candidate_is_pointer) {
> > + if (!candidate_is_pointer) {
> > // The candidate just doesn't copies our pointer arg, might copy other pointers we want.
> > continue;
> > - }
> > + }
> > } else {
> > if (candidate_is_pointer) {
> > // The candidate might copy a pointer we don't have, skip it.
> > diff --git a/tools/perf/jvmti/libjvmti.c b/tools/perf/jvmti/libjvmti.c
> > index fcca275e5bf9..faf5925f7433 100644
> > --- a/tools/perf/jvmti/libjvmti.c
> > +++ b/tools/perf/jvmti/libjvmti.c
> > @@ -312,10 +312,10 @@ compiled_method_load_cb(jvmtiEnv *jvmti,
> > (*jvmti)->Deallocate(jvmti, (unsigned char *)class_sign);
> > free(line_tab);
> > while (line_file_names && (nr_lines > 0)) {
> > - if (line_file_names[nr_lines - 1]) {
> > - free(line_file_names[nr_lines - 1]);
> > - }
> > - nr_lines -= 1;
> > + if (line_file_names[nr_lines - 1]) {
> > + free(line_file_names[nr_lines - 1]);
> > + }
> > + nr_lines -= 1;
> > }
> > free(line_file_names);
> > }
> > diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c
> > index 78fb01d6ad63..ba725ed5c86d 100644
> > --- a/tools/perf/ui/browser.c
> > +++ b/tools/perf/ui/browser.c
> > @@ -40,8 +40,8 @@ int ui_browser__set_color(struct ui_browser *browser, int color)
> > void ui_browser__set_percent_color(struct ui_browser *browser,
> > double percent, bool current)
> > {
> > - int color = ui_browser__percent_color(browser, percent, current);
> > - ui_browser__set_color(browser, color);
> > + int color = ui_browser__percent_color(browser, percent, current);
> > + ui_browser__set_color(browser, color);
> > }
> >
> > void ui_browser__gotorc_title(struct ui_browser *browser, int y, int x)
> > diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> > index b72ee6822222..947f8023a140 100644
> > --- a/tools/perf/ui/browsers/hists.c
> > +++ b/tools/perf/ui/browsers/hists.c
> > @@ -402,7 +402,7 @@ static bool hist_browser__selection_has_children(struct hist_browser *browser)
> > return false;
> >
> > if (ms == &he->ms)
> > - return he->has_children;
> > + return he->has_children;
> >
> > return container_of(ms, struct callchain_list, ms)->has_children;
> > }
> > @@ -421,7 +421,7 @@ static bool hist_browser__selection_unfolded(struct hist_browser *browser)
> > return false;
> >
> > if (ms == &he->ms)
> > - return he->unfolded;
> > + return he->unfolded;
> >
> > return container_of(ms, struct callchain_list, ms)->unfolded;
> > }
> > @@ -436,8 +436,8 @@ static char *hist_browser__selection_sym_name(struct hist_browser *browser, char
> > return NULL;
> >
> > if (ms == &he->ms) {
> > - hist_entry__sym_snprintf(he, bf, size, 0);
> > - return bf + 4; // skip the level, e.g. '[k] '
> > + hist_entry__sym_snprintf(he, bf, size, 0);
> > + return bf + 4; // skip the level, e.g. '[k] '
> > }
> >
> > callchain_entry = container_of(ms, struct callchain_list, ms);
> > @@ -3599,7 +3599,7 @@ static bool evlist__single_entry(struct evlist *evlist)
> > int nr_entries = evlist->core.nr_entries;
> >
> > if (nr_entries == 1)
> > - return true;
> > + return true;
> >
> > if (nr_entries == 2) {
> > struct evsel *last = evlist__last(evlist);
> > diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> > index 41882ae8452e..32bd70d7b712 100644
> > --- a/tools/perf/util/symbol-elf.c
> > +++ b/tools/perf/util/symbol-elf.c
> > @@ -304,7 +304,7 @@ static char *demangle_sym(struct dso *dso, int kmodule, const char *elf_name)
> > * to it...
> > */
> > if (!want_demangle(dso->kernel || kmodule))
> > - return demangled;
> > + return demangled;
> >
> > demangled = bfd_demangle(NULL, elf_name, demangle_flags);
> > if (demangled == NULL) {
> > --
> > 2.34.1
> >