← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] SCH_LEGACY_PLUGIN::loadWire: fix parsing of wire color

 

Le 08/12/2018 à 22:38, Robert Abel a écrit :
> Wire color was parsed using parseInt alone.
> However, input contains a comma after every color component:
>  rgb(red, green, blue)
> 
> This comma wasn't skipped and subsequent calls to parseInt would
> return 0 without error while not advancing the line pointer.
> 
> By chance, red and green were always parsed successfully, because
> red is part of the initial token "rgb(red".
> 
> Blue was always read a 0 before.
> 
> Input files containing no commas were parsed successfully before.
> This patch keeps backwards compatiblity by not requiring commas to
> be present, but skipping exactly one comma if present.
> An error message is generated if parseInt does not advance the line
> pointer.

Thanks Robert.

However I committed a more basic patch.

> 
> Signed-off-by: Robert Abel <rabel@xxxxxxxxxxxxx>
> ---
>  eeschema/sch_legacy_plugin.cpp | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/eeschema/sch_legacy_plugin.cpp b/eeschema/sch_legacy_plugin.cpp
> index 8e9396df8..a34ecc9c8 100644
> --- a/eeschema/sch_legacy_plugin.cpp
> +++ b/eeschema/sch_legacy_plugin.cpp
> @@ -1222,8 +1222,11 @@ SCH_LINE* SCH_LEGACY_PLUGIN::loadWire( FILE_LINE_READER& aReader )
>              // and because there is no space between ( and 150
>              // the first param is inside buf.
>              // So break keyword and the first param into 2 separate strings.
> +            // Syntax:
> +            //     color ::= "rgb(" red [","] green [","] blue ")"
>              wxString prm, keyword;
>              keyword = buf.BeforeLast( '(', &prm );
> +            const char* lastLine = NULL;
>  
>              if( ( keyword == T_COLOR ) || ( keyword == T_COLORA ) )
>              {
> @@ -1241,8 +1244,19 @@ SCH_LINE* SCH_LEGACY_PLUGIN::loadWire( FILE_LINE_READER& aReader )
>                  // fix opacity to 1.0 or 255, when not exists in file
>                  color[3] = 255;
>  
> -                for(; ii < prm_count && !is_eol( *line ); ii++ )
> -                    color[ii] = parseInt( aReader, line, &line );
> +                for(; ii < prm_count && !is_eol( *line ); ii++ ) {
> +                    lastLine = line;
> +                    color[ii] = parseInt( aReader, line , &line);
> +                    if (line == lastLine)
> +                        SCH_PARSE_ERROR( "invalid wire color definition", aReader, lastLine );
> +                    // line points to ',' or ')' now
> +                    // whitespace following int was skipped by parseInt
> +                    if (*line == ',')
> +                        line++;
> +                    // skip rest of whitespace
> +                    while( *line && isspace( *line ))
> +                        line++;
> +                }
>  
>                  wire->SetLineColor( color[0]/255.0, color[1]/255.0, color[2]/255.0,color[3]/255.0 );
>              }
> 


-- 
Jean-Pierre CHARRAS


Follow ups

References