← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] fix MANDATORY_FIELDS comparisons

 

Apologies for the delay, but here's the patch as an attachment.

On Thu, 9 Nov 2017, Wayne Stambaugh wrote:

Julius,

I cannot git am this patch.  Apparently, the extra info added by
launchpad is causing an issue.  Please resend this patch as an
attachment so I can get it merged.

Thanks,

Wayne

On 11/7/2017 4:31 AM, Julius Schmidt wrote:
I found a few places where SCH_FIELD::GetId() is incorrectly (signed)
compared to MANDATORY_FIELDS.
I'm not sure all of these are bugs but at least the sch_component.cpp
one triggers an assertion here when creating components with
non-mandatory fields defined.

---
 eeschema/class_libentry.cpp    | 2 +-
 eeschema/lib_field.cpp         | 2 +-
 eeschema/sch_component.cpp     | 2 +-
 eeschema/sch_legacy_plugin.cpp | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/eeschema/class_libentry.cpp b/eeschema/class_libentry.cpp
index cfe3d99..fe6f8e1 100644
--- a/eeschema/class_libentry.cpp
+++ b/eeschema/class_libentry.cpp
@@ -1096,7 +1096,7 @@ bool LIB_PART::LoadField( LINE_READER&
aLineReader, wxString& aErrorMsg )
         return false;
     }

-    if( field->GetId() < MANDATORY_FIELDS )
+    if( (unsigned) field->GetId() < MANDATORY_FIELDS )
     {
         LIB_FIELD* fixedField = GetField( field->GetId() );

diff --git a/eeschema/lib_field.cpp b/eeschema/lib_field.cpp
index 26237bd..35f74a1 100644
--- a/eeschema/lib_field.cpp
+++ b/eeschema/lib_field.cpp
@@ -264,7 +264,7 @@ bool LIB_FIELD::Load( LINE_READER& aLineReader,
wxString& errorMsg )
     }

     // fields in RAM must always have names.
-    if( m_id < MANDATORY_FIELDS )
+    if( (unsigned) m_id < MANDATORY_FIELDS )
     {
         // Fields in RAM must always have names, because we are trying
to get
         // less dependent on field ids and more dependent on names.
diff --git a/eeschema/sch_component.cpp b/eeschema/sch_component.cpp
index e03b077..d36cb55 100644
--- a/eeschema/sch_component.cpp
+++ b/eeschema/sch_component.cpp
@@ -874,7 +874,7 @@ void SCH_COMPONENT::UpdateFields( bool aResetStyle,
bool aResetRef )

             if( idx == REFERENCE && !aResetRef )
                 continue;
-            if( idx < MANDATORY_FIELDS )
+            if( (unsigned) idx < MANDATORY_FIELDS )
                 schField = GetField( idx );
             else
                 schField = FindField( field.GetName() );
diff --git a/eeschema/sch_legacy_plugin.cpp
b/eeschema/sch_legacy_plugin.cpp
index b7daee5..f70fdce 100644
--- a/eeschema/sch_legacy_plugin.cpp
+++ b/eeschema/sch_legacy_plugin.cpp
@@ -2657,7 +2657,7 @@ void SCH_LEGACY_PLUGIN_CACHE::loadField(
std::unique_ptr< LIB_PART >& aPart,
     }

     // Fields in RAM must always have names.
-    if( id < MANDATORY_FIELDS )
+    if( (unsigned) id < MANDATORY_FIELDS )
     {
         // Fields in RAM must always have names, because we are trying
to get
         // less dependent on field ids and more dependent on names.

_______________________________________________
Mailing list: https://launchpad.net/~kicad-developers
Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
Unsubscribe : https://launchpad.net/~kicad-developers
More help   : https://help.launchpad.net/ListHelp
From aefd5fd6b6a2aea67f05568f6271c39ac36877f0 Mon Sep 17 00:00:00 2001
From: Julius Schmidt <aiju@xxxxxxxxxx>
Date: Tue, 7 Nov 2017 09:18:08 +0000
Subject: [PATCH] fix MANDATORY_FIELDS comparisons (need to exclude -1)
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="------------2.10.2"

This is a multi-part message in MIME format.
--------------2.10.2
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit

---
 eeschema/class_libentry.cpp    | 2 +-
 eeschema/lib_field.cpp         | 2 +-
 eeschema/sch_component.cpp     | 2 +-
 eeschema/sch_legacy_plugin.cpp | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)


--------------2.10.2
Content-Type: text/x-patch; name="0001-fix-MANDATORY_FIELDS-comparisons-need-to-exclude-1.patch"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename="0001-fix-MANDATORY_FIELDS-comparisons-need-to-exclude-1.patch"

diff --git a/eeschema/class_libentry.cpp b/eeschema/class_libentry.cpp
index cfe3d99..fe6f8e1 100644
--- a/eeschema/class_libentry.cpp
+++ b/eeschema/class_libentry.cpp
@@ -1096,7 +1096,7 @@ bool LIB_PART::LoadField( LINE_READER& aLineReader, wxString& aErrorMsg )
         return false;
     }
 
-    if( field->GetId() < MANDATORY_FIELDS )
+    if( (unsigned) field->GetId() < MANDATORY_FIELDS )
     {
         LIB_FIELD* fixedField = GetField( field->GetId() );
 
diff --git a/eeschema/lib_field.cpp b/eeschema/lib_field.cpp
index 26237bd..35f74a1 100644
--- a/eeschema/lib_field.cpp
+++ b/eeschema/lib_field.cpp
@@ -264,7 +264,7 @@ bool LIB_FIELD::Load( LINE_READER& aLineReader, wxString& errorMsg )
     }
 
     // fields in RAM must always have names.
-    if( m_id < MANDATORY_FIELDS )
+    if( (unsigned) m_id < MANDATORY_FIELDS )
     {
         // Fields in RAM must always have names, because we are trying to get
         // less dependent on field ids and more dependent on names.
diff --git a/eeschema/sch_component.cpp b/eeschema/sch_component.cpp
index e03b077..d36cb55 100644
--- a/eeschema/sch_component.cpp
+++ b/eeschema/sch_component.cpp
@@ -874,7 +874,7 @@ void SCH_COMPONENT::UpdateFields( bool aResetStyle, bool aResetRef )
 
             if( idx == REFERENCE && !aResetRef )
                 continue;
-            if( idx < MANDATORY_FIELDS )
+            if( (unsigned) idx < MANDATORY_FIELDS )
                 schField = GetField( idx );
             else
                 schField = FindField( field.GetName() );
diff --git a/eeschema/sch_legacy_plugin.cpp b/eeschema/sch_legacy_plugin.cpp
index b7daee5..f70fdce 100644
--- a/eeschema/sch_legacy_plugin.cpp
+++ b/eeschema/sch_legacy_plugin.cpp
@@ -2657,7 +2657,7 @@ void SCH_LEGACY_PLUGIN_CACHE::loadField( std::unique_ptr< LIB_PART >& aPart,
     }
 
     // Fields in RAM must always have names.
-    if( id < MANDATORY_FIELDS )
+    if( (unsigned) id < MANDATORY_FIELDS )
     {
         // Fields in RAM must always have names, because we are trying to get
         // less dependent on field ids and more dependent on names.

--------------2.10.2--



Follow ups

References