D28129: Read the new message string after rather than before

Harald Sitter noreply at phabricator.kde.org
Mon Mar 23 13:38:44 GMT 2020


sitter added a comment.


  Mh. Not quite what I had in mind but I suppose it makes sense this way. 
  I think we need a test case for the highlighter though :| It totally blows up in my face when I trace a running dolphin because toskip isn't quite right.

INLINE COMMENTS

> backtracegenerator.cpp:95
>  
> +    auto preamble = new QTemporaryFile;
> +    preamble->open();

This is leaking the file, is it not? It never deletes this object.

> gdbhighlighter.cpp:59
>      int lineNr = currentBlock().firstLineNumber();
> +    int toskip = 1 + m_infoLinesCount; // 1st line contains 'Application: ...'
>      while ( cur < text.length() ) {

lineNr is initialized to currentBlock().firstLineNumber() that is not necessarily 0, so toskip needs to be 
`lineNr + 1 + infocount` otherwise the skipping doesn't work as expected.

Should also be camel toSkip.

> gdbhighlighter.cpp:65
>          }
> -        if (lineNr == 0) {
> -            // line that contains 'Application: ...'
> +        if (lineNr <= toskip) {
>              ++lineNr;

Isn't this off-by-one versus the original code?
Say we have no infolines we'd then skip
[0] and [1] when previously we'd only skip [0].

> gdbhighlighter.cpp:76
> +        // toskip since we skip the first line and the info lines
> +        QMap< int, BacktraceLine >::iterator it = lines.lowerBound(lineNr - toskip);
>          Q_ASSERT(it != lines.end());

The assert below fails when I trace a running dolphin, I am not super sure why but I am guessing it's because the toskip init being bugged vis a vis the lineNr being an offset.

REPOSITORY
  R871 DrKonqi

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D28129

To: apol, #frameworks, broulik, sitter
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20200323/677af610/attachment.html>


More information about the Plasma-devel mailing list