Skip to content

py: func KwargsToDict #20

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
visualfc wants to merge 1 commit into from
Closed

py: func KwargsToDict #20

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions py/call.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,4 +122,14 @@ func (o *Object) VectorcallMethod(name *Object, args **Object, nargs uintptr, kw
return nil
}

type Kwargs map[string]*Object

func KwargsToDict(kw Kwargs) *Object {
dict := NewDict()
for k, v := range kw {
dict.DictSetItem(FromGoString(k), v)
}
return dict
}
Comment on lines +127 to +133
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

This function has several significant issues:

  1. Potential nil pointer dereference (critical): NewDict() can return nil if dictionary creation fails. The code does not check for this, which will cause a panic when dict.DictSetItem is called.
  2. Memory leak (high): FromGoString(k) likely returns a new Python string object. PyDict_SetItem increments the reference count of the key, but the original reference from FromGoString is never decremented. This leads to a memory leak for every key in the map.
  3. Unhandled errors (high): FromGoString could fail and return nil. This is not handled and would lead to a panic when DictSetItem is called with a nil key.

Please add checks for nil returns and manage the reference count of the key object correctly to prevent panics and memory leaks.

Suggested change
func KwargsToDict(kw Kwargs) *Object {
dict := NewDict()
for k, v := range kw {
dict.DictSetItem(FromGoString(k), v)
}
return dict
}
func KwargsToDict(kw Kwargs) *Object {
dict := NewDict()
if dict == nil {
return nil
}
for k, v := range kw {
key := FromGoString(k)
if key == nil {
dict.DecRef()
return nil
}
// TODO: The return value of DictSetItem should be checked for errors if its wrapper is fixed.
dict.DictSetItem(key, v)
key.DecRef()
}
return dict
}

Comment on lines +127 to +133
Copy link
Copy Markdown

@niupilot niupilot Bot Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add proper error handling for dictionary creation and item setting. The current implementation ignores potential failures from NewDict() and DictSetItem(). Note: The actual error checking for DictSetItem may need adjustment based on the correct return type - Python C API typically returns int (-1 for error, 0 for success), but the current Go binding returns *Object.

Suggested change
func KwargsToDict(kw Kwargs) *Object {
dict := NewDict()
for k, v := range kw {
dict.DictSetItem(FromGoString(k), v)
}
return dict
}
func KwargsToDict(kw Kwargs) (*Object, error) {
dict := NewDict()
if dict == nil {
return nil, errors.New("failed to create new dictionary")
}
for k, v := range kw {
if result := dict.DictSetItem(FromGoString(k), v); result != nil {
// Note: Actual error checking depends on DictSetItem return type
// Current implementation returns *Object, but Python C API should return int
}
}
return dict, nil
}


// -----------------------------------------------------------------------------
Loading