Re: [sigrok-devel] [PATCH] MainWindow, View::View: Fix two memory errors reported by valgrind (use of freed data, use of uninitialized variable)

[Soeren A. <so...@ap...>]

Hi Pettu,

thanks for the fixes, it's always nice if others help out =)

> diff --git a/pv/mainwindow.cpp b/pv/mainwindow.cpp
> index f7c4712..9e82fc8 100644
> --- a/pv/mainwindow.cpp
> +++ b/pv/mainwindow.cpp
> @@ -269,8 +269,8 @@ void
> MainWindow::remove_session(shared_ptr<Session> session)
>  
>  	for (shared_ptr<views::ViewBase> view : session->views()) {
>  		// Find the dock the view is contained in and remove
> it
> -		for (auto entry : view_docks_)
> -			if (entry.second == view) {
> +		for (auto it = view_docks_.begin(); it !=
> view_docks_.end(); )
> +			if (it->second == view) {
> 

I see the issue here now, nice catch! Instead of using an iterator
though, I would prefer a more straightforward approach: simply
break-ing from the inner loop when a match was found. There can be
only a single match at most, so there's no point continuing after
finding the only match. Would you mind supplying a patch that
does this instead?

 
> diff --git a/pv/view/view.cpp b/pv/view/view.cpp
> index a086d23..9751f00 100644
> --- a/pv/view/view.cpp
> +++ b/pv/view/view.cpp
> @@ -155,7 +155,8 @@ View::View(Session &session, QWidget *parent) :
>  	cursors_(new CursorPair(*this)),
>  	next_flag_text_('A'),
>  	trigger_markers_(),
> -	hover_point_(-1, -1)
> +	hover_point_(-1, -1),
> +	scroll_needs_defaults(false)
>  {
>  	connect(scrollarea_.horizontalScrollBar(),
> SIGNAL(valueChanged(int)),
>  		this, SLOT(h_scroll_value_changed(int)));


That's good to go in.

Feel free to report any other issues/errors you encounter :)

All the best,
 -Soeren