← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] Warnings cleanup

 

Hi,

On 04.07.2015 12:53, Camille 019 wrote:

> -            if( layer == NO_AVAILABLE_LAYERS )
> +            if( layer == static_cast<int>(NO_AVAILABLE_LAYERS) )

Nope, that relies on undefined overflow semantics, and compilers that do
constant propagation through casts would miscompile this code.

The correct way to solve this is the patch from Chris Pavlina:

--- a/include/layers_id_colors_and_visibility.h
+++ b/include/layers_id_colors_and_visibility.h
@@ -55,7 +55,7 @@ typedef int     LAYER_NUM;
  */
 enum LAYER_ID
 #if __cplusplus >= 201103L
-    : unsigned char
+    : signed char
 #endif
 {
     F_Cu,           // 0
@@ -116,12 +116,12 @@ enum LAYER_ID
     B_Fab,
     F_Fab,

-    LAYER_ID_COUNT
-};
+    LAYER_ID_COUNT,

+    UNDEFINED_LAYER = -1,
+    UNSELECTED_LAYER = -2
+};

-#define UNDEFINED_LAYER     LAYER_ID(-1)
-#define UNSELECTED_LAYER    LAYER_ID(-2)

 #define MAX_CU_LAYERS       (B_Cu - F_Cu + 1)


By including the negative values in the enum, the compiler is forced to
use an underlying data type that can represent them.

The alternative would be to remove the special cases from the enum, and
create a separate variable for validity of layer selection in those
parts of the program that need it.

   Simon

Attachment: signature.asc
Description: OpenPGP digital signature


References