Bug 250086 - Filter valgrind errors in main window.
Summary: Filter valgrind errors in main window.
Status: RESOLVED UNMAINTAINED
Alias: None
Product: valkyrie
Classification: Unmaintained
Component: general (other bugs)
Version First Reported In: unspecified
Platform: Unlisted Binaries Linux
: HI normal
Target Milestone: ---
Assignee: Cerion Armour-Brown
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-03 22:12 UTC by Cerion Armour-Brown
Modified: 2018-09-04 16:05 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed/Implemented In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Cerion Armour-Brown 2010-09-03 22:12:20 UTC
Request from long ago:

Right now we can see (in situation with a real huge amount of errors) very loooooong error’s listing in main window – and sometimes to open each kind of found error from this list and make something useful in source code for removing this error – is quite difficult because this list is very looong.

BUT – If we had an opportunity to use Filter-system then we would reduce quantity of errors on the screen and the correction of mistakes would be more comfortable.

I suggest making filters based on the basic fields of the xml structures describing one error:
TAG <obj> - filters:[“equal”,”contains”, “starts with”, “ends with”]
TAG <fn> - filters:[“equal”,”contains”, “starts with”, “ends with”]
TAG <dir> - filters:[“equal”,”contains”, “starts with”, “ends with”]
TAG <file> - filters:[“equal”,”contains”, “starts with”, “ends with”]
And also – for more gobal tags like:
TAG <kind> - filters:[“equal”]  {here should be only predefined list of error types}
TAG <leakedbytes> - filters:[“equal”,”more that”, “less than”, “from .. till ..”]


CAB: If we do this, we must have a big clear warning saying
"WARNING: Filtering ON: Shown errors may be dependent on hidden errors“,
or something like that.
Comment 1 Cerion Armour-Brown 2010-09-03 22:29:46 UTC
forgot to mention: this one came from Anton Dudarenko
Comment 2 Cerion Armour-Brown 2011-07-03 11:39:04 UTC
Finally added a filter mechanism (repo r546)

Currently only a single filter line, let me know if this is enough, or if you need multiple filter lines (check it out, you'll see what I mean!)
Comment 3 adu 2011-07-04 11:30:52 UTC
HI! and First of all thanks a lot for such amazing result which you've produce ;)

but I do not understand why I cannot reopen this BZ (((
coz from my pt. of view it should be done due to:

1. ALL filters (as in MS Excel's auto-filter) should have all values as a simple variant for their using. Exactly as you did it for the 'Kind' filter. Hence I can OR simply choose one of these values as a filter's value(as I can now do this for  the 'Kind' filter) _OR_ (and this _new_ action should be implemented in code) EDIT in 'edit line'(part of the combo-box control) this chosen value - for ex. I can truncate it.
Especially this approach is a VERY useful for the 'Object' filter = coz it does not have visual representation in error's list-view for ALL items. Concerning that case I even want to say that this is also very strange - why some items have substring '(in /usr/lib...)' where "/usr/lib..." == tag's <obj> content - and some do not have it.

2. All Filters looks like are case-sensitive. -> should be case-INsensitive.

3. Some static info concerning used filter should be added:
at least:
'NN records from global MM was filtered'
'None matching records was found for current filter'
and maybe some thing else.

4. For 2 'Leaked' filters multi choice (filter lines) should be added. 
I mean - < and > sometimes should be used simultaneously. Today I can use them only in a separate manner. But for ex. I want to choose all leaks > 300 KB BUT < 1400KB.
The same for the 'Line' filter. 
btw. this filter can be by default turned off.
But if we want to use it - in tool's 'options' page we can make a choice: 'To use "Line" filter' with help of a new radio-button control.

5. Filter's Toolbar does not have tooltips at all. Very unfriendly IMHO.
6. Filter's Toolbar should have one additional button: 'Clear Filter'

And as a some very strange behavior I am considering this:
Open some error's item and 'stack' sub-item. By default short source paths will be used. Let's imagine that this is not correct right now - we want to see full paths -> so, just press corresponding button on a toolbar and ...
and in a listview you will see ".." at the end of each line - but not a full paths! To see full path you will have to close and open 'stack' sub-item forcibly. Only after these 2 actions you will be able to see full source paths.
Comment 4 Cerion Armour-Brown 2011-07-04 18:12:57 UTC
Anton, thx muchly for your quick feedback.

re your comments:
1a) I can imagine how that would be useful. Difference with 'Kind' is that the kinds are static. For the other filter types, the valgrind output would have to be scanned first to provide a list to choose from. The filter couldn't (obviously) be set before the Valgrind run. This is doable, just a question of time/effort.

1b) As for 'Object' not always being visible, if you take a look at the Valgrind XML output, I think you'll find that the <obj> tag does not exist in those cases - it's not always output from Valgrind.

2) If that's true, then I agree

3) Agreed - good as a warning, too.

4a) That would be an argument for multiple filter lines. This is doable. Will consider.

4b) Re turning 'Line' off, I'm not sure what you mean: if you don't _choose_ 'Line', then it's not 'on' anyway... why do you want an option to turn it off?
Actually, I find the separate 'dir/file/line' filter options questionable. I think they should be brought together to one filter option. At the moment all frames of a stack trace are tested against the filter, so if only _one_ frame somewhere in the stack has a corresponding line number, then then it will be a match. This is confusing, I think. Better to combine 'path/file:line' for clarity - what do you think?

5) Agreed, on the TODO list.

6) Agreed, on the TODO list.

Good to know about the 'show full src path' bug - please open another bug for that.

Thx again for the extensive feedback.
Am on holiday for the coming month, but will see if I can assign these issues to other contributors.
Cerion
Comment 5 adu 2011-07-05 06:21:51 UTC
Ok  - looks like we have a strong coop.sense of importance of this tool ;)

Concerning your answers:

1a) - I fully agree that in this case filter can be used only after some period of time (definitely much less than time of whole data's loading) AFTER the valgrind's execution. And ALL my current practice of work with the big number of valgrind's logs show that Filter'system I want (and looks like I can & I will) use ONLY after the moment when log will be parsed and visually presented in valkyrie's tree-view. So let's put it in TODO list ;)

1b) NO: 
in xml:
    <frame>
      <ip>0x5387EC3</ip>
      <obj>/usr/local/qt/qt-x11local-4.5.3-64-debug/lib/libQtGui.so.4.5.3</obj>
      <fn>qt_init(QApp*, int, _XDisplay*, unsigned long, unsigned long)</fn>
      <dir>/home/packages/qt-x11-opensource-src-4.5.3/src/gui/kernel</dir>
      <file>qapplication_x11.cpp</file>
      <line>1881</line>
    </frame>
in tree-view:
0x5387EC3 /usr/local/qt/qt-x11local-4.5.3-64-debug/lib/libQtGui.so.4.5.3 qt_init(QApp*, int, _XDisplay*, unsigned long, unsigned long) /home/packages/qt-x11-opensource-src-4.5.3/src/gui/kernelqapplication_x11.cpp 1881

so as you see - content of <obj> tag simply was not used in creation of single line in tree-view

2) - yes, I've tested more-or-less all filters - and found that they are case-sensitive ;(

3) Good)) if we agreed - let's add this in TODO list.

4a) OK, so if you agreed with multiple filter lines - let's add this in TODO list.

4b) Yep, looks like my english was not very nice and correct - but you've found the core of my "speech" ))) Yes, to combine 'path/file:line' is a better for clarity. Concerning 'turned off/on' possibilities: I wanted to say that by default NOT all filters can be accessible for using. For ex. we can "hide" "complex" filters like 'path/file:line' from user. But if he will insist to show and to use it - hence 'user! - you are welcome!'

ok if 5+6) already in TODO list - so that's great!

'show full src path' bug will be ASAP added in BZ.
Comment 6 adu 2011-07-05 09:29:28 UTC
new bug 277122 has been added to the database.
Comment 7 adu 2011-07-21 20:18:43 UTC
another bullet!
7) right now function LogViewFilterMC::compare_strings is saying that if _any_ of compared strings is matched str_flt then return true (don't hide).
That's not good.
For cmpfuntype=(FUN_EQL, FUN_CONT, FUN_STRT, FUN_END) I want to apply _any_ condition, but for cmpfuntype=(FUN_NEQL, FUN_NCONT, FUN_NSTRT, FUN_NEND) I *should&want* to apply _all_ condition.
see this new code:

/*!
  Compare strings.
  If _any_ of the strings in lst_xml for cmpfuntype=(FUN_EQL, FUN_CONT, FUN_STRT, FUN_END) 
    match str_flt, return true (don't hide)
  If _all_ of the strings in lst_xml for cmpfuntype=(FUN_NEQL, FUN_NCONT, FUN_NSTRT, FUN_NEND)
    match str_flt, return true (don't hide)
  In other case return false (hide)
*/
bool LogViewFilterMC::compare_strings( const QStringList& lst_xml,
                                        const QString& str_flt,
                                        CmpFunType cmpfuntype )
{
   bool res_cmp = true;//by def. - do not hide it!
   foreach ( QString str_xml, lst_xml ) {
      //vkDebug( "LogViewFilterMC::compare_strings(%d): '%s' - '%s'", cmpfuntype, qPrintable( str_xml ), qPrintable( str_flt ) );
      
      switch ( cmpfuntype ) {
      case FUN_EQL:   res_cmp = bool( str_xml == str_flt ); if (res_cmp) return true; break;
      case FUN_NEQL:  res_cmp &= bool( str_xml != str_flt ); break;
      case FUN_CONT:  res_cmp = bool( str_xml.contains( str_flt )); if (res_cmp) return true; break;
      case FUN_NCONT: res_cmp &= bool( !str_xml.contains( str_flt )); break;
      case FUN_STRT:  res_cmp = bool( str_xml.startsWith( str_flt )); if (res_cmp) return true; break;
      case FUN_NSTRT: res_cmp &= bool( !str_xml.startsWith( str_flt )); break;
      case FUN_END:   res_cmp = bool( str_xml.endsWith( str_flt )); if (res_cmp) return true; break;
      case FUN_NEND:  res_cmp &= bool( !str_xml.endsWith( str_flt )); break;
      default:
         vk_assert_never_reached();
      }
   }
   
   return res_cmp;
}
Comment 8 adu 2011-07-21 20:24:30 UTC
8) I understood that the multiple filters are become more and more important. Right now I am analyzing log with more than 2000 errors and for better view I would like to hide ALL errors where [obj] contains libX11 && libfontconfig && libexpat and etc. Hence all current filters should have an opportunity to turn on MULTIPLE mode.
Comment 9 adu 2011-07-22 21:50:16 UTC
for 7) bullet:
correction:

WAS:
   bool res_cmp = true;//by def. - do not hide it!
   foreach ( QString str_xml, lst_xml ) {

SHOULD BE:
   bool res_cmp = true;//by def. - do not hide it!
   
   if (lst_xml.size()==0)
     return false;
   
   foreach ( QString str_xml, lst_xml ) {
Comment 10 Andrew Crouthamel 2018-09-04 16:05:34 UTC
Hello! Sorry to be the bearer of bad news, but this project has been unmaintained for many years so I am closing this bug.