Skip to content

Null Pointer Exception Potential#207

Closed
exgalibas wants to merge 0 commit into
apache:mainfrom
exgalibas:main
Closed

Null Pointer Exception Potential#207
exgalibas wants to merge 0 commit into
apache:mainfrom
exgalibas:main

Conversation

@exgalibas

Copy link
Copy Markdown
Contributor

No description provided.

@wu-sheng

Copy link
Copy Markdown
Member

Why we have npe here? What is the case?

@wu-sheng wu-sheng requested a review from mrproliu October 10, 2024 03:28
@mrproliu

Copy link
Copy Markdown
Contributor

Please add more scenario description here, why we need this kind of check?

@exgalibas

Copy link
Copy Markdown
Contributor Author

Please add more scenario description here, why we need this kind of check?

because Entity is interface, go interface contains type and data, when type is not nil, the interface will not be nil, but the data can be nil, show some code example:
type Entity interface {
Do()
}

type ImpEntity struct {
Name string
}

func (e *ImpEntity) Do() {
fmt.Println(e.Name)
}

func main() {
var i *ImpEntity
fmt.Println(i == nil) // true
Test(i)
}

func Test(e Entity) {
if e != nil { // not nil
e.Do() // panic
}
}
so when tracer.ServiceEntity is nil, will panic

@mrproliu

Copy link
Copy Markdown
Contributor

Then, can we use the reflect.ValueOf().IsNil() to judgment the reference is nil or not? The reflect is the more lower level case, we can use the standard way to do this.

@mrproliu

mrproliu commented Oct 10, 2024

Copy link
Copy Markdown
Contributor

Also, we have change the implementation into the core, please sync the upstream to fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants