Skip to content

Commit

Permalink
Add comments aspell check (sonic-net#382)
Browse files Browse the repository at this point in the history
* Add cold discovered oids

* Add cold discovered oids

* Add buffer pool logic and unittests

* Add comments aspell check

* Bring back logging
  • Loading branch information
kcudnik authored Nov 16, 2018
1 parent 08fccb0 commit 398d24a
Show file tree
Hide file tree
Showing 41 changed files with 643 additions and 243 deletions.
2 changes: 1 addition & 1 deletion lib/inc/sairedis.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions lib/src/sai_redis_generic_create.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/

Expand Down Expand Up @@ -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
*/

Expand All @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions lib/src/sai_redis_generic_get.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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.
*/

Expand Down Expand Up @@ -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.
*/

Expand Down
4 changes: 2 additions & 2 deletions lib/src/sai_redis_generic_remove.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/

Expand All @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions lib/src/sai_redis_generic_set.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/

Expand All @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion lib/src/sai_redis_interfacequery.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ void clear_local_state()
clear_notifications();

/*
* Initialize metatada database.
* Initialize metadata database.
*/

meta_init_db();
Expand Down
2 changes: 1 addition & 1 deletion lib/src/sai_redis_record.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/

Expand Down
2 changes: 1 addition & 1 deletion meta/sai_extra_acl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions meta/sai_extra_buffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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 ?

Expand Down
2 changes: 1 addition & 1 deletion meta/sai_extra_mirror.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion meta/sai_extra_schedulergroup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
6 changes: 3 additions & 3 deletions meta/sai_extra_tunnel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion meta/sai_extra_udf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion meta/sai_extra_wred.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
34 changes: 17 additions & 17 deletions meta/sai_meta.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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.
*/

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand All @@ -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)
Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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.
*/

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
*/
Expand Down
10 changes: 5 additions & 5 deletions meta/sai_meta.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
*
Expand Down
Loading

0 comments on commit 398d24a

Please sign in to comment.