Messages in this thread | | | Subject | Re: [PATCH 1/2] perf jevents: Tidy error handling | From | John Garry <> | Date | Tue, 20 Oct 2020 09:35:28 +0100 |
| |
On 20/10/2020 09:14, kajoljain wrote: > > > On 10/20/20 11:13 AM, Namhyung Kim wrote: >> Hello, >> >> On Tue, Oct 20, 2020 at 12:42 AM John Garry <john.garry@huawei.com> wrote: >>> >>> There is much duplication in the error handling for directory traversing >>> for processing JSONs. >>> >>> Factor out the common code to tidy a bit. >>> >>> Signed-off-by: John Garry <john.garry@huawei.com> >>> --- >> [SNIP] >>> -empty_map: >>> +err_processing_std_arch_event_dir: >>> + err_string_ext = " for std arch event"; >>> +err_processing_dir: >>> + if (verbose || rc > 0) { >>> + pr_info("%s: Error walking file tree %s%s\n", prog, ldirname, >>> + err_string_ext); >> >> This was printed only if verbose is set but now changed. >
pr_info() is silenced if verbose is unset, so in effect it is the same - see pr_info() definition in json.h
> Hi John, > Yes I agree with Namhyung. > So, this is our original checks > > if (rc && verbose) { > pr_info("%s: Error walking file tree %s\n", prog, ldirname); > goto empty_map; > } else if (rc < 0) { > /* Make build fail */ > fclose(eventsfp); > free_arch_std_events(); > ret = 1; > goto out_free_mapfile; > } else if (rc) { > goto empty_map; > } > > May be we can use similar checks: > > if( verbose) > pr_info("%s: Error walking file tree %s%s\n", prog, ldirname,err_string_ext); > if(rc > 0) > empty_map = 1; > else > ret = 1; >
Not that it matters much, this logic is slightly different for verbose set and rc < 0. I don't mind going with that, so let me know.
Thanks for checking, John
> Thanks, > Kajol Jain > >> >> Thanks >> Namhyung >> >> >>> + empty_map = 1; >>> + } else { >>> + ret = 1; >>> + } >>> +err_close_eventsfp: >>> fclose(eventsfp); >>> - create_empty_mapping(output_file); >>> + if (empty_map) >>> + create_empty_mapping(output_file); >>> +err_out: >>> free_arch_std_events(); >>> -out_free_mapfile: >>> free(mapfile); >>> return ret; >>> } >>> -- >>> 2.26.2 >>> > . >
| |