← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] fix MANDATORY_FIELDS comparisons

 

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.


Follow ups

References