[RFR]: Field suppression list

[Arthur Eubanks]

On Tue, Jul 9, 2019 at 9:06 AM Jean Christophe Beyler <jcbeyler at google.com>
wrote:

> Hi Arthur,
>
> Sorry for the late reply :)
>
>
> http://cr.openjdk.java.net/~aeubanks/tsanfieldsupp/webrev.00/src/hotspot/share/classfile/tsanIgnoreList.hpp.html
>
>   33 // Lines that start with '#' are considered comment.
>
> -> are considered comments.
>
> Done

>
>   36 // Here are few examples.
>
> -> Here are a few examples:
>
> Done

>
> http://cr.openjdk.java.net/~aeubanks/tsanfieldsupp/webrev.00/src/hotspot/share/classfile/tsanIgnoreList.cpp.html
>
>   - line 156 could be removed (extra empty line)
>
> Done

>   - whereas line 175 could use an extra line :)
>
> Done

>   - Is there a reason that we are using fgetc? I would just use fgets and be done with all this logic handling lines...
>
> Done

>   - FWIW, I think you should extract the whole file parsing into a parse_from_file method, which would make the init : open a file, parse it via parse_from_file, close it
>
> Done

>   - parse_from_line:
>
>      - I find this error prone:
>
>         - I would really find a '#' and replace it with '\0' if found
>
> If we have to check for '#' either way, I think bailing out when we see it
is fine.

>         - then I would do a sscanf and check its return
>
>  Done.

>         - I would add a few extra lines to better understand this code btw :)
>
> Added some comments

>
>   -   35     Exact,
>
>      -> If we are going to do = 1, = 2, ... ; might as well do = 0
>
> Done

>
>   41   FieldMatcher(const Symbol* class_name, const Symbol* field_name,
>   42                Mode class_mode, Mode field_mode, FieldMatcher* next)
>
>      -> Somehow class_name, class_mode, field_name, field_mode, next seems like a better order when looking at it (to me :-))
>
> Done

>
> Thanks,
>
> Jc
>
> Also added some tests for testing Any and Prefix matching
Updated webrev:
http://cr.openjdk.java.net/~aeubanks/tsanfieldsupp/webrev.01/