harmless little fix

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

harmless little fix

Paul Licameli
So it appears there will be an RC3 and some low-risk fixes are getting committed to master.  May I nominate this, which just changes 1 to 1u in two places:

ce5cec30157b57466e63bc2471ea31702d55a0ac

PRL


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
audacity-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/audacity-devel
Reply | Threaded
Open this post in threaded view
|

Re: harmless little fix

Darrell Walisser
I have a couple of those. Valgrind found some uninitialized class members. The first one I already posted about. Not sure about the right starting value for mIsWE, but false/0 is probably what it was getting.

--- src/toolbars/ToolDock.cpp
+++ src/toolbars/ToolDock.cpp
@@ -356,6 +356,7 @@ ToolDock::ToolDock( ToolManager *manager, wxWindow *parent, int dockid ):
    // Init
    mManager = manager;
 
+   memset(mBars, 0, sizeof(mBars)); // otherwise uninitialized
    // Use for testing gaps
    // SetOwnBackgroundColour( wxColour( 255, 0, 0 ) );
 }

--- src/widgets/Ruler.cpp
+++ src/widgets/Ruler.cpp
@@ -1959,6 +1959,7 @@ AdornedRulerPanel::AdornedRulerPanel(AudacityProject* project,
    mCursorDefault = wxCursor(wxCURSOR_DEFAULT);
    mCursorHand = wxCursor(wxCURSOR_HAND);
    mCursorSizeWE = wxCursor(wxCURSOR_SIZEWE);
+   mIsWE = false; // was uninitialized member
 
    mLeftOffset = 0;
    mIndTime = -1;


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
audacity-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/audacity-devel
Reply | Threaded
Open this post in threaded view
|

Re: harmless little fix

James Crook
In reply to this post by Paul Licameli
Thanks.  Committed.  Looks very safe, and good.  Was there an associated
bug number?  Is it http://bugzilla.audacityteam.org/show_bug.cgi?id=608 
or is this an untracked bug?

Do you have others that you would like to get in?  If so let me know
today/tomorrow as I'll be saying 'no' much more as we get closer to RC3.

--James.

On 3/1/2017 9:25 PM, Paul Licameli wrote:
> So it appears there will be an RC3 and some low-risk fixes are getting
> committed to master.  May I nominate this, which just changes 1 to 1u in
> two places:
>
> ce5cec30157b57466e63bc2471ea31702d55a0ac
>
> PRL


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
audacity-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/audacity-devel
Reply | Threaded
Open this post in threaded view
|

Re: harmless little fix

James Crook
In reply to this post by Darrell Walisser
Thanks.  They're now in master  too.
https://github.com/audacity/audacity/commit/d1b49952e992819c599909b5546cfbbba8b3c574


On 3/1/2017 9:56 PM, Darrell Walisser wrote:
> I have a couple of those. Valgrind found some uninitialized class members.
> The first one I already posted about. Not sure about the right starting
> value for mIsWE, but false/0 is probably what it was getting.
false is correct because of arrow cursor being the default.



>
> --- src/toolbars/ToolDock.cpp
>
> +++ src/toolbars/ToolDock.cpp
>
> @@ -356,6 +356,7 @@ ToolDock::ToolDock( ToolManager *manager, wxWindow
> *parent, int dockid ):
>
>      // Init
>
>      mManager = manager;
>
>   +   memset(mBars, 0, sizeof(mBars)); // otherwise uninitialized
>
>      // Use for testing gaps
>
>      // SetOwnBackgroundColour( wxColour( 255, 0, 0 ) );
>
>   }
>
>
> --- src/widgets/Ruler.cpp
> +++ src/widgets/Ruler.cpp
> @@ -1959,6 +1959,7 @@
> AdornedRulerPanel::AdornedRulerPanel(AudacityProject* project,
>      mCursorDefault = wxCursor(wxCURSOR_DEFAULT);
>      mCursorHand = wxCursor(wxCURSOR_HAND);
>      mCursorSizeWE = wxCursor(wxCURSOR_SIZEWE);
> +   mIsWE = false; // was uninitialized member
>
>      mLeftOffset = 0;
>      mIndTime = -1;
>


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
audacity-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/audacity-devel
Reply | Threaded
Open this post in threaded view
|

Re: harmless little fix

Paul Licameli
In reply to this post by Paul Licameli
And to avoid this confusion this time, I suggest a cherry-pick not a merge!

PRL


On Wed, Mar 1, 2017 at 4:25 PM, Paul Licameli <[hidden email]> wrote:
So it appears there will be an RC3 and some low-risk fixes are getting committed to master.  May I nominate this, which just changes 1 to 1u in two places:

ce5cec30157b57466e63bc2471ea31702d55a0ac

PRL



------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
audacity-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/audacity-devel
Reply | Threaded
Open this post in threaded view
|

Re: harmless little fix

James Crook
Oh, a shame, so I can't take all the other cool stuff and pretend it was a mistake :-(

Yep.  I did cherry pick.  I knew better this time.  Was it 608?  If so one of us should update that bug.
(The description of 608 doesn't quite match what I'd expect from the lack of 'u', which is inversion and doubling of volume, not silence or near silence)

--James


On 3/2/2017 12:31 AM, Paul Licameli wrote:
And to avoid this confusion this time, I suggest a cherry-pick not a merge!

PRL


On Wed, Mar 1, 2017 at 4:25 PM, Paul Licameli [hidden email]
wrote:

So it appears there will be an RC3 and some low-risk fixes are getting
committed to master.  May I nominate this, which just changes 1 to 1u in
two places:

ce5cec30157b57466e63bc2471ea31702d55a0ac

PRL



      

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot


_______________________________________________
audacity-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/audacity-devel



------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
audacity-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/audacity-devel
Reply | Threaded
Open this post in threaded view
|

Re: harmless little fix

Gale
Administrator
I just closed #608 WORKSFORME. I can't reproduce it, and couldn't
in 2.1.2.

If I import a 24-bit PCM WAV file into 2.1.2 release using FFmpeg,
I don't see inversion compared to libsndfile import, nor in HEAD now.

Perhaps this broke after 2.1.2 with the upgrade to FFmpeg 3.x, or
perhaps I don't understand what the steps to reproduce were.


Gale


On 2 March 2017 at 09:56, James Crook <[hidden email]> wrote:

> Oh, a shame, so I can't take all the other cool stuff and pretend it was a
> mistake :-(
>
> Yep.  I did cherry pick.  I knew better this time.  Was it 608?  If so one
> of us should update that bug.
> (The description of 608 doesn't quite match what I'd expect from the lack of
> 'u', which is inversion and doubling of volume, not silence or near silence)
>
> --James
>
>
>
> On 3/2/2017 12:31 AM, Paul Licameli wrote:
>
> And to avoid this confusion this time, I suggest a cherry-pick not a merge!
>
> PRL
>
>
> On Wed, Mar 1, 2017 at 4:25 PM, Paul Licameli <[hidden email]>
> wrote:
>
> So it appears there will be an RC3 and some low-risk fixes are getting
> committed to master.  May I nominate this, which just changes 1 to 1u in
> two places:
>
> ce5cec30157b57466e63bc2471ea31702d55a0ac
>
> PRL
>
>
>
>
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
>
>
>
> _______________________________________________
> audacity-devel mailing list
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/audacity-devel
>
>
>
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
> _______________________________________________
> audacity-devel mailing list
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/audacity-devel
>

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
audacity-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/audacity-devel