Add comments aspell check (#382)

* Add cold discovered oids

* Add cold discovered oids

* Add buffer pool logic and unittests

* Add comments aspell check

* Bring back logging

lib/inc/sairedis.h

@@ -95,7 +95,7 @@ typedef enum _sai_redis_switch_attr_t
      * to ".n" suffix, and when we reopen file, we will actually create new
      * one.
      *
-     * This attribute is only setting variable in memroy, it's safe to call
+     * This attribute is only setting variable in memory, it's safe to call
      * this from signal handler.
      *
      * @type bool

lib/src/sai_redis_generic_create.cpp

@@ -93,7 +93,7 @@ sai_object_type_t sai_object_type_query(
         SWSS_LOG_ERROR("invalid object id 0x%lx", object_id);
 
         /*
-         * We can't throw here, since it would give no meaningfull message.
+         * We can't throw here, since it would give no meaningful message.
          * Throwing at one level up is better.
          */
 
@@ -356,7 +356,7 @@ sai_status_t internal_redis_bulk_generic_create(
     }
 
     /*
-     * We are adding number of entries to actualy add ':' to be compatible
+     * We are adding number of entries to actually add ':' to be compatible
      * with previous
      */
 
@@ -372,7 +372,7 @@ sai_status_t internal_redis_bulk_generic_create(
         }
 
         /*
-         * Capital 'C' stads for bulk CREATE operation.
+         * Capital 'C' stands for bulk CREATE operation.
          */
 
         recordLine("C|" + str_object_type + joined);

lib/src/sai_redis_generic_get.cpp

@@ -23,7 +23,7 @@ sai_status_t internal_redis_get_process(
 
     sai_deserialize_status(str_sai_status, status);
 
-    // we could deserialize directly to user data, but list is alocated by deserializer
+    // we could deserialize directly to user data, but list is allocated by deserializer
     if (status == SAI_STATUS_SUCCESS)
     {
         SaiAttributeList list(object_type, values, false);
@@ -117,7 +117,7 @@ void clear_oid_values(
             default:
 
                 /*
-                 * If in futre new attribute with object id will be added this
+                 * If in future new attribute with object id will be added this
                  * will make sure that we will need to add handler here.
                  */
 
@@ -155,7 +155,7 @@ sai_status_t internal_redis_generic_get(
 
     /*
      * Since user may reuse buffers, then oid list buffers maybe not cleared
-     * and contain som garbage, let's clean them so we send all oids as null to
+     * and contain some garbage, let's clean them so we send all oids as null to
      * syncd.
      */
 

lib/src/sai_redis_generic_remove.cpp

@@ -102,7 +102,7 @@ sai_status_t internal_redis_bulk_generic_remove(
     }
 
     /*
-     * We are adding number of entries to actualy add ':' to be compatible
+     * We are adding number of entries to actually add ':' to be compatible
      * with previous
      */
 
@@ -118,7 +118,7 @@ sai_status_t internal_redis_bulk_generic_remove(
         }
 
         /*
-         * Capital 'C' stads for bulk CREATE operation.
+         * Capital 'C' stands for bulk CREATE operation.
          */
 
         recordLine("C|" + str_object_type + joined);

lib/src/sai_redis_generic_set.cpp

@@ -81,7 +81,7 @@ sai_status_t internal_redis_bulk_generic_set(
     }
 
     /*
-     * We are adding number of entries to actualy add ':' to be compatible
+     * We are adding number of entries to actually add ':' to be compatible
      * with previous
      */
 
@@ -97,7 +97,7 @@ sai_status_t internal_redis_bulk_generic_set(
         }
 
         /*
-         * Capital 'S' stads for bulk SET operation.
+         * Capital 'S' stands for bulk SET operation.
          */
 
         recordLine("S|" + str_object_type + joined);

lib/src/sai_redis_interfacequery.cpp

@@ -42,7 +42,7 @@ void clear_local_state()
     clear_notifications();
 
     /*
-     * Initialize metatada database.
+     * Initialize metadata database.
      */
 
     meta_init_db();

lib/src/sai_redis_record.cpp

@@ -37,7 +37,7 @@ void logfileReopen()
 
     /*
      * On log rotate we will use the same file name, we are assuming that
-     * logrotate deamon move filename to filename.1 and we will create new
+     * logrotate daemon move filename to filename.1 and we will create new
      * empty file here.
      */
 

meta/sai_extra_acl.cpp

@@ -162,7 +162,7 @@ sai_status_t meta_pre_get_acl_entry_attribute(
 {
     SWSS_LOG_ENTER();
 
-    // TODO for get we need to check if attrib was set previously ?
+    // TODO for get we need to check if attribute was set previously ?
     // like field or action, or can we get action that was not set?
 
     return SAI_STATUS_SUCCESS;

meta/sai_extra_buffer.cpp

@@ -44,7 +44,7 @@ sai_status_t meta_pre_create_buffer_profile(
     SWSS_LOG_ENTER();
 
     // TODO extra logic on checking profile buffer size may be needed
-    // TODO we need to query other attribute pool id assigned and check wheter mode is dynamic/static
+    // TODO we need to query other attribute pool id assigned and check whether mode is dynamic/static
 
     const sai_attribute_t* attr_shared_dynamic_th = get_attribute_by_id(SAI_BUFFER_PROFILE_ATTR_SHARED_DYNAMIC_TH, attr_count, attr_list);
 
@@ -80,7 +80,7 @@ sai_status_t meta_pre_set_buffer_profile_attr(
 {
     SWSS_LOG_ENTER();
 
-    // TODO on set, changing buffer pool on profiles should noe be possible
+    // TODO on set, changing buffer pool on profiles should not be possible
     // to change dynamic to static on the fly
     // pool_id on profile should be create_only ?
 

meta/sai_extra_mirror.cpp

@@ -57,7 +57,7 @@ sai_status_t meta_pre_set_mirror_session_attribute(
     // TODO we need to type to decide which parameters are safe to set
     // SAI_MIRROR_SESSION_ATTR_MONITOR_PORT:
     // TODO should changing port during mirror session should be possible ?
-    // what if new port is somehow incompattible with current session?
+    // what if new port is somehow incompatible with current session?
 
     // SAI_MIRROR_SESSION_ATTR_GRE_PROTOCOL_TYPE: // TODO validate GRE protocol
     return is_header_version_ok(1, attr);

meta/sai_extra_schedulergroup.cpp

@@ -19,7 +19,7 @@ sai_status_t meta_pre_create_scheduler_group(
         return SAI_STATUS_INVALID_PARAMETER;
     }
 
-    // TODO level will require some additional validation to not crete loops
+    // TODO level will require some additional validation to not create loops
 
     uint8_t max_childs = attr_max_childs->value.u8;
 

meta/sai_extra_tunnel.cpp

@@ -77,10 +77,10 @@ sai_status_t meta_pre_create_tunnel(
     // TODO for GRE/VXLAN it may be different type (router interface, overlay/underlay
     //
     // TODO validate object on that list! if they exist
-    // shoud this list contain at least 1 element ? or can it be empty?
+    // should this list contain at least 1 element ? or can it be empty?
     // check for duplicates on list ? - ecn mappers
 
-    // TODO sai spec is inconsisten here, if this is mandatory attribute on some condition,
+    // TODO sai spec is inconsistent here, if this is mandatory attribute on some condition,
     // then it cannot have default value, dscp mode and ttl mode
 
     return SAI_STATUS_SUCCESS;
@@ -145,7 +145,7 @@ sai_status_t meta_pre_create_tunnel_term_table_entry (
     // TODO check is this conditional attribute, maybe this action is only
     // required for ip in ip tunnel types
 
-    // TODO additional checks may be required sinec this action tunnel id is used for
+    // TODO additional checks may be required since this action tunnel id is used for
     // decap so maybe this tunnel must have special attributes on creation set
 
     return SAI_STATUS_SUCCESS;

meta/sai_extra_udf.cpp

@@ -9,7 +9,7 @@ sai_status_t meta_pre_create_udf(
     SWSS_LOG_ENTER();
 
     // TODO validate offset value range
-    // TODO length must be quual to group attr length ?
+    // TODO length must be equal to group attr length ?
     // TODO extra validation may be required here on mask
 
     return SAI_STATUS_SUCCESS;

meta/sai_extra_wred.cpp

@@ -40,7 +40,7 @@ sai_status_t meta_pre_create_wred_profile(
     // green_min < green_max < yellow_min < yellow_max < red_min < red_max
     // This logic will be required also inside SET, so
 
-    // TODO need to be obrained from switch, also will be required during set
+    // TODO need to be obtained from switch, also will be required during set
     uint32_t max_buffer_size = 0x10000; // is this SAI_SWITCH_ATTR_TOTAL_BUFFER_SIZE ?
 
     // TODO change numbers to defines

meta/sai_meta.cpp

@@ -83,7 +83,7 @@ static bool meta_unittests_get_and_erase_set_readonly_flag(
 
     if (!unittests_enabled)
     {
-        // explicityly  to not produce false alarms
+        // explicitly to not produce false alarms
         SWSS_LOG_NOTICE("unittests are not enabled");
         return false;
     }
@@ -192,8 +192,8 @@ class SaiAttrWrapper
             SWSS_LOG_ENTER();
 
             /*
-             * On destructor we need to call free to dealocate possible
-             * alocated list on constructor.
+             * On destructor we need to call free to deallocate possible
+             * allocated list on constructor.
              */
 
             sai_deserialize_free_attribute_value(m_meta->attrvaluetype, m_attr);
@@ -220,7 +220,7 @@ std::string get_attr_info(const sai_attr_metadata_t& md)
 
     /*
      * Attribute name will contain object type as well so we don't need to
-     * serialize object type separatly.
+     * serialize object type separately.
      */
 
     return std::string(md.attridname) + ":" + sai_serialize_attr_value_type(md.attrvaluetype);
@@ -596,7 +596,7 @@ sai_status_t meta_generic_validation_objlist(
     }
 
     /*
-     * We need oids set and object type to check whehter oids are not repeated
+     * We need oids set and object type to check whether oids are not repeated
      * on list and whether all oids are same object type.
      */
 
@@ -741,7 +741,7 @@ std::string construct_key(
     SWSS_LOG_ENTER();
 
     /*
-     * Use map to make sure that keys will be always sorded by id.
+     * Use map to make sure that keys will be always sorted by id.
      */
 
     std::map<int32_t, std::string> keys;
@@ -878,9 +878,9 @@ sai_status_t meta_generic_validate_non_object_on_create(
 
     /*
      * Since non object id objects can contain several object id's inside
-     * object id strucutre, we need to check whether they all belong to the
+     * object id structure, we need to check whether they all belong to the
      * same switch (sine multiple switches can be present and whether all those
-     * objects are allowd respectivly on their members.
+     * objects are allowed respectively on their members.
      *
      * This check is required only on creation, since on set/get/remove we
      * check in object hash whether this object exists.
@@ -894,7 +894,7 @@ sai_status_t meta_generic_validate_non_object_on_create(
     }
 
     /*
-     * This will be most utilzed for createing route entries.
+     * This will be most utilized for creating route entries.
      */
 
     for (size_t j = 0; j < info->structmemberscount; ++j)
@@ -1075,7 +1075,7 @@ sai_status_t meta_generic_validation_create(
 
     bool haskeys = false;
 
-    // check each attribute separetly
+    // check each attribute separately
     for (uint32_t idx = 0; idx < attr_count; ++idx)
     {
         const sai_attribute_t* attr = &attr_list[idx];
@@ -1468,7 +1468,7 @@ sai_status_t meta_generic_validation_create(
         return SAI_STATUS_FAILURE;
     }
 
-    // check if all mandatory attrributes were passed
+    // check if all mandatory attributes were passed
 
     for (auto mdp: metadata)
     {
@@ -1588,7 +1588,7 @@ sai_status_t meta_generic_validation_create(
         {
             const auto& c = *md.conditions[index];
 
-            // condtions may only be on the same object type
+            // conditions may only be on the same object type
             const auto& cmd = *sai_metadata_get_attr_metadata(meta_key.objecttype, c.attrid);
 
             const sai_attribute_value_t* cvalue = cmd.defaultvalue;
@@ -1749,7 +1749,7 @@ sai_status_t meta_generic_validation_remove(
         {
             /*
              * We allow to remove switch object even if there are ROUTE_ENTRY
-             * created and refrencing this switch, since remove could be used
+             * created and referencing this switch, since remove could be used
              * in WARM boot scenario.
              */
 
@@ -2255,7 +2255,7 @@ sai_status_t meta_generic_validation_get(
              * attribute also oid, and then did a "set" on that value, and now
              * reference is not decreased since previous oid was not snooped.
              *
-             * TODO This concearn all attributes not only conditionals
+             * TODO This concern all attributes not only conditionals
              *
              * If attribute is conditional, we need to check if condition is
              * met, if not then this attribute is not mandatory so we can
@@ -3180,7 +3180,7 @@ void meta_generic_validation_post_get_objlist(
      * when we doing get on acl field/action. But none of those are created
      * internally by switch.
      *
-     * TODO Similar stuff is with SET, when we will set oid obejct on existing
+     * TODO Similar stuff is with SET, when we will set oid object on existing
      * switch object, but we will not have it's previous value.  We can check
      * whether default value is present and it's const NULL.
      */
@@ -3249,7 +3249,7 @@ void meta_generic_validation_post_get_objlist(
         if (!object_reference_exists(oid))
         {
             // NOTE: there may happen that user will request multiple object lists
-            // and first list was retrived ok, but second failed with overflow
+            // and first list was retrieved ok, but second failed with overflow
             // then we may forget to snoop
 
             META_LOG_INFO(md, "returned get object on list [%u] oid 0x%lx object type %d does not exists in local DB (snoop)", i, oid, ot);
@@ -3297,7 +3297,7 @@ void meta_generic_validation_post_get(
     switch_id = meta_extract_switch_id(meta_key, switch_id);
 
     /*
-     * TODO We should snoop attributes retrived from switch and put them to
+     * TODO We should snoop attributes retrieved from switch and put them to
      * local db if they don't exist since if attr is oid it may lead to
      * inconsistency when counting reference
      */

meta/sai_meta.h

@@ -152,19 +152,19 @@ void meta_unittests_enable(
         _In_ bool enable);
 
 /**
- * @brief Indicates whethre unittests are enabled;
+ * @brief Indicates whether unittests are enabled;
  */
 bool meta_unittests_enabled();
 
 /**
- * @bried Allow to perform SET operation on READ_ONLY attribue only once.
+ * @brief Allow to perform SET operation on READ_ONLY attribute only once.
  *
  * This function relaxes metadata checking on SET operation, it allows to
  * perform SET api on READ_ONLY attribute only once on specific object type and
- * specific attribue.
+ * specific attribute.
  *
  * Once means that SET operation is only relaxed for the very next SET call on
- * that specific object type and attrirbute id.
+ * that specific object type and attribute id.
  *
  * Function is explicitly named ONCE, since it will force test developer to not
  * forget that SET check is relaxed, and not forget for future unittests.
@@ -186,7 +186,7 @@ bool meta_unittests_enabled();
  * It can be dangerous to set any readonly attribute to different values since
  * internal metadata logic maybe using that value and in some cases metadata
  * database may get out of sync and cause unexpected results in api calls up to
- * application carash.
+ * application crash.
  *
  * This function is not thread safe.
  *