D16523: [Extractor] Replace homegrown IO handler with QDataStream, catch HUP

This revision was automatically updated to reflect the committed changes. Closed by commit R293:52107a8af3ee: [Extractor] Replace homegrown IO handler with QDataStream, catch HUP (authored by bruns). REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE

D16523: [Extractor] Replace homegrown IO handler with QDataStream, catch HUP

lbeltrame accepted this revision. This revision is now accepted and ready to land. REPOSITORY R293 Baloo BRANCH extractor REVISION DETAIL https://phabricator.kde.org/D16523 To: bruns, #baloo, #frameworks, ngraham, poboiko, lbeltrame Cc: lbeltrame, kde-frameworks-devel, ashaposhnikov,

D16523: [Extractor] Replace homegrown IO handler with QDataStream, catch HUP

bruns added a comment. good to go? REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D16523 To: bruns, #baloo, #frameworks, ngraham, poboiko Cc: lbeltrame, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams

D16523: [Extractor] Replace homegrown IO handler with QDataStream, catch HUP

bruns marked an inline comment as done. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D16523 To: bruns, #baloo, #frameworks, ngraham, poboiko Cc: lbeltrame, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams

D16523: [Extractor] Replace homegrown IO handler with QDataStream, catch HUP

bruns marked an inline comment as done. bruns added inline comments. INLINE COMMENTS > poboiko wrote in app.cpp:74 > I haven't work with `QDataStream` before, but shouldn't `m_inputStream >> > nIds` do the same? (and same below) > And probably you should avoid magical constants (4, 8, etc) -

D16523: [Extractor] Replace homegrown IO handler with QDataStream, catch HUP

bruns added a comment. In D16523#350888 , @poboiko wrote: > That's nice! I'll test it a little. This step keeps binary compatibility with the current streaming format. The big cleanup happens in the next commit (D16524

D16523: [Extractor] Replace homegrown IO handler with QDataStream, catch HUP

poboiko added a comment. That's nice! I'll test it a little. INLINE COMMENTS > app.cpp:74 > + > +memcpy(, buf, 4); > +for (quint32 i = 0; i < nIds; i++) { I haven't work with `QDataStream` before, but shouldn't `m_inputStream >> nIds` do the same? (and same below) And probably you

D16523: [Extractor] Replace homegrown IO handler with QDataStream, catch HUP

lbeltrame added a comment. In general (for my limited Baloo knowledge) this makes sense. You might want to add a few CCBUGs if you are aware of specific bugs this alleviates (@ngraham or someone from the bugsquad may help). REPOSITORY R293 Baloo REVISION DETAIL

D16523: [Extractor] Replace homegrown IO handler with QDataStream, catch HUP

bruns added a dependent revision: D16524: [Extractor] Use QDataStream serialization in place of cooked one. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D16523 To: bruns, #baloo, #frameworks, ngraham, poboiko Cc: kde-frameworks-devel, ashaposhnikov, michaelh,

D16523: [Extractor] Replace homegrown IO handler with QDataStream, catch HUP

bruns created this revision. bruns added reviewers: Baloo, Frameworks, ngraham, poboiko. Herald added projects: Frameworks, Baloo. Herald added a subscriber: kde-frameworks-devel. bruns requested review of this revision. REVISION SUMMARY The handler does not handle partial reads, and most