lkml.org 
[lkml]   [2020]   [Oct]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 1/2] perf jevents: Tidy error handling
From
Date
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
>>>
> .
>

\
 
 \ /
  Last update: 2020-10-20 10:38    [W:0.048 / U:0.264 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site