ec_commands: Remove zero-size structs

The size of empty structs (and unions) varies between C and C++.  When
including in C++ code our external API in ec_commands.h header with
extern "C".  clang will complain (correctly) for all empty structs:
       error: empty struct has size 0 in C, size 1 in C++
       [-Werror,-Wextern-c-compat]

Remove them from the ec_commands.h header file.

ectool.c has some ugly macros which assume subcommands have both
requests and responses.  Change those macros so they only reference
the non-empty sub-structs.  The macros are still ugly, but generate
identical output, and don't rely upon zero-length structs.

BUG=chromium:792408
BRANCH=none
TEST=manual

	1) Compile the following using 'clang -Wall -Werror':

	   #include <stdint.h>
	   extern "C" {
	   #include "include/ec_commands.h"
	   }
	   int main(void) { return 0; }

	   It compiles without error.

	2) Copy the lb_command_paramcount, ms_command_sizes, and
	   cs_paramcount globals from ectool.c to a dummy .c file and
	   compile with 'gcc -S' to generate assembly.  Do the same
	   after applying this patch.

	   Confirm the arrays have the same contents.

Change-Id: Iad76f10315b97205b42118ce070463071fe97128
Signed-off-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/820649
Reviewed-by: Vincent Palatin <vpalatin@chromium.org>
Reviewed-by: Shawn N <shawnn@chromium.org>

3 files changed, 125 insertions, 119 deletions

diff --git a/common/motion_sense.c b/common/motion_sense.c
index 4e3711842b..51d1f4ec03 100644
--- a/common/motion_sense.c
+++ b/common/motion_sense.c
@@ -1375,7 +1375,7 @@ static int host_cmd_motion_sense(struct host_cmd_handler_args *args)
 		}
 		if (ret != EC_RES_SUCCESS)
 			return ret;
-		args->response_size = sizeof(out->set_activity);
+		args->response_size = 0;
 		break;
 	}
 #endif /* defined(CONFIG_GESTURE_HOST_DETECTION) */
diff --git a/include/ec_commands.h b/include/ec_commands.h
index cc53a4f314..3be0ac61c8 100644
--- a/include/ec_commands.h
+++ b/include/ec_commands.h
@@ -1856,13 +1856,17 @@ struct __ec_todo_unpacked lightbar_program {
 struct __ec_todo_packed ec_params_lightbar {
 	uint8_t cmd;		      /* Command (see enum lightbar_command) */
 	union {
-		struct __ec_todo_unpacked {
-			/* no args */
-		} dump, off, on, init, get_seq, get_params_v0, get_params_v1,
-			version, get_brightness, get_demo, suspend, resume,
-			get_params_v2_timing, get_params_v2_tap,
-			get_params_v2_osc, get_params_v2_bright,
-			get_params_v2_thlds, get_params_v2_colors;
+		/*
+		 * The following commands have no args:
+		 *
+		 * dump, off, on, init, get_seq, get_params_v0, get_params_v1,
+		 * version, get_brightness, get_demo, suspend, resume,
+		 * get_params_v2_timing, get_params_v2_tap, get_params_v2_osc,
+		 * get_params_v2_bright, get_params_v2_thlds,
+		 * get_params_v2_colors
+		 *
+		 * Don't use an empty struct, because C++ hates that.
+		 */
 
 		struct __ec_todo_unpacked {
 			uint8_t num;
@@ -1932,14 +1936,15 @@ struct __ec_todo_packed ec_response_lightbar {
 			uint8_t red, green, blue;
 		} get_rgb;
 
-		struct __ec_todo_unpacked {
-			/* no return params */
-		} off, on, init, set_brightness, seq, reg, set_rgb,
-			demo, set_params_v0, set_params_v1,
-			set_program, manual_suspend_ctrl, suspend, resume,
-			set_v2par_timing, set_v2par_tap,
-			set_v2par_osc, set_v2par_bright, set_v2par_thlds,
-			set_v2par_colors;
+		/*
+		 * The following commands have no response:
+		 *
+		 * off, on, init, set_brightness, seq, reg, set_rgb, demo,
+		 * set_params_v0, set_params_v1, set_program,
+		 * manual_suspend_ctrl, suspend, resume, set_v2par_timing,
+		 * set_v2par_tap, set_v2par_osc, set_v2par_bright,
+		 * set_v2par_thlds, set_v2par_colors
+		 */
 	};
 };
 
@@ -2446,8 +2451,7 @@ struct __ec_todo_packed ec_params_motion_sense {
 		} sensor_offset;
 
 		/* Used for MOTIONSENSE_CMD_FIFO_INFO */
-		struct __ec_todo_unpacked {
-		} fifo_info;
+		/* (no params) */
 
 		/* Used for MOTIONSENSE_CMD_FIFO_READ */
 		struct __ec_todo_unpacked {
@@ -2461,8 +2465,7 @@ struct __ec_todo_packed ec_params_motion_sense {
 		struct ec_motion_sense_activity set_activity;
 
 		/* Used for MOTIONSENSE_CMD_LID_ANGLE */
-		struct __ec_todo_unpacked {
-		} lid_angle;
+		/* (no params) */
 
 		/* Used for MOTIONSENSE_CMD_FIFO_INT_ENABLE */
 		struct __ec_todo_unpacked {
@@ -2571,8 +2574,7 @@ struct __ec_todo_packed ec_response_motion_sense {
 			uint32_t disabled;
 		} list_activities;
 
-		struct __ec_todo_unpacked {
-		} set_activity;
+		/* No params for set activity */
 
 		/* Used for MOTIONSENSE_CMD_LID_ANGLE */
 		struct __ec_todo_unpacked {
@@ -3746,9 +3748,7 @@ enum charge_state_params {
 struct __ec_todo_packed ec_params_charge_state {
 	uint8_t cmd;				/* enum charge_state_command */
 	union {
-		struct __ec_align1 {
-			/* no args */
-		} get_state;
+		/* get_state has no args */
 
 		struct __ec_todo_unpacked {
 			uint32_t param;		/* enum charge_state_param */
@@ -3774,9 +3774,8 @@ struct __ec_align4 ec_response_charge_state {
 		struct __ec_align4 {
 			uint32_t value;
 		} get_param;
-		struct __ec_align4 {
-			/* no return values */
-		} set_param;
+
+		/* set_param returns no args */
 	};
 };
 
@@ -3989,9 +3988,7 @@ struct __ec_align4 ec_params_sb_fw_update {
 		/* EC_SB_FW_UPDATE_END      = 0x4 */
 		/* EC_SB_FW_UPDATE_STATUS   = 0x5 */
 		/* EC_SB_FW_UPDATE_PROTECT  = 0x6 */
-		struct __ec_align4 {
-			/* no args */
-		} dummy;
+		/* Those have no args */
 
 		/* EC_SB_FW_UPDATE_WRITE    = 0x3 */
 		struct __ec_align4 {
diff --git a/util/ectool.c b/util/ectool.c
index eb4dd398b6..562a84e53a 100644
--- a/util/ectool.c
+++ b/util/ectool.c
@@ -2324,54 +2324,58 @@ static const char * const lightbar_cmds[] = {
 };
 #undef LBMSG
 
-/* This needs to match the values defined in lightbar.h. I'd like to
- * define this in one and only one place, but I can't think of a good way to do
- * that without adding bunch of complexity. This will do for now.
- */
-#define LB_SIZES(SUBCMD) { \
-		sizeof(((struct ec_params_lightbar *)0)->SUBCMD) \
-		+ sizeof(((struct ec_params_lightbar *)0)->cmd), \
-		sizeof(((struct ec_response_lightbar *)0)->SUBCMD) }
+/* Size of field <FLD> in structure <ST> */
+#define ST_FLD_SIZE(ST, FLD) sizeof(((struct ST *)0)->FLD)
+
+#define ST_CMD_SIZE ST_FLD_SIZE(ec_params_lightbar, cmd)
+#define ST_PRM_SIZE(SUBCMD) \
+	(ST_CMD_SIZE + ST_FLD_SIZE(ec_params_lightbar, SUBCMD))
+#define ST_RSP_SIZE(SUBCMD) ST_FLD_SIZE(ec_response_lightbar, SUBCMD)
+
 static const struct {
 	uint8_t insize;
 	uint8_t outsize;
 } lb_command_paramcount[] = {
-	LB_SIZES(dump),
-	LB_SIZES(off),
-	LB_SIZES(on),
-	LB_SIZES(init),
-	LB_SIZES(set_brightness),
-	LB_SIZES(seq),
-	LB_SIZES(reg),
-	LB_SIZES(set_rgb),
-	LB_SIZES(get_seq),
-	LB_SIZES(demo),
-	LB_SIZES(get_params_v0),
-	LB_SIZES(set_params_v0),
-	LB_SIZES(version),
-	LB_SIZES(get_brightness),
-	LB_SIZES(get_rgb),
-	LB_SIZES(get_demo),
-	LB_SIZES(get_params_v1),
-	LB_SIZES(set_params_v1),
-	LB_SIZES(set_program),
-	LB_SIZES(manual_suspend_ctrl),
-	LB_SIZES(suspend),
-	LB_SIZES(resume),
-	LB_SIZES(get_params_v2_timing),
-	LB_SIZES(set_v2par_timing),
-	LB_SIZES(get_params_v2_tap),
-	LB_SIZES(set_v2par_tap),
-	LB_SIZES(get_params_v2_osc),
-	LB_SIZES(set_v2par_osc),
-	LB_SIZES(get_params_v2_bright),
-	LB_SIZES(set_v2par_bright),
-	LB_SIZES(get_params_v2_thlds),
-	LB_SIZES(set_v2par_thlds),
-	LB_SIZES(get_params_v2_colors),
-	LB_SIZES(set_v2par_colors),
+	{ ST_CMD_SIZE, ST_RSP_SIZE(dump) },
+	{ ST_CMD_SIZE, 0 },
+	{ ST_CMD_SIZE, 0 },
+	{ ST_CMD_SIZE, 0 },
+	{ ST_PRM_SIZE(set_brightness), 0},
+	{ ST_PRM_SIZE(seq), 0},
+	{ ST_PRM_SIZE(reg), 0},
+	{ ST_PRM_SIZE(set_rgb), 0},
+	{ ST_CMD_SIZE, ST_RSP_SIZE(get_seq) },
+	{ ST_PRM_SIZE(demo), 0},
+	{ ST_CMD_SIZE, ST_RSP_SIZE(get_params_v0) },
+	{ ST_PRM_SIZE(set_params_v0), 0},
+	{ ST_CMD_SIZE, ST_RSP_SIZE(version) },
+	{ ST_CMD_SIZE, ST_RSP_SIZE(get_brightness) },
+	{ ST_PRM_SIZE(get_rgb), ST_RSP_SIZE(get_rgb) },
+	{ ST_CMD_SIZE, ST_RSP_SIZE(get_demo) },
+	{ ST_CMD_SIZE, ST_RSP_SIZE(get_params_v1) },
+	{ ST_PRM_SIZE(set_params_v1), 0},
+	{ ST_PRM_SIZE(set_program), 0},
+	{ ST_PRM_SIZE(manual_suspend_ctrl), 0},
+	{ ST_CMD_SIZE, 0 },
+	{ ST_CMD_SIZE, 0 },
+	{ ST_CMD_SIZE, ST_RSP_SIZE(get_params_v2_timing) },
+	{ ST_PRM_SIZE(set_v2par_timing), 0},
+	{ ST_CMD_SIZE, ST_RSP_SIZE(get_params_v2_tap) },
+	{ ST_PRM_SIZE(set_v2par_tap), 0},
+	{ ST_CMD_SIZE, ST_RSP_SIZE(get_params_v2_osc) },
+	{ ST_PRM_SIZE(set_v2par_osc), 0},
+	{ ST_CMD_SIZE, ST_RSP_SIZE(get_params_v2_bright) },
+	{ ST_PRM_SIZE(set_v2par_bright), 0},
+	{ ST_CMD_SIZE, ST_RSP_SIZE(get_params_v2_thlds) },
+	{ ST_PRM_SIZE(set_v2par_thlds), 0},
+	{ ST_CMD_SIZE, ST_RSP_SIZE(get_params_v2_colors) },
+	{ ST_PRM_SIZE(set_v2par_colors), 0},
 };
-#undef LB_SIZES
+BUILD_ASSERT(ARRAY_SIZE(lb_command_paramcount) == LIGHTBAR_NUM_CMDS);
+
+#undef ST_CMD_SIZE
+#undef ST_PRM_SIZE
+#undef ST_RSP_SIZE
 
 static int lb_help(const char *cmd)
 {
@@ -3567,53 +3571,55 @@ static int cmd_lightbar(int argc, char **argv)
 }
 
 /* Create an array to store sizes of motion sense param and response structs. */
-#define MS_SIZES(SUBCMD) { \
-		sizeof(((struct ec_params_motion_sense *)0)->SUBCMD) \
-		+ sizeof(((struct ec_params_motion_sense *)0)->cmd), \
-		sizeof(((struct ec_response_motion_sense *)0)->SUBCMD) }
+#define ST_CMD_SIZE ST_FLD_SIZE(ec_params_motion_sense, cmd)
+#define ST_PRM_SIZE(SUBCMD) \
+	(ST_CMD_SIZE + ST_FLD_SIZE(ec_params_motion_sense, SUBCMD))
+#define ST_RSP_SIZE(SUBCMD) ST_FLD_SIZE(ec_response_motion_sense, SUBCMD)
+#define ST_BOTH_SIZES(SUBCMD) { ST_PRM_SIZE(SUBCMD), ST_RSP_SIZE(SUBCMD) }
+
 /*
- * For ectool only, assume no more than 16 sensors.
- * More advanced implementation would allocate the right amount of
- * memory depending on the number of sensors.
+ * For ectool only, assume no more than 16 sensors.  More advanced
+ * implementation would allocate the right amount of memory depending on the
+ * number of sensors.
  */
 #define ECTOOL_MAX_SENSOR 16
-#define MS_DUMP_SIZE() { \
-		sizeof(((struct ec_params_motion_sense *)0)->dump) \
-		+ sizeof(((struct ec_params_motion_sense *)0)->cmd), \
-		sizeof(((struct ec_response_motion_sense *)0)->dump) \
-		+ sizeof(struct ec_response_motion_sensor_data) * \
-		  ECTOOL_MAX_SENSOR}
-
-#define MS_FIFO_INFO_SIZE() { \
-		sizeof(((struct ec_params_motion_sense *)0)->fifo_info) \
-		+ sizeof(((struct ec_params_motion_sense *)0)->cmd), \
-		sizeof(((struct ec_response_motion_sense *)0)->fifo_info) \
-		+ sizeof(uint16_t) * ECTOOL_MAX_SENSOR}
 
 static const struct {
 	uint8_t outsize;
 	uint8_t insize;
 } ms_command_sizes[] = {
-	MS_DUMP_SIZE(),
-	MS_SIZES(info_3),
-	MS_SIZES(ec_rate),
-	MS_SIZES(sensor_odr),
-	MS_SIZES(sensor_range),
-	MS_SIZES(kb_wake_angle),
-	MS_SIZES(data),
-	MS_FIFO_INFO_SIZE(),
-	MS_SIZES(fifo_flush),
-	MS_SIZES(fifo_read),
-	MS_SIZES(perform_calib),
-	MS_SIZES(sensor_offset),
-	MS_SIZES(list_activities),
-	MS_SIZES(set_activity),
-	MS_SIZES(lid_angle),
-	MS_SIZES(fifo_int_enable),
-	MS_SIZES(spoof),
+	{
+		ST_PRM_SIZE(dump),
+		ST_RSP_SIZE(dump) +
+		sizeof(struct ec_response_motion_sensor_data) *
+		ECTOOL_MAX_SENSOR
+	},
+	ST_BOTH_SIZES(info_3),
+	ST_BOTH_SIZES(ec_rate),
+	ST_BOTH_SIZES(sensor_odr),
+	ST_BOTH_SIZES(sensor_range),
+	ST_BOTH_SIZES(kb_wake_angle),
+	ST_BOTH_SIZES(data),
+	{
+		ST_CMD_SIZE,
+		ST_RSP_SIZE(fifo_info) + sizeof(uint16_t) * ECTOOL_MAX_SENSOR
+	},
+	ST_BOTH_SIZES(fifo_flush),
+	ST_BOTH_SIZES(fifo_read),
+	ST_BOTH_SIZES(perform_calib),
+	ST_BOTH_SIZES(sensor_offset),
+	ST_BOTH_SIZES(list_activities),
+	{ ST_PRM_SIZE(set_activity), 0 },
+	{ ST_CMD_SIZE, ST_RSP_SIZE(lid_angle) },
+	ST_BOTH_SIZES(fifo_int_enable),
+	ST_BOTH_SIZES(spoof),
 };
 BUILD_ASSERT(ARRAY_SIZE(ms_command_sizes) == MOTIONSENSE_NUM_CMDS);
-#undef MS_SIZES
+
+#undef ST_CMD_SIZE
+#undef ST_PRM_SIZE
+#undef ST_RSP_SIZE
+#undef ST_BOTH_SIZES
 
 static int ms_help(const char *cmd)
 {
@@ -5630,24 +5636,27 @@ int cmd_charge_control(int argc, char *argv[])
 }
 
 
+#define ST_CMD_SIZE ST_FLD_SIZE(ec_params_charge_state, cmd)
+#define ST_PRM_SIZE(SUBCMD) \
+	(ST_CMD_SIZE + ST_FLD_SIZE(ec_params_charge_state, SUBCMD))
+#define ST_RSP_SIZE(SUBCMD) ST_FLD_SIZE(ec_response_charge_state, SUBCMD)
 
 /* Table of subcommand sizes for EC_CMD_CHARGE_STATE */
-#define CB_SIZES(SUBCMD) { \
-		sizeof(((struct ec_params_charge_state *)0)->SUBCMD) \
-		+ sizeof(((struct ec_params_charge_state *)0)->cmd), \
-		sizeof(((struct ec_response_charge_state *)0)->SUBCMD) }
 static const struct {
 	uint8_t to_ec_size;
 	uint8_t from_ec_size;
 } cs_paramcount[] = {
 	/* Order must match enum charge_state_command */
-	CB_SIZES(get_state),
-	CB_SIZES(get_param),
-	CB_SIZES(set_param),
+	{ ST_CMD_SIZE, ST_RSP_SIZE(get_state) },
+	{ ST_PRM_SIZE(get_param), ST_RSP_SIZE(get_param) },
+	{ ST_PRM_SIZE(set_param), 0},
 };
-#undef CB_SIZES
 BUILD_ASSERT(ARRAY_SIZE(cs_paramcount) == CHARGE_STATE_NUM_CMDS);
 
+#undef ST_CMD_SIZE
+#undef ST_PRM_SIZE
+#undef ST_RSP_SIZE
+
 static int cs_do_cmd(struct ec_params_charge_state *to_ec,
 		     struct ec_response_charge_state *from_ec)
 {