----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/252/#review575 -----------------------------------------------------------
indra/newview/llviewertexturelist.cpp <http://codereview.secondlife.com/r/252/#comment497> Almost every line of this method is a potential crasher. Are you sure we should handle errors this way? I would: * Add a NULL check for the "image" parameter (besides the llassert which is not evaluated in release builds). * Try handling the other errors without crashing, by issuing a warning and returning FALSE. Otherwise we may fix one crash and add two. indra/newview/llviewertexturelist.cpp <http://codereview.secondlife.com/r/252/#comment500> ditto indra/newview/llviewertexturelist.cpp <http://codereview.secondlife.com/r/252/#comment501> * No check for mInitialized before accessing sRenderThreadID. * I don't quite get what you're trying to achieve here by passing sRenderThreadID. It doesn't guarantee that the method is invoked from thread #<sRenderThreadID>, does it? - Vadim On April 8, 2011, 10:26 a.m., Xiaohong Bao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://codereview.secondlife.com/r/252/ > ----------------------------------------------------------- > > (Updated April 8, 2011, 10:26 a.m.) > > > Review request for Viewer. > > > Summary > ------- > > this is to resubmit the patch for storm-973. > > We are not very clear what causes this. But this fix is targeting three most > possible causes: > 1, a texture is failed to add into mImageList but its flag is set to be > successful; > 2, a texture status is changed not from the main thread, because gTextureList > is not thread-safe; > 3, gTextureList is accessed before it is initialized. > > I regenerated the viewer-development-storm-973 branch based on the latest > viewer-development branch. If you still can not apply the patch directly, I > am afraid you should do the manual merge. Otherwise grant me the permission, > I will do it. > > > This addresses bug storm-973. > http://jira.secondlife.com/browse/storm-973 > > > Diffs > ----- > > indra/newview/lldrawpoolbump.cpp 13670741a0a8 > indra/newview/llviewertexturelist.h 13670741a0a8 > indra/newview/llviewertexturelist.cpp 13670741a0a8 > > Diff: http://codereview.secondlife.com/r/252/diff > > > Testing > ------- > > > Thanks, > > Xiaohong > >
_______________________________________________ Policies and (un)subscribe information available here: http://wiki.secondlife.com/wiki/OpenSource-Dev Please read the policies before posting to keep unmoderated posting privileges
