mohae / deepcopy

Deep copy things
MIT License
569 stars 122 forks source link

proposed fix for a nil case #18

Closed seaguest closed 2 years ago

seaguest commented 4 years ago

Hello,

I got an error in my case:

package main

import (
    "fmt"
    "github.com/mohae/deepcopy"
)

type A struct {
    Id uint32
}

type B struct {
    Id   string
    Data *A
}

func (p *A) DeepCopy() interface{} {
    if p == nil {
        return nil
    }
    c := *p
    return &c
}

func main() {
    b := &B{Id: "b"}
    c := deepcopy.Copy(b)
    fmt.Println(c)
}

My fix is to add a nil check here

    if original.CanInterface() {
        if copier, ok := original.Interface().(Interface); ok {
            // return if copier is nil
            if reflect.ValueOf(copier).IsNil() {
                return
            }
            cpy.Set(reflect.ValueOf(copier.DeepCopy()))
            return
        }
    }
dolmen commented 3 years ago

The test case on the Go Playground: https://play.golang.org/p/aRqYwob8YM6

dolmen commented 3 years ago

The proposed fix is not the right solution, because the problem is not about the original value but about the output of .DeepCopy().

Here is the correct fix:

    if original.CanInterface() {
        if copier, ok := original.Interface().(Interface); ok {
            out := copier.DeepCopy()
            cpy.Set(reflect.ValueOf(&out).Elem())
            return
        }
    }
seaguest commented 3 years ago

@dolmen

have you verified your fix? it doesn't work. the problem here is copier could be nil, thus copier.Deepcopy will panic.

it seems this author forgot the project now.