> On June 21, 2011, 7:57 a.m., Alain Linden wrote: > > indra/llvfs/tests/lldiriterator_test.cpp, line 46 > > <http://codereview.secondlife.com/r/357/diff/1/?file=3024#file3024line46> > > > > Personally, I dislike referencing jira items in code comments. Its > > referencing something ephemeral in something that will long out live it.
In general I'd agree, but here my interpretation was that this test was inspired by a specific issue. Should in that case the whole issue description be copied into the comment? - Boroondas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/357/#review795 ----------------------------------------------------------- On June 20, 2011, 8:21 p.m., Squire Linden wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://codereview.secondlife.com/r/357/ > ----------------------------------------------------------- > > (Updated June 20, 2011, 8:21 p.m.) > > > Review request for Viewer, Oz Linden, Seth ProductEngine, and Alain Linden. > > > Summary > ------- > > Changes to fix CHOP-662. CHOP-662 was caused by the recent addition of > lldiriterator.cpp which is used to iterate over directories. It takes a mask > in the form of a glob string to use as a pattern for which files to iterate > over. > > Unfortunately, as originally implemented, it converted the glob to a regex > pattern without escaping any legal glob characters which are illegal regex > characters. As a result if the passed in mask was an illegal regex it would > fail and llerrs would cause a crash. > > In CHOP-662 this happens whenever a group name has, e.g. a '+' character at > the beginning. When logging in the viewer would attempt to load the group > chat log file which would result in a glob starting with a '+' character and > in turn to an illegal regex in lldiriterator. > > Also, the chat code was doing nothing to ensure that illegal glob characters > were not being passed to the lldiriterator constructor. > > > This addresses bug CHOP-662. > http://jira.secondlife.com/browse/CHOP-662 > > > Diffs > ----- > > doc/contributions.txt e8f2a53c3d6e > indra/llvfs/CMakeLists.txt e8f2a53c3d6e > indra/llvfs/lldiriterator.cpp e8f2a53c3d6e > indra/llvfs/tests/lldiriterator_test.cpp PRE-CREATION > indra/newview/lllogchat.cpp e8f2a53c3d6e > > Diff: http://codereview.secondlife.com/r/357/diff > > > Testing > ------- > > Added a unit test for lldiriterator which checks for construction failures on > examples of the most common group names which were causing the crashes. > > > Thanks, > > Squire > >
_______________________________________________ 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
