----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105057/#review17275 -----------------------------------------------------------
Ship it! Almost there; will be OK iff everything below is fixed. kjs/jsonstringify.cpp <http://git.reviewboard.kde.org/r/105057/#comment13452> Does this do the right thing with NaN and the like? kjs/jsonstringify.cpp <http://git.reviewboard.kde.org/r/105057/#comment13453> Resetting m_state, m_rootIsUndefined here might be a good defensive move (just in case stringify starts getting called twice) kjs/jsonstringify.cpp <http://git.reviewboard.kde.org/r/105057/#comment13454> This looks like we could end up looping indefinitely here and crashing with out of stack depth (if they keep returning new objects which have .toJSON set0. I think some checking for m_objectStack.size() may be in order. Probably a good idea in case someone tries to serialize something really deep, too. kjs/jsonstringify.cpp <http://git.reviewboard.kde.org/r/105057/#comment13455> Can get an exception here kjs/jsonstringify.cpp <http://git.reviewboard.kde.org/r/105057/#comment13456> You don't want to be keying by UString here, but rather than Identifier, since they guarantee reference equality of equal strings, unlike UString. CommonIdentifiers.h has the traits for them. - Maks Orlovich On July 25, 2012, 2:54 p.m., Bernd Buschinski wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/105057/ > ----------------------------------------------------------- > > (Updated July 25, 2012, 2:54 p.m.) > > > Review request for kdelibs. > > > Description > ------- > > kjs: Implement JSON.stringify > > patch needs https://git.reviewboard.kde.org/r/105056/ (JSON.parse) > > > Diffs > ----- > > kjs/CMakeLists.txt 1188064 > kjs/CommonIdentifiers.h 8ee40e8 > kjs/json_object.h PRE-CREATION > kjs/json_object.cpp PRE-CREATION > kjs/jsonstringify.h PRE-CREATION > kjs/jsonstringify.cpp PRE-CREATION > kjs/ustring.h 49370ef > > Diff: http://git.reviewboard.kde.org/r/105057/diff/ > > > Testing > ------- > > > Thanks, > > Bernd Buschinski > >
