New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 792408 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

ec_commands.h header cannot be used with C++

Reported by vpalatin@chromium.org, Dec 6 2017

Issue description

When trying to use the ec_commands.h header in C++ code with
extern "C" {
#include "ec_commands.h"
}

clang doesn't like the empty structs, and produced 8 errors like this:
error: empty struct has size 0 in C, size 1 in C++ [-Werror,-Wextern-c-compat]

It seems which should be able to fix this with harmless changes to make it compatible.


ec_commands.h:1730:3: error: empty struct has size 0 in C, size 1 in C++ [-Werror,-Wextern-c-compat]
                struct __ec_todo_unpacked {
                ^
ec_commands.h:1806:3: error: empty struct has size 0 in C, size 1 in C++ [-Werror,-Wextern-c-compat]
                struct __ec_todo_unpacked {
                ^
ec_commands.h:2319:3: error: empty struct has size 0 in C, size 1 in C++ [-Werror,-Wextern-c-compat]
                struct __ec_todo_unpacked {
                ^
ec_commands.h:2334:3: error: empty struct has size 0 in C, size 1 in C++ [-Werror,-Wextern-c-compat]
                struct __ec_todo_unpacked {
                ^
ec_commands.h:2444:3: error: empty struct has size 0 in C, size 1 in C++ [-Werror,-Wextern-c-compat]
                struct __ec_todo_unpacked {
                ^
ec_commands.h:3530:3: error: empty struct has size 0 in C, size 1 in C++ [-Werror,-Wextern-c-compat]
                struct __ec_align1 {
                ^
ec_commands.h:3558:3: error: empty struct has size 0 in C, size 1 in C++ [-Werror,-Wextern-c-compat]
                struct __ec_align4 {
                ^
ec_commands.h:3763:3: error: empty struct has size 0 in C, size 1 in C++ [-Werror,-Wextern-c-compat]
                struct __ec_align4 {

 
it seems that just putting a dummy uint8_t should no impact on the structure layouts.
The only harm might be if one of the users have a sizeof() check on the struct member / transaction response.



Project Member

Comment 2 by bugdroid1@chromium.org, Dec 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/ec/+/7d2ce0c47e06622e2f423d33eec5ffceeeeb6a01

commit 7d2ce0c47e06622e2f423d33eec5ffceeeeb6a01
Author: Randall Spangler <rspangler@chromium.org>
Date: Wed Dec 13 20:33:37 2017

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>

[modify] https://crrev.com/7d2ce0c47e06622e2f423d33eec5ffceeeeb6a01/include/ec_commands.h
[modify] https://crrev.com/7d2ce0c47e06622e2f423d33eec5ffceeeeb6a01/common/motion_sense.c
[modify] https://crrev.com/7d2ce0c47e06622e2f423d33eec5ffceeeeb6a01/util/ectool.c

Owner: rspangler@chromium.org
Status: Fixed (was: Untriaged)

Sign in to add a comment