Hi Juha, On Sun, 2016-02-07 at 23:37 +0200, Juha-Pekka Heikkilä wrote: > Hi Iago, > > I know there are lot of places where there is malloc unchecked still > -- and then there is ralloc which is a story of its own. Reason why I > think checking these would be remotely useful in windows only (or > other way around, not under linux kernel) is on Windows one can get > the null pointer from malloc. On Androids I think memory over > committing has always been enabled and on Linux I suspect I belong to > the minority who like to set ulimits for memory. > > I agree checking these mostly is quite useless but there are those > corners where it may suddenly become valuable. When process is running > and everything has settled it will be weird if hit any of these checks > but any code which is run when process is starting I notice is the > place where things will fail if they fail. This is of course just my > opinion about the value of these checks but I really dislike > possibility of segfault when it is coming from a library. > > I didn't quickly notice where _mesa_error() get more heap. Stack it of > course needs but when I did stress test these _mesa_error() did still > work. Cannot promise my test was 100% correct though, I think it was > over year ago when I was playing with it. >
I think Matt explained this but I suppose it might still help in some platforms. Ian's point about using NULL as an offset is also valid. Anyway, I am not blocking anything, the patches are a small thing and they might help in some cases so feel to go ahead with them. If you still need a Reviewed-by for these you can add mine or Samuel's. Iago > > On Wed, Feb 3, 2016 at 5:12 PM, Iago Toral <ito...@igalia.com> wrote: > > Hi Juha, > > > > I don't know why checking for this might be more relevant in Windows, > > but in any case: > > > > There are a ton of other places in mesa where we allocate memory via > > calloc/malloc and we don't check that the allocation actually succeeded > > so I am not sure that fixing a couple of instances of *small* > > allocations changes anything. > > > > IMHO, this kind of things are only really useful when allocating memory > > for large amounts of data, otherwise even if you check for a NULL > > allocation you still need to make sure that you don't need any extra > > memory to handle that situation, and _mesa_error() needs memory, so it > > is probably not really giving us anything in practice other than > > silencing Klocwork... > > > > Iago > > > > On Wed, 2016-02-03 at 10:56 +0200, Juha-Pekka Heikkila wrote: > >> I'm thinking these things maybe could be wrapped up inside something like > >> "#ifdef windows" or so in the future. At least for Android and Linux these > >> are normally quite useless. > >> > >> /Juha-Pekka > >> > >> Juha-Pekka Heikkila (2): > >> i965: in brw_link_shader() react to low memory > >> glsl: Check for null pointer at ir_variable_refcount_visitor() > >> > >> src/compiler/glsl/ir_variable_refcount.cpp | 7 +++++++ > >> src/mesa/drivers/dri/i965/brw_link.cpp | 4 ++++ > >> src/mesa/main/ff_fragment_shader.cpp | 6 ++++-- > >> 3 files changed, 15 insertions(+), 2 deletions(-) > >> > > > > > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev