code review 4859042: runtime: better checks for syscall.NewCallback parameter

runtime: better checks for syscall.NewCallback parameter

[brainman, 2011-08-10 06:53:22]

Hello golang-dev@googlegroups.com,

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

[rsc, 2011-08-10 12:49:56]

LGTM

[brainman, 2011-08-11 00:36:52]

When I set about to make these change, I was going to use reflect package
instead. But I have discovered that I can't use reflect package in syscall,
because of dependencies. It appears, reflect needs os package, becuase it uses
strconv.Itoa and strconv.Unquote (once I have removed both, I could use reflect
in syscall).

It seems to me, reflect package is just a window into runtime and should not
need os to perform. Would it be a good idea to change reflect, so it does not
need os and therefore could be used in situations where os for one reason or
other is not available? We already have itoa in syscall, perhaps, it could be
shared. Do not know about strconv.Unquote. Perhaps strconv package could be
changed instead, because it should not need os either, but just for os.Error and
couple of os constants, but that is bigger change?

Please comment. Thank you.

Alex

[rsc, 2011-08-11 01:40:16]

On Wed, Aug 10, 2011 at 20:36,  <alex.brainman@gmail.com> wrote:
> When I set about to make these change, I was going to use reflect
> package instead. But I have discovered that I can't use reflect package
> in syscall, because of dependencies. It appears, reflect needs os
> package, becuase it uses strconv.Itoa and strconv.Unquote (once I have
> removed both, I could use reflect in syscall).
>
> It seems to me, reflect package is just a window into runtime and should
> not need os to perform. Would it be a good idea to change reflect, so it
> does not need os and therefore could be used in situations where os for
> one reason or other is not available? We already have itoa in syscall,
> perhaps, it could be shared. Do not know about strconv.Unquote. Perhaps
> strconv package could be changed instead, because it should not need os
> either, but just for os.Error and couple of os constants, but that is
> bigger change?

I'd like to be able to link programs without reflect,
and syscall should be very very low-level.
It is probably best if neither depends heavily on the other.

I think what you have is fine.

Russ

[rsc, 2011-08-11 01:40:29]

> It is probably best if neither depends heavily on the other.

s/heavily //

[brainman, 2011-08-11 02:24:04]

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

runtime: better checks for syscall.NewCallback parameter

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