kicad-developers team mailing list archive

[Robert Abel <rabel@xxxxxxxxxxxxx>]

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.

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 );
             }
-- 
2.17.1.windows.2