> On April 8, 2011, 4:51 p.m., Boroondas Gupte wrote: > > indra/llimage/llimagedimensionsinfo.cpp, lines 217-219 > > <http://codereview.secondlife.com/r/255/diff/2/?file=1426#file1426line217> > > > > Wouldn't a seek be a "cheaper" way to determine size than reading into > > an actual buffer? (According to indra/llcommon/llapr.h, it returns -1 on > > failure.) > > > > There's also a static method LLAPRFile::size, but that seems to operate > > on not-yet-opened files given by filename. > > Vadim ProductEngine wrote: > seek() may go beyond the file end, so we can't use it to check whether > the file contains the needed header. > Well, we could seek to the end of file, but accessing the whole file just > to find out that it's of wrong type looks like overkill. > I agree that reading into a dynamically allocated buffer doesn't look > nice, but I think we can live with it as long as we only read small chunks > this way. > > Boroondas Gupte wrote: > There's no pointer equivalent of /dev/null that could be used here > instead of an actual buffer, is there? > > Vadim ProductEngine wrote: > No. Well, there is NULL, but you'll get a crash if you use it as a buffer > address. :-)
The LLAPRFile::seek() with offset < 0 does apr_file_seek(file_handle, APR_END, 0), and apr_file_seek() doesn't "access the whole file" it just does an apr_file_info_get(), checks the size (the OS should know...) and jumps there, then returns the result. So you should really do a seek() :) - Kiptic ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/255/#review580 ----------------------------------------------------------- On April 7, 2011, 4:20 p.m., Vadim ProductEngine wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://codereview.secondlife.com/r/255/ > ----------------------------------------------------------- > > (Updated April 7, 2011, 4:20 p.m.) > > > Review request for Viewer. > > > Summary > ------- > > * Added checks for image file contents not matching the file extension (e.g. > a bitmap named file.jpg). > * Added checks for abnormally short files to avoid crashes when parsing them. > > > This addresses bug STORM-1118. > http://jira.secondlife.com/browse/STORM-1118 > > > Diffs > ----- > > indra/llimage/llimagedimensionsinfo.h 33ca961b0870 > indra/llimage/llimagedimensionsinfo.cpp 33ca961b0870 > > Diff: http://codereview.secondlife.com/r/255/diff > > > Testing > ------- > > > Thanks, > > Vadim > >
_______________________________________________ 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
