acelyc111 commented on code in PR #1378:
URL: 
https://github.com/apache/incubator-pegasus/pull/1378#discussion_r1126707705


##########
src/runtime/ranger/ranger_resource_policy_manager.cpp:
##########
@@ -22,6 +22,34 @@
 namespace dsn {
 namespace ranger {
 
+#define RETURN_ERR_IF_MISSING_MEMBER(obj, member)                              
                    \
+    do {                                                                       
                    \
+        if (!obj.IsObject() || !obj.HasMember(member)) {                       
                    \
+            return dsn::ERR_RANGER_PARSE_ACL;                                  
                    \
+        }                                                                      
                    \
+    } while (0)
+
+#define CONTINUE_IF_MISSING_MEMBER(obj, member)                                
                    \
+    do {                                                                       
                    \
+        if (!obj.IsObject() || !obj.HasMember(member)) {                       
                    \
+            continue;                                                          
                    \
+        }                                                                      
                    \
+    } while (0)
+
+#define RETURN_ERR_IF_NOT_ARRAY(obj)                                           
                    \
+    do {                                                                       
                    \
+        if (!obj.IsArray() || obj.Size() == 0) {                               
                    \

Review Comment:
   ```suggestion
           if (!obj.IsArray() || obj.Empty()) {                                 
                  \
   ```



##########
src/runtime/ranger/ranger_resource_policy_manager.cpp:
##########
@@ -97,5 +134,32 @@ 
ranger_resource_policy_manager::ranger_resource_policy_manager(
                               "RPC_CM_RENAME_APP"},
                              _ac_type_of_database_rpcs);
 }
+
+void ranger_resource_policy_manager::parse_policies_from_json(const 
rapidjson::Value &data,
+                                                              
std::vector<policy_item> &policies)
+{
+    CHECK(policies.empty(), "Ranger policy list should not be empty.");
+    RETURN_VOID_IF_NOT_ARRAY(data);
+    for (const auto &item : data.GetArray()) {
+        policy_item pi;

Review Comment:
   nit: move to L155 where is closer to the place use it? 



##########
src/runtime/ranger/ranger_resource_policy_manager.cpp:
##########
@@ -22,6 +22,34 @@
 namespace dsn {
 namespace ranger {
 
+#define RETURN_ERR_IF_MISSING_MEMBER(obj, member)                              
                    \
+    do {                                                                       
                    \
+        if (!obj.IsObject() || !obj.HasMember(member)) {                       
                    \
+            return dsn::ERR_RANGER_PARSE_ACL;                                  
                    \
+        }                                                                      
                    \
+    } while (0)
+
+#define CONTINUE_IF_MISSING_MEMBER(obj, member)                                
                    \
+    do {                                                                       
                    \
+        if (!obj.IsObject() || !obj.HasMember(member)) {                       
                    \
+            continue;                                                          
                    \
+        }                                                                      
                    \
+    } while (0)
+
+#define RETURN_ERR_IF_NOT_ARRAY(obj)                                           
                    \
+    do {                                                                       
                    \
+        if (!obj.IsArray() || obj.Size() == 0) {                               
                    \
+            return dsn::ERR_RANGER_PARSE_ACL;                                  
                    \
+        }                                                                      
                    \
+    } while (0)
+
+#define RETURN_VOID_IF_NOT_ARRAY(obj)                                          
                    \
+    do {                                                                       
                    \
+        if (!obj.IsArray() || obj.Size() == 0) {                               
                    \

Review Comment:
   ```suggestion
           if (!obj.IsArray() || obj.Empty()) {                                 
                  \
   ```



##########
src/runtime/ranger/ranger_resource_policy.h:
##########
@@ -46,12 +47,18 @@ extern access_type operator|(access_type lhs, access_type 
rhs);
 
 extern access_type operator&(access_type lhs, access_type rhs);
 
+extern access_type &operator|=(access_type &lhs, access_type rhs);
+
+extern uint8_t access_type_to_int8_t(const access_type &ac_type);
+
 // Ranger policy data structure
 struct policy_item
 {
-    access_type access_types;
+    uint8_t access_types;

Review Comment:
   I found you have overrided some operators, e.g. `|`, `&`, and etc, so where 
else needed to use `uint8_t`?
   
   @empiredan do you think it's worth to use enum class? It seems there are too 
many static_casts.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to