Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/cgo: panic if C.CString can't allocate memory #3403

Closed
alberts opened this issue Mar 26, 2012 · 9 comments

Comments

@alberts
Copy link
Contributor

commented Mar 26, 2012

Please provide any additional information below.

C.CString currently just returns nil if it can't malloc. Maybe it should panic. After Go
1.

https://groups.google.com/forum/?fromgroups#!topic/golang-dev/G7T0_FPU20w
@rsc

This comment has been minimized.

Copy link
Contributor

commented Mar 26, 2012

Comment 1:

Not sure about this.  C.malloc is an ordinary function, so it's 
hard to make that one panic.  It's difficult to see why C.CString
should be different.
FWIW, you're on Linux so it will never happen anyway.

Labels changed: added priority-later, removed priority-triage.

Status changed to Thinking.

@alberts

This comment has been minimized.

Copy link
Contributor Author

commented Mar 27, 2012

Comment 2:

Just to check, what will never happen? malloc returning ENOMEM?
@rsc

This comment has been minimized.

Copy link
Contributor

commented Mar 27, 2012

Comment 3:

Yes, Linux overallocates VM, so seeing a NULL from malloc or strdup is
very very rare.
@alberts

This comment has been minimized.

Copy link
Contributor Author

commented Mar 27, 2012

Comment 4:

With /proc/sys/vm/overcommit_memory set to 2 or with a memory controller cgroup, this
picture changes a little.
@rsc

This comment has been minimized.

Copy link
Contributor

commented Jul 30, 2013

Comment 5:

OS X may force us into handling C.malloc specially, because it changed
the argument from size_t to ulong in the XCode for 10.9 Mavericks.
If so, maybe it makes sense for both C.malloc and C.CString to panic
when out of memory.

Labels changed: added go1.2maybe.

@rsc

This comment has been minimized.

Copy link
Contributor

commented Sep 11, 2013

Comment 6:

Status changed to Started.

@rsc

This comment has been minimized.

Copy link
Contributor

commented Sep 11, 2013

Comment 7:

This issue was closed by revision 397ba2c.

Status changed to Fixed.

@gopherbot

This comment has been minimized.

Copy link

commented Sep 14, 2013

Comment 8 by ralph.corderoy:

The definition of Cmalloc,
http://code.google.com/p/go/source/browse/src/cmd/cgo/out.go?spec=svn46fd4ef6c0deeb184aa0b843dfc080cc869a7e19&name=46fd4ef6c0de&r=46fd4ef6c0deeb184aa0b843dfc080cc869a7e19#1226
assumes a NULL return from malloc() means it's out of memory, but NULL is a 
valid return for malloc(0).  Perhaps Cmalloc will never be called for zero 
bytes of memory and this isn't an issue, but I thought it worth checking.  If
Cmalloc(0) isn't possible, a comment to that affect would stop future readers
wondering.
@rsc

This comment has been minimized.

Copy link
Contributor

commented Sep 15, 2013

Comment 9:

Thank you for the reminder; how quickly we forget. 
Filed issue #6390

@alberts alberts added fixed labels Sep 15, 2013

@rsc rsc added this to the Go1.2 milestone Apr 14, 2015

@rsc rsc removed the go1.2maybe label Apr 14, 2015

@golang golang locked and limited conversation to collaborators Jun 24, 2016

This issue was closed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
3 participants
You can’t perform that action at this time.