Skip to content

Commit

Permalink
Fix memleaks in cvl (sonic-net#95)
Browse files Browse the repository at this point in the history
  • Loading branch information
@sachinholla
sachinholla authored Jun 30, 2023
1 parent f24fc03 commit 38eef09
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 19 deletions.
16 changes: 10 additions & 6 deletions cvl/cvl_api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ func (c *CVL) ValidateIncrementalConfig(jsonData string) CVLRetCode {
var dataMap map[string]interface{} = v.(map[string]interface{})

root, _ := c.translateToYang(&dataMap)
defer c.yp.FreeNode(root)
if root == nil {
return CVL_SYNTAX_ERROR

Expand All @@ -261,7 +262,7 @@ func (c *CVL) ValidateIncrementalConfig(jsonData string) CVLRetCode {
//Merge existing data for update syntax or checking duplicate entries
if (existingData != nil) {
if _, errObj = c.yp.MergeSubtree(root, existingData);
errObj.ErrCode != yparser.YP_SUCCESS {
errObj.ErrCode != yparser.YP_SUCCESS {
return CVL_ERROR
}
}
Expand All @@ -286,6 +287,7 @@ func (c *CVL) ValidateConfig(jsonData string) CVLRetCode {
if err := json.Unmarshal(b, &v); err == nil {
var value map[string]interface{} = v.(map[string]interface{})
root, _ := c.translateToYang(&value)
defer c.yp.FreeNode(root)

if root == nil {
return CVL_FAILURE
Expand Down Expand Up @@ -324,7 +326,7 @@ func (c *CVL) ValidateEditConfig(cfgData []CVLEditConfigData) (cvlErr CVLErrorIn
caller = f.Name()
}

CVL_LOG(INFO_DEBUG, "ValidateEditConfig() called from %s() : %v", caller, cfgData)
CVL_LOG(INFO_DEBUG, "ValidateEditConfig() called from %s() : %v", caller, cfgData)

if SkipValidation() {
CVL_LOG(INFO_TRACE, "Skipping CVL validation.")
Expand Down Expand Up @@ -455,6 +457,8 @@ func (c *CVL) ValidateEditConfig(cfgData []CVLEditConfigData) (cvlErr CVLErrorIn

//Step 2 : Perform syntax validation only
yang, errN := c.translateToYang(&requestedData)
defer c.yp.FreeNode(yang)

if (errN.ErrCode == CVL_SUCCESS) {
if cvlErrObj, cvlRetCode := c.validateSyntax(yang); cvlRetCode != CVL_SUCCESS {
return cvlErrObj, cvlRetCode
Expand All @@ -463,7 +467,7 @@ func (c *CVL) ValidateEditConfig(cfgData []CVLEditConfigData) (cvlErr CVLErrorIn
return errN,errN.ErrCode
}

//Step 3 : Check keys and perform semantics validation
//Step 3 : Check keys and perform semantics validation
for i := 0; i < cfgDataLen; i++ {

if (cfgData[i].VType != VALIDATE_ALL && cfgData[i].VType != VALIDATE_SEMANTICS) {
Expand All @@ -478,7 +482,7 @@ func (c *CVL) ValidateEditConfig(cfgData []CVLEditConfigData) (cvlErr CVLErrorIn
//Check key should not already exist
n, err1 := redisClient.Exists(cfgData[i].Key).Result()
if (err1 == nil && n > 0) {
//Check if key deleted and CREATE done in same session,
//Check if key deleted and CREATE done in same session,
//allow to create the entry
deletedInSameSession := false
if tbl != "" && key != "" {
Expand All @@ -494,7 +498,7 @@ func (c *CVL) ValidateEditConfig(cfgData []CVLEditConfigData) (cvlErr CVLErrorIn
CVL_LOG(WARNING, "\nValidateEditConfig(): Key = %s already exists", cfgData[i].Key)
cvlErrObj.ErrCode = CVL_SEMANTIC_KEY_ALREADY_EXIST
cvlErrObj.CVLErrDetails = cvlErrorMap[cvlErrObj.ErrCode]
return cvlErrObj, CVL_SEMANTIC_KEY_ALREADY_EXIST
return cvlErrObj, CVL_SEMANTIC_KEY_ALREADY_EXIST

} else {
TRACE_LOG(TRACE_CREATE, "\nKey %s is deleted in same session, " +
Expand Down Expand Up @@ -1099,4 +1103,4 @@ func (c *CVL) GetAllReferringTables(tableName string) (map[string][]string) {
}

return refTbls
}
}
1 change: 1 addition & 0 deletions cvl/cvl_cache.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ func (c *CVL) fetchTableDataToTmpCache(tableName string, dbKeys map[string]inter
}

_, err := pipe.Exec()
defer pipe.Close()
if err != nil {
CVL_LOG(WARNING, "Failed to fetch details for table %s", tableName)
return 0
Expand Down
45 changes: 32 additions & 13 deletions cvl/internal/yparser/yparser.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -389,8 +389,9 @@ func ParseSchemaFile(modelFile string) (*YParserModule, YParserError) {

//AddChildNode Add child node to a parent node
func(yp *YParser) AddChildNode(module *YParserModule, parent *YParserNode, name string) *YParserNode {

ret := (*YParserNode)(C.lyd_new((*C.struct_lyd_node)(parent), (*C.struct_lys_module)(module), C.CString(name)))
nameCStr := C.CString(name)
defer C.free(unsafe.Pointer(nameCStr))
ret := (*YParserNode)(C.lyd_new((*C.struct_lyd_node)(parent), (*C.struct_lys_module)(module), (*C.char)(nameCStr)))
if (ret == nil) {
TRACE_LOG(TRACE_YPARSER, "Failed parsing node %s", name)
}
Expand All @@ -400,14 +401,20 @@ func(yp *YParser) AddChildNode(module *YParserModule, parent *YParserNode, name

//IsLeafrefMatchedInUnion Check if value matches with leafref node in union
func (yp *YParser) IsLeafrefMatchedInUnion(module *YParserModule, xpath, value string) bool {

return C.lyd_node_leafref_match_in_union((*C.struct_lys_module)(module), C.CString(xpath), C.CString(value)) == 0
xpathCStr := C.CString(xpath)
valCStr := C.CString(value)
defer func() {
C.free(unsafe.Pointer(xpathCStr))
C.free(unsafe.Pointer(valCStr))
}()
return C.lyd_node_leafref_match_in_union((*C.struct_lys_module)(module), (*C.char)(xpathCStr), (*C.char)(valCStr)) == 0
}

//AddMultiLeafNodes dd child node to a parent node
func(yp *YParser) AddMultiLeafNodes(module *YParserModule, parent *YParserNode, multiLeaf []*YParserLeafValue) YParserError {

leafValArr := make([]C.struct_leaf_value, len(multiLeaf))
tmpArr := make([]*C.char, len(multiLeaf) * 2)

size := C.int(0)
for index := 0; index < len(multiLeaf); index++ {
Expand All @@ -416,11 +423,22 @@ func(yp *YParser) AddMultiLeafNodes(module *YParserModule, parent *YParserNode,
}

//Accumulate all name/value in array to be passed in lyd_multi_new_leaf()
leafValArr[index].name = C.CString(multiLeaf[index].Name)
leafValArr[index].value = C.CString(multiLeaf[index].Value)
nameCStr := C.CString(multiLeaf[index].Name)
valCStr := C.CString(multiLeaf[index].Value)
leafValArr[index].name = (*C.char)(nameCStr)
leafValArr[index].value = (*C.char)(valCStr)
size++

tmpArr = append(tmpArr, (*C.char)(nameCStr))
tmpArr = append(tmpArr, (*C.char)(valCStr))
}

defer func() {
for _, cStr := range tmpArr {
C.free(unsafe.Pointer(cStr))
}
}()

if C.lyd_multi_new_leaf((*C.struct_lyd_node)(parent), (*C.struct_lys_module)(module), (*C.struct_leaf_value)(unsafe.Pointer(&leafValArr[0])), size) != 0 {
if Tracing {
TRACE_LOG(TRACE_ONERROR, "Failed to create Multi Leaf Data = %v", multiLeaf)
Expand Down Expand Up @@ -477,7 +495,7 @@ func (yp *YParser) DestroyCache() YParserError {
return YParserError {ErrCode : YP_SUCCESS,}
}

//SetOperation Set operation
//SetOperation Set operation
func (yp *YParser) SetOperation(op string) YParserError {
if (ypOpNode == nil) {
return YParserError {ErrCode : YP_INTERNAL_UNKNOWN,}
Expand Down Expand Up @@ -516,8 +534,10 @@ func (yp *YParser) ValidateSyntax(data, depData *YParserNode) YParserError {
}

func (yp *YParser) FreeNode(node *YParserNode) YParserError {

C.lyd_free_withsiblings((*C.struct_lyd_node)(node))
if node != nil {
C.lyd_free_withsiblings((*C.struct_lyd_node)(node))
node = nil
}

return YParserError {ErrCode : YP_SUCCESS,}
}
Expand Down Expand Up @@ -578,7 +598,7 @@ func getErrorDetails() YParserError {
var errText string
var msg string
var ypErrCode YParserRetCode = YP_INTERNAL_UNKNOWN
var errMsg, errPath, errAppTag string
var errMsg, errPath, errAppTag string

ctx := (*C.struct_ly_ctx)(ypCtx)
ypErrFirst := C.ly_err_first(ctx);
Expand Down Expand Up @@ -611,13 +631,13 @@ func getErrorDetails() YParserError {
}


/* Example error messages.
/* Example error messages.
1. Leafref "/sonic-port:sonic-port/sonic-port:PORT/sonic-port:ifname" of value "Ethernet668" points to a non-existing leaf.
(path: /sonic-interface:sonic-interface/INTERFACE[portname='Ethernet668'][ip_prefix='10.0.0.0/31']/portname)
2. A vlan interface member cannot be part of portchannel which is already a vlan member
(path: /sonic-vlan:sonic-vlan/VLAN[name='Vlan1001']/members[.='Ethernet8'])
3. Value "ch1" does not satisfy the constraint "Ethernet([1-3][0-9]{3}|[1-9][0-9]{2}|[1-9][0-9]|[0-9])" (range, length, or pattern).
(path: /sonic-vlan:sonic-vlan/VLAN[name='Vlan1001']/members[.='ch1'])*/
(path: /sonic-vlan:sonic-vlan/VLAN[name='Vlan1001']/members[.='ch1'])*/


/* Fetch the TABLE Name which are in CAPS. */
Expand Down Expand Up @@ -681,7 +701,6 @@ func getErrorDetails() YParserError {

if (C.ly_errno == C.LY_EVALID) { //Validation failure
ypErrCode = translateLYErrToYParserErr(int(ypErrFirst.prev.vecode))

} else {
switch (C.ly_errno) {
case C.LY_EMEM:
Expand Down

0 comments on commit 38eef09

Please sign in to comment.