Remove locale-specific floating point from io

Use only C-locale functions wxString::ToCDouble and fmt::format{}
functions so that we do not need to set the locale for file io
operations

This also introduces a cmake check that looks for certain banned
functions (strtod, strtof and atof) that convert strings into doubles
by the user locale or formatting specifiers like %f/%g that convert
doubles into strings by the users locale.  It will prevent compilation
in this case.

For the limited cases where we want to show the user a string in their
locale, we have an override string "format:allow" that can be added to
lines in order to allow them with %f/%g format specifiers
This commit is contained in:
Seth Hillbrand 2025-07-14 09:40:42 -07:00
parent 60822714de
commit 55da0e46af
13 changed files with 166 additions and 41 deletions

View File

@ -0,0 +1,54 @@
# CheckBannedFunctions.cmake
# Usage: include(CheckBannedFunctions.cmake)
# This script scans for banned functions in io directories and fails the build if found.
# Set CMP0007 to NEW to ensure list variables are not stringified
cmake_policy(SET CMP0007 NEW)
# Usage: check_banned_functions() after setting BANNED_DIRS
macro(check_banned_functions)
set(BANNED_FUNCTIONS "strtod" "strtof" "atof")
set(BANNED_SCAN_FILES)
# Ensure BANNED_DIRS is set and not empty
if(NOT DEFINED BANNED_DIRS OR "${BANNED_DIRS}" STREQUAL "")
message(FATAL_ERROR "BANNED_DIRS variable is not set or empty.")
endif()
# Convert space-separated to semicolon-separated list
string(REPLACE " " ";" BANNED_DIRS "${BANNED_DIRS}")
foreach(dir IN LISTS BANNED_DIRS)
message(STATUS "Checking for banned functions in: ${dir}")
file(GLOB_RECURSE found_files
RELATIVE ${CMAKE_SOURCE_DIR}
${dir}/*.c
${dir}/*.cpp
${dir}/*.h
${dir}/*.hpp
)
list(APPEND BANNED_SCAN_FILES ${found_files})
endforeach()
foreach(file ${BANNED_SCAN_FILES})
# Read file into lines using file(STRINGS)
file(STRINGS ${CMAKE_SOURCE_DIR}/${file} file_lines)
foreach(line IN LISTS file_lines)
foreach(func ${BANNED_FUNCTIONS})
if(line MATCHES "\\b${func}\\b")
message(FATAL_ERROR "Banned function '${func}' found in ${file}")
endif()
endforeach()
# Only error if the line does not end with //format:allow (allow spaces between // and format)
# we allow this for specific cases where the format specifier is needed because the
# string is shown to the user and needs to be formatted in their locale e.g. error messages
if(line MATCHES "%\\d*\\.?\\d*[fg]" AND NOT line MATCHES "format:allow")
message(FATAL_ERROR "Banned format specifier '%f' or '%g' found in ${file}")
endif()
endforeach()
endforeach()
endmacro()
check_banned_functions()

View File

@ -47,6 +47,12 @@ add_custom_target(
COMMENT "Generating version string header"
)
add_custom_target(
banned_functions ALL
COMMAND ${CMAKE_COMMAND}
-DBANNED_DIRS="${PROJECT_SOURCE_DIR}/common/io;${PROJECT_SOURCE_DIR}/pcbnew/pcb_io;${PROJECT_SOURCE_DIR}/eeschema/sch_io"
-P ${KICAD_CMAKE_MODULE_PATH}/CheckBannedFunctions.cmake
)
# A shared library used by multiple *.kiface files and one or two program
# launchers. Object files can migrate into here over time, but only if they are
# surely needed and certainly used from more than one place without recompilation.

View File

@ -407,11 +407,30 @@ EROT Convert<EROT>( const wxString& aRot )
value.spin = aRot.find( 'S' ) != aRot.npos;
value.mirror = aRot.find( 'M' ) != aRot.npos;
value.degrees = strtod( aRot.c_str()
+ 1 // skip leading 'R'
+ int( value.spin ) // skip optional leading 'S'
+ int( value.mirror ), // skip optional leading 'M'
nullptr );
size_t rPos = aRot.find( 'R' );
if( rPos == wxString::npos )
{
value.degrees = 0.0;
return value;
}
// Calculate the offset after 'R', 'S', and 'M'
size_t offset = rPos + 1;
if( value.spin )
++offset;
if( value.mirror )
++offset;
wxString degreesStr = aRot.Mid( offset );
// Use locale-independent conversion
if( !degreesStr.ToCDouble( &value.degrees ) )
value.degrees = 0.0;
return value;
}
@ -504,7 +523,9 @@ VECTOR2I ConvertArcCenter( const VECTOR2I& aStart, const VECTOR2I& aEnd, double
if( !std::isnormal( dlen ) || !std::isnormal( aAngle ) )
{
THROW_IO_ERROR( wxString::Format( _( "Invalid Arc with radius %f and angle %f" ),
// Note that we allow the floating point output here because this message is displayed to the user and should
// be in their locale.
THROW_IO_ERROR( wxString::Format( _( "Invalid Arc with radius %0.2f and angle %0.2f" ), //format:allow
dlen,
aAngle ) );
}

View File

@ -469,7 +469,7 @@ void CADSTAR_SCH_ARCHIVE_LOADER::Load( SCHEMATIC* aSchematic, SCH_SHEET* aRootSh
return aNumber - error;
};
// When exporting to pdf, CADSTAR applies a margin of 3% of the longest dimension (height
// When exporting to pdf, CADSTAR applies a margin of 3percent of the longest dimension (height
// or width) to all 4 sides (top, bottom, left right). For the import, we are also rounding
// the margin to the nearest grid, ensuring all items remain on the grid.
VECTOR2I targetSheetSize = sheetBoundingBox.GetSize();
@ -538,8 +538,8 @@ void CADSTAR_SCH_ARCHIVE_LOADER::checkDesignLimits()
_( "The design is too large and cannot be imported into KiCad. \n"
"Please reduce the maximum design size in CADSTAR by navigating to: \n"
"Design Tab -> Properties -> Design Options -> Maximum Design Size. \n"
"Current Design size: %.2f, %.2f millimeters. \n"
"Maximum permitted design size: %.2f, %.2f millimeters.\n" ),
"Current Design size: %.2f, %.2f millimeters. \n" //format:allow
"Maximum permitted design size: %.2f, %.2f millimeters.\n" ), //format:allow
(double) designSizeXkicad / SCH_IU_PER_MM,
(double) designSizeYkicad / SCH_IU_PER_MM,
(double) maxDesignSizekicad / SCH_IU_PER_MM,
@ -2107,7 +2107,7 @@ SCH_SYMBOL* CADSTAR_SCH_ARCHIVE_LOADER::loadSchematicSymbol( const SYMBOL& aCads
if( test1.Normalize180() != test2.Normalize180() )
{
m_reporter->Report( wxString::Format( _( "Symbol '%s' is rotated by an angle of %.1f "
m_reporter->Report( wxString::Format( _( "Symbol '%s' is rotated by an angle of %.1f " //format:allow
"degrees in the original CADSTAR design but "
"KiCad only supports rotation angles multiples "
"of 90 degrees. The connecting wires will need "

View File

@ -34,6 +34,7 @@
#include <wx_filename.h> // For ::ResolvePossibleSymlinks()
#include <bitmap_base.h>
#include <fmt/format.h>
#include <kiway.h>
#include <string_utils.h>
#include <locale_io.h>
@ -1803,7 +1804,7 @@ void SCH_IO_KICAD_LEGACY::saveBitmap( const SCH_BITMAP& aBitmap )
m_out->Print( 0, "Pos %-4d %-4d\n",
schIUScale.IUToMils( aBitmap.GetPosition().x ),
schIUScale.IUToMils( aBitmap.GetPosition().y ) );
m_out->Print( 0, "Scale %f\n", refImage.GetImageScale() );
m_out->Print( "%s", fmt::format("Scale {:g}\n atof", refImage.GetImageScale()).c_str() );
m_out->Print( 0, "Data\n" );
wxMemoryOutputStream stream;

View File

@ -1856,8 +1856,8 @@ void SCH_IO_KICAD_LEGACY_LIB_CACHE::saveText( const SCH_TEXT* aText, OUTPUTFORMA
text = wxT( "\"" ) + text + wxT( "\"" );
}
aFormatter.Print( 0, "T %g %d %d %d %d %d %d %s",
(double) aText->GetTextAngle().AsTenthsOfADegree(),
aFormatter.Print( 0, "T %d %d %d %d %d %d %d %s",
aText->GetTextAngle().AsTenthsOfADegree(),
schIUScale.IUToMils( aText->GetTextPos().x ),
schIUScale.IUToMils( -aText->GetTextPos().y ),
schIUScale.IUToMils( aText->GetTextWidth() ),

View File

@ -22,6 +22,8 @@
#include <algorithm>
#include <fmt/format.h>
#include <wx/log.h>
#include <wx/mstream.h>
@ -951,7 +953,7 @@ void SCH_IO_KICAD_SEXPR::saveBitmap( const SCH_BITMAP& aBitmap )
scale = scale * 300.0 / bitmapBase.GetPPI();
if( scale != 1.0 )
m_out->Print( "(scale %g)", scale );
m_out->Print( "%s", fmt::format("(scale {:g})", refImage.GetImageScale()).c_str() );
KICAD_FORMAT::FormatUuid( m_out, aBitmap.m_Uuid );

View File

@ -19,7 +19,10 @@
* with this program. If not, see <http://www.gnu.org/licenses/>.
*/
#include <fmt/format.h>
#include <wx/log.h>
#include <base_units.h>
#include <build_version.h>
#include <sch_shape.h>
@ -426,7 +429,7 @@ void SCH_IO_KICAD_SEXPR_LIB_CACHE::saveField( SCH_FIELD* aField, OUTPUTFORMATTER
if( aField->IsMandatory() )
fieldName = GetCanonicalFieldName( aField->GetId() );
aFormatter.Print( "(property %s %s %s (at %s %s %g)",
aFormatter.Print( "(property %s %s %s (at %s %s %s)",
aField->IsPrivate() ? "private" : "",
aFormatter.Quotew( fieldName ).c_str(),
aFormatter.Quotew( aField->GetText() ).c_str(),
@ -434,7 +437,7 @@ void SCH_IO_KICAD_SEXPR_LIB_CACHE::saveField( SCH_FIELD* aField, OUTPUTFORMATTER
aField->GetPosition().x ).c_str(),
EDA_UNIT_UTILS::FormatInternalUnits( schIUScale,
-aField->GetPosition().y ).c_str(),
aField->GetTextAngle().AsDegrees() );
fmt::format( "{:g}", aField->GetTextAngle().AsDegrees() ).c_str() );
if( aField->IsNameShown() )
aFormatter.Print( "(show_name)" );
@ -508,14 +511,14 @@ void SCH_IO_KICAD_SEXPR_LIB_CACHE::saveText( SCH_TEXT* aText, OUTPUTFORMATTER& a
{
wxCHECK_RET( aText && aText->Type() == SCH_TEXT_T, "Invalid SCH_TEXT object." );
aFormatter.Print( "(text %s %s (at %s %s %g)",
aFormatter.Print( "(text %s %s (at %s %s %d)",
aText->IsPrivate() ? "private" : "",
aFormatter.Quotew( aText->GetText() ).c_str(),
EDA_UNIT_UTILS::FormatInternalUnits( schIUScale,
aText->GetPosition().x ).c_str(),
EDA_UNIT_UTILS::FormatInternalUnits( schIUScale,
-aText->GetPosition().y ).c_str(),
(double) aText->GetTextAngle().AsTenthsOfADegree() );
aText->GetTextAngle().AsTenthsOfADegree() );
aText->EDA_TEXT::Format( &aFormatter, 0 );
aFormatter.Print( ")" );

View File

@ -3373,12 +3373,12 @@ void ALTIUM_PCB::ConvertPads6ToFootprintItemOnCopper( FOOTPRINT* aFootprint, con
{
wxString msg;
msg.Printf( _( "Loading library '%s':\n"
"Footprint %s pad %s has a hole-rotation of %f degrees. "
"Footprint %s pad %s has a hole-rotation of %d degrees. "
"KiCad only supports 90 degree rotations." ),
m_library,
m_footprintName,
aElem.name,
slotRotation.AsDegrees() );
KiROUND( slotRotation.AsDegrees() ) );
m_reporter->Report( msg, RPT_SEVERITY_DEBUG );
}
}
@ -3387,11 +3387,11 @@ void ALTIUM_PCB::ConvertPads6ToFootprintItemOnCopper( FOOTPRINT* aFootprint, con
if( m_reporter )
{
wxString msg;
msg.Printf( _( "Footprint %s pad %s has a hole-rotation of %f degrees. "
msg.Printf( _( "Footprint %s pad %s has a hole-rotation of %d degrees. "
"KiCad only supports 90 degree rotations." ),
aFootprint->GetReference(),
aElem.name,
slotRotation.AsDegrees() );
KiROUND( slotRotation.AsDegrees() ) );
m_reporter->Report( msg, RPT_SEVERITY_DEBUG );
}
}

View File

@ -69,12 +69,14 @@ void CADSTAR_PCB_ARCHIVE_LOADER::Load( BOARD* aBoard, PROJECT* aProject )
if( designSizeXkicad > maxDesignSizekicad || designSizeYkicad > maxDesignSizekicad )
{
// Note that we allow the floating point output here because this message is displayed to the user and should
// be in their locale.
THROW_IO_ERROR( wxString::Format(
_( "The design is too large and cannot be imported into KiCad. \n"
"Please reduce the maximum design size in CADSTAR by navigating to: \n"
"Design Tab -> Properties -> Design Options -> Maximum Design Size. \n"
"Current Design size: %.2f, %.2f millimeters. \n"
"Maximum permitted design size: %.2f, %.2f millimeters.\n" ),
"Current Design size: %.2f, %.2f millimeters. \n" //format:allow
"Maximum permitted design size: %.2f, %.2f millimeters.\n" ), //format:allow
(double) designSizeXkicad / PCB_IU_PER_MM,
(double) designSizeYkicad / PCB_IU_PER_MM,
(double) maxDesignSizekicad / PCB_IU_PER_MM,
@ -2056,8 +2058,8 @@ void CADSTAR_PCB_ARCHIVE_LOADER::loadTemplates()
{
wxLogWarning( wxString::Format(
_( "The CADSTAR template '%s' has thermal reliefs in the original design "
"but the spoke width (%.2f mm) is thinner than the minimum thickness of "
"the zone (%.2f mm). KiCad requires the minimum thickness of the zone "
"but the spoke width (%.2f mm) is thinner than the minimum thickness of " //format:allow
"the zone (%.2f mm). KiCad requires the minimum thickness of the zone " //format:allow
"to be preserved. Therefore the minimum thickness has been applied as "
"the new spoke width and will be applied next time the zones are "
"filled." ),
@ -2597,7 +2599,7 @@ int CADSTAR_PCB_ARCHIVE_LOADER::loadNetVia(
{
wxLogError( _( "The CADSTAR via code '%s' has different shape from a circle defined. "
"KiCad only supports circular vias so this via type has been changed to "
"be a via with circular shape of %.2f mm diameter." ),
"be a via with circular shape of %.2f mm diameter." ), //format:allow
csViaCode.Name,
(double) ( (double) getKiCadLength( csViaCode.Shape.Size ) / 1E6 ) );
}
@ -3623,7 +3625,7 @@ void CADSTAR_PCB_ARCHIVE_LOADER::checkAndLogHatchCode( const HATCHCODE_ID& aCads
_( "The CADSTAR Hatching code '%s' has different line widths for each "
"hatch. KiCad only supports one width for the hatching. The imported "
"hatching uses the width defined in the first hatch definition, i.e. "
"%.2f mm." ),
"%.2f mm." ), //format:allow
hcode.Name,
(double) ( (double) getKiCadLength( hcode.Hatches.at( 0 ).LineWidth ) )
/ 1E6 ) );
@ -3635,7 +3637,7 @@ void CADSTAR_PCB_ARCHIVE_LOADER::checkAndLogHatchCode( const HATCHCODE_ID& aCads
_( "The CADSTAR Hatching code '%s' has different step sizes for each "
"hatch. KiCad only supports one step size for the hatching. The imported "
"hatching uses the step size defined in the first hatching definition, "
"i.e. %.2f mm." ),
"i.e. %.2f mm." ), //format:allow
hcode.Name,
(double) ( (double) getKiCadLength( hcode.Hatches.at( 0 ).Step ) )
/ 1E6 ) );
@ -3646,9 +3648,9 @@ void CADSTAR_PCB_ARCHIVE_LOADER::checkAndLogHatchCode( const HATCHCODE_ID& aCads
{
wxLogWarning( wxString::Format(
_( "The hatches in CADSTAR Hatching code '%s' have an angle "
"difference of %.1f degrees. KiCad only supports hatching 90 "
"difference of %.1f degrees. KiCad only supports hatching 90 " //format:allow
"degrees apart. The imported hatching has two hatches 90 "
"degrees apart, oriented %.1f degrees from horizontal." ),
"degrees apart, oriented %.1f degrees from horizontal." ), //format:allow
hcode.Name,
getAngle( abs( hcode.Hatches.at( 0 ).OrientAngle
- hcode.Hatches.at( 1 ).OrientAngle ) ).AsDegrees(),

View File

@ -1311,7 +1311,8 @@ FABMASTER::GRAPHIC_POLYGON* FABMASTER::processPolygon( const FABMASTER::GRAPHIC_
if( s.x != s.y )
{
wxLogError( _( "Expected x and y to be the same, got x = %f and y = %f " ), s.x, s.y );
wxLogDebug( "FABMASTER::processPolygon: Expected x and y to be the same, got x = %s and y = %s ",
aData.graphic_data3, aData.graphic_data4 );
}
auto new_poly = std::make_unique<GRAPHIC_POLYGON>();

View File

@ -1812,7 +1812,36 @@ void PCB_IO_KICAD_LEGACY::load3D( FOOTPRINT* aFootprint )
{
FP_3DMODEL t3D;
char* line;
// Lambda to parse three space-separated doubles using wxString::ToCDouble with C locale
auto parseThreeDoubles = []( const char* str, double& x, double& y, double& z ) -> bool
{
wxString wxStr( str );
wxStr.Trim( false ).Trim( true );
wxStringTokenizer tokenizer( wxStr, wxT( " \t" ), wxTOKEN_STRTOK );
if( !tokenizer.HasMoreTokens() )
return false;
wxString token1 = tokenizer.GetNextToken();
if( !token1.ToCDouble( &x ) || !tokenizer.HasMoreTokens() )
return false;
wxString token2 = tokenizer.GetNextToken();
if( !token2.ToCDouble( &y ) || !tokenizer.HasMoreTokens() )
return false;
wxString token3 = tokenizer.GetNextToken();
if( !token3.ToCDouble( &z ) )
return false;
return true;
};
char* line;
while( ( line = READLINE( m_reader ) ) != nullptr )
{
@ -1824,18 +1853,24 @@ void PCB_IO_KICAD_LEGACY::load3D( FOOTPRINT* aFootprint )
}
else if( TESTLINE( "Sc" ) ) // Scale
{
sscanf( line + SZ( "Sc" ), "%lf %lf %lf\n", &t3D.m_Scale.x, &t3D.m_Scale.y,
&t3D.m_Scale.z );
if (!parseThreeDoubles(line + SZ("Sc"), t3D.m_Scale.x, t3D.m_Scale.y, t3D.m_Scale.z))
{
THROW_IO_ERROR( wxT( "Invalid scale values in 3D model" ) );
}
}
else if( TESTLINE( "Of" ) ) // Offset
{
sscanf( line + SZ( "Of" ), "%lf %lf %lf\n", &t3D.m_Offset.x, &t3D.m_Offset.y,
&t3D.m_Offset.z );
if (!parseThreeDoubles(line + SZ("Of"), t3D.m_Offset.x, t3D.m_Offset.y, t3D.m_Offset.z))
{
THROW_IO_ERROR( wxT( "Invalid offset values in 3D model" ) );
}
}
else if( TESTLINE( "Ro" ) ) // Rotation
{
sscanf( line + SZ( "Ro" ), "%lf %lf %lf\n", &t3D.m_Rotation.x, &t3D.m_Rotation.y,
&t3D.m_Rotation.z );
if (!parseThreeDoubles(line + SZ("Ro"), t3D.m_Rotation.x, t3D.m_Rotation.y, t3D.m_Rotation.z))
{
THROW_IO_ERROR( wxT( "Invalid rotation values in 3D model" ) );
}
}
else if( TESTLINE( "$EndSHAPE3D" ) )
{

View File

@ -1113,7 +1113,7 @@ void PCB_IO_KICAD_SEXPR::format( const PCB_REFERENCE_IMAGE* aBitmap ) const
formatLayer( aBitmap->GetLayer() );
if( refImage.GetImageScale() != 1.0 )
m_out->Print( "(scale %g)", refImage.GetImageScale() );
m_out->Print( "%s", fmt::format("(scale {:g})", refImage.GetImageScale()).c_str() );
if( aBitmap->IsLocked() )
KICAD_FORMAT::FormatBool( m_out, "locked", true );
@ -1399,7 +1399,7 @@ void PCB_IO_KICAD_SEXPR::format( const FOOTPRINT* aFootprint ) const
KICAD_FORMAT::FormatBool( m_out, "hide", !bs3D->m_Show );
if( bs3D->m_Opacity != 1.0 )
m_out->Print( "(opacity %0.4f)", bs3D->m_Opacity );
m_out->Print( "%s", fmt::format("(opacity {:.4f})", bs3D->m_Opacity).c_str() );
m_out->Print( "(offset (xyz %s %s %s))",
FormatDouble2Str( bs3D->m_Offset.x ).c_str(),