code review 5128053: runtime: check for nil value pointer in select syncsend case

runtime: check for nil value pointer in select syncsend case

Fixes issue 2309.

[iant, 2011-09-26 23:58:03]

Hello rsc (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg/

[bradfitz, 2011-09-27 00:02:35]

Forget to include the test from the issue?

On Mon, Sep 26, 2011 at 4:58 PM, <iant@golang.org> wrote:

> Reviewers: rsc,
>
> Message:
> Hello rsc (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change to
> https://go.googlecode.com/hg/
>
>
> Description:
> runtime: check for nil value pointer in select syncsend case
>
> Fixes issue 2309.
>
> Please review this at
http://codereview.appspot.com/**5128053/<http://codereview.appspot.com/5128053/>
>
> Affected files:
>  M src/pkg/runtime/chan.c
>
>
> Index: src/pkg/runtime/chan.c
> ==============================**==============================**=======
> --- a/src/pkg/runtime/chan.c
> +++ b/src/pkg/runtime/chan.c
> @@ -1024,7 +1024,8 @@
>        selunlock(sel);
>        if(debug)
>                runtime·printf("syncsend: sel=%p c=%p o=%d\n", sel, c, o);
> -       c->elemalg->copy(c->elemsize, sg->elem, cas->sg.elem);
> +       if(sg->elem != nil)
> +               c->elemalg->copy(c->elemsize, sg->elem, cas->sg.elem);
>        gp = sg->g;
>        gp->param = sg;
>        runtime·ready(gp);
>
>
>

[iant2, 2011-09-27 00:03:57]

Brad Fitzpatrick <bradfitz@golang.org> writes:

> Forget to include the test from the issue?

Chose not to.  But I can investigate why the existing select tests did
not test this case.

Ian

> On Mon, Sep 26, 2011 at 4:58 PM, <iant@golang.org> wrote:
>
>> Reviewers: rsc,
>>
>> Message:
>> Hello rsc (cc: golang-dev@googlegroups.com),
>>
>> I'd like you to review this change to
>> https://go.googlecode.com/hg/
>>
>>
>> Description:
>> runtime: check for nil value pointer in select syncsend case
>>
>> Fixes issue 2309.
>>
>> Please review this at
http://codereview.appspot.com/**5128053/<http://codereview.appspot.com/5128053/>
>>
>> Affected files:
>>  M src/pkg/runtime/chan.c
>>
>>
>> Index: src/pkg/runtime/chan.c
>> ==============================**==============================**=======
>> --- a/src/pkg/runtime/chan.c
>> +++ b/src/pkg/runtime/chan.c
>> @@ -1024,7 +1024,8 @@
>>        selunlock(sel);
>>        if(debug)
>>                runtime·printf("syncsend: sel=%p c=%p o=%d\n", sel, c, o);
>> -       c->elemalg->copy(c->elemsize, sg->elem, cas->sg.elem);
>> +       if(sg->elem != nil)
>> +               c->elemalg->copy(c->elemsize, sg->elem, cas->sg.elem);
>>        gp = sg->g;
>>        gp->param = sg;
>>        runtime·ready(gp);
>>
>>
>>

[rsc, 2011-09-27 00:11:30]

looks good but please add a test one way or another.

[iant, 2011-09-27 00:38:15]

PTAL

[rsc, 2011-09-27 01:45:53]

LGTM

Amazing that we hadn't hit that yet.

[iant, 2011-09-27 03:46:42]

*** Submitted as http://code.google.com/p/go/source/detail?r=8224ca964ef1 ***

runtime: check for nil value pointer in select syncsend case

Fixes issue 2309.

R=rsc, bradfitz
CC=golang-dev
http://codereview.appspot.com/5128053