harmony-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alexey Varlamov (JIRA)" <j...@apache.org>
Subject [jira] Commented: (HARMONY-2505) Class file parser improvements
Date Fri, 08 Dec 2006 10:54:22 GMT
    [ http://issues.apache.org/jira/browse/HARMONY-2505?page=comments#action_12456765 ] 
Alexey Varlamov commented on HARMONY-2505:

Thank you for spending efforts in this area, unfortunately it is mostly uncovered by tests
yet. What testing did you perform, BTW?
Please find my notes below:
1) Please do not intermix code beautifications (as white space changes) with functional modifications;
2) It would be nice to chop your work to smaller patches with descriptive comments, e.g. it
is hard to match the following change to the JIRA declaration:
-    unsigned get_num_arg_bytes() const { return _arguments_size; }
+    unsigned get_num_arg_slots() const { return _arguments_slot_num; }
3) As your changes are not fully testable at the moment, it is crucial to comment each validation
accurately referencing JVMS; you mostly did good job on this matter and please keep it going;

4) The following might give a warning on some compilers, sorta "pointless comparison of unsigned
to zero". 
     bool is_valid_index(uint16 index) const {
-        return /*index > 0 && */index < m_size;
+        // index is valid if it's greater than zero and less than m_size
+        // See specification 4.2 about constant_pool_count
+        return index > 0 && index < m_size;
5)  In the Class_File_Loader.cpp, you dropped common_attrs and must_recognize_attrs - could
you please comment why?

> Class file parser improvements
> ------------------------------
>                 Key: HARMONY-2505
>                 URL: http://issues.apache.org/jira/browse/HARMONY-2505
>             Project: Harmony
>          Issue Type: Improvement
>          Components: DRLVM
>            Reporter: Vera Volynets
>         Attachments: class_file_parser_improvements_20061207.patch
> I've found out that classloader misses a number of
> checks from specification about Class File Format.
> The patch with improvements is attached.
> Some checks described in TODO list in the patch are omitted.
> I'll continue working on them.

This message is automatically generated by JIRA.
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira


View raw message