Hi Ted,
I'd like to reply to this comment in a recent commit (from TriggerSegment.cpp)
+ // ??? With GCC 6.2.0, this results in a warning:
+ // warning: nonnull argument ‘this’ compared to NULL
[-Wnonnull-compare]
+ // Assuming "this" is always non-null is overly optimistic. The
+ // following can happen:
+ // TriggerSegmentRec *p = NULL;
+ // p->ExpandInto(...);
+ // Defending against this possibility shouldn't be punished with a
+ // warning. In this case, the warning is wrong, and we should
+ // probably just suppress the warning until the compiler is fixed.
+ #pragma GCC diagnostic push
+ #pragma GCC diagnostic ignored "-Wnonnull-compare"
if (!this || !getSegment() || getSegment()->empty())
{ return false; }
+ #pragma GCC diagnostic pop
The compiler won't ever get "fixed", since it's correct: calling a method on a
null pointer is invalid.
Declaring this to be invalid allows compilers - and developers - to always know
that
"this" is never null, which removes the need for checking this in each and
every method
(that would be crazy overhead).
I added a check at the calling site instead, see patch attached.
(the other calling site is just after "new", and typically in Qt code we assume
new to succeed,
because on a desktop machine you'll have rebooted the machine before it even
gets to the
point where new returns null).
--
David Faure, [email protected], http://www.davidfaure.fr
Working on KDE Frameworks 5
diff --git i/src/base/TriggerSegment.cpp w/src/base/TriggerSegment.cpp
index d99ee98..43e9161 100644
--- i/src/base/TriggerSegment.cpp
+++ w/src/base/TriggerSegment.cpp
@@ -403,20 +403,9 @@ ExpandInto(Segment *target,
Segment *containing,
ControllerContextParams *controllerContextParams) const
{
- // ??? With GCC 6.2.0, this results in a warning:
- // warning: nonnull argument ‘this’ compared to NULL [-Wnonnull-compare]
- // Assuming "this" is always non-null is overly optimistic. The
- // following can happen:
- // TriggerSegmentRec *p = NULL;
- // p->ExpandInto(...);
- // Defending against this possibility shouldn't be punished with a
- // warning. In this case, the warning is wrong, and we should
- // probably just suppress the warning until the compiler is fixed.
- #pragma GCC diagnostic push
- #pragma GCC diagnostic ignored "-Wnonnull-compare"
- if (!this || !getSegment() || getSegment()->empty())
- { return false; }
- #pragma GCC diagnostic pop
+ if (!getSegment() || getSegment()->empty()) {
+ return false;
+ }
const int maxDepth = 10;
diff --git i/src/gui/seqmanager/InternalSegmentMapper.cpp w/src/gui/seqmanager/InternalSegmentMapper.cpp
index d0ecbc2..340908e 100644
--- i/src/gui/seqmanager/InternalSegmentMapper.cpp
+++ w/src/gui/seqmanager/InternalSegmentMapper.cpp
@@ -178,7 +178,7 @@ void InternalSegmentMapper::fillBuffer()
// Add triggered events into m_triggeredEvents.
// This invalidates `implied'.
- bool insertedSomething =
+ bool insertedSomething = rec &&
rec->ExpandInto(m_triggeredEvents,
j, m_segment, ¶ms);
if (insertedSomething) {
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Rosegarden-devel mailing list
[email protected] - use the link below to unsubscribe
https://lists.sourceforge.net/lists/listinfo/rosegarden-devel
