Fix panic in ExtractIntoStructPtr#2873
Conversation
There was a problem hiding this comment.
Thank you for submitting your first PR! Be sure that we will be looking at it but keep in mind
this sometimes takes a while.
Please let the maintainers know if your PR has not got enough attention after a few days.
If any doubt, please consult our PR tutorial.
|
The comment to the function you are modifying specifically mentions that it's "for internal use only". However I realise that since it's exported, it's integral part of the API surface. I think your change request has merit and it's correctly written. Can you please do the same for |
kayrus
left a comment
There was a problem hiding this comment.
@bobuhiro11 thanks for the PR. I think it's also worth adding a check, whether interface is not a pointer. And an error message should be more specific, this should help during the development.
|
I have a last-minute doubt about this PR. see #2872 (comment) |
Passing nil as an argument to 'ExtractIntoStructPtr' or 'ExtractIntoSlicePtr' causes "panic: runtime error: invalid memory address or nil pointer dereference". This is an invalid argument and should be corrected to return an error. For reference, standard package functions such as 'json.Unmarshal' return an error instead of panic. Signed-off-by: Nobuhiro MIKI <nob@bobuhiro11.net> Co-authored-by: pýrus <kayrus@users.noreply.github.com>
6ce8f91 to
571d3f5
Compare
|
@pierreprinetti @kayrus
Of course. I added the same change for
I appied your suggestions. |
|
@pierreprinetti @kayrus |
pierreprinetti
left a comment
There was a problem hiding this comment.
OK, let's merge this. I was hesitant, but if the standard library goes out of its way to match a literal nil, maybe we can too.
|
@kayrus are you fine with the changes you requested? |
|
@kayrus ping? |
|
@kayrus Could you please check? |
|
I don't understand why we can't merge it. |
|
Thank you for your review. |
Passing nil as an argument to 'ExtractIntoStructPtr' causes "panic: runtime error: invalid memory address or nil pointer dereference". This is an invalid argument and should be corrected to return an error. For reference, standard package functions such as 'json.Unmarshal' return an error instead of panic.
Fixes #2872