On 07/12/2018 11:37, Hans Verkuil wrote:
On 12/07/2018 12:31 PM, Mauro Carvalho Chehab wrote:
Em Fri, 7 Dec 2018 12:14:50 +0100
Hans Verkuil <hverkuil@xxxxxxxxx> escreveu:
On 12/07/2018 11:56 AM, Mauro Carvalho Chehab wrote:
A common mistake is to assume that initializing a var with:
struct foo f = { 0 };
Would initialize a zeroed struct. Actually, what this does is
to initialize the first element of the struct to zero.
According to C99 Standard 6.7.8.21:
"If there are fewer initializers in a brace-enclosed
list than there are elements or members of an aggregate,
or fewer characters in a string literal used to initialize
an array of known size than there are elements in the array,
the remainder of the aggregate shall be initialized implicitly
the same as objects that have static storage duration."
So, in practice, it could zero the entire struct, but, if the
first element is not an integer, it will produce warnings:
drivers/staging/media/sunxi/cedrus/cedrus.c:drivers/staging/media/sunxi/cedrus/cedrus.c:78:49: warning: Using plain integer as NULL pointer
drivers/staging/media/sunxi/cedrus/cedrus_dec.c:drivers/staging/media/sunxi/cedrus/cedrus_dec.c:29:35: warning: Using plain integer as NULL pointer
A proper way to initialize it with gcc is to use:
struct foo f = { };
But that seems to be a gcc extension. So, I decided to check upstream
No, this is not a gcc extension. It's part of the latest C standard.
Sure? Where the C standard spec states that? I've been seeking for
such info for a while, as '= {}' is also my personal preference.
I believe it was added in C11, but I've loaned the book that stated
that to someone else, so I can't check.
Sadly I don't see mention of empty initializer lists in section 6.7.9 of
ISO/IEC 9899:2011, though I've only got a draft version.
I had a play with Compiler Explorer[1] and it seems like clang is OK
with the {} form though just out of interest msvc isn't. I didn't try
exploring other command line options.
[1] https://gcc.godbolt.org/z/XIDC7W
Regards,
Ian
In any case, it's used everywhere:
git grep '= *{ *}' drivers/
So it is really pointless to *not* use it.
Regards,
Hans
I tried to build the Kernel with clang, just to be sure that this
won't cause issues with the clang support, but, unfortunately, the
clang compiler (not even the upstream version) is able to build
the upstream Kernel yet, as it lacks asm-goto support (there is an
OOT patch for llvm solving it - but it sounds too much effort for
my time to have to build llvm from their sources just for a simple
check like this).
It's used all over in the kernel, so please use {} instead of { NULL }.
Personally I think {} is a fantastic invention and I wish C++ had it as
well.
Fully agreed on that.
Regards,
Hans
what's the most commonly pattern. The gcc pattern has about 2000 entries:
$ git grep -E "=\s*\{\s*\}"|wc -l
1951
The standard-C compliant pattern has about 2500 entries:
$ git grep -E "=\s*\{\s*NULL\s*\}"|wc -l
137
$ git grep -E "=\s*\{\s*0\s*\}"|wc -l
2323
So, let's initialize those structs with:
= { NULL }
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@xxxxxxxxxx>
---
drivers/staging/media/sunxi/cedrus/cedrus.c | 2 +-
drivers/staging/media/sunxi/cedrus/cedrus_dec.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
index b538eb0321d8..44c45c687503 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
@@ -75,7 +75,7 @@ static int cedrus_init_ctrls(struct cedrus_dev *dev, struct cedrus_ctx *ctx)
memset(ctx->ctrls, 0, ctrl_size);
for (i = 0; i < CEDRUS_CONTROLS_COUNT; i++) {
- struct v4l2_ctrl_config cfg = { 0 };
+ struct v4l2_ctrl_config cfg = { NULL };
cfg.elem_size = cedrus_controls[i].elem_size;
cfg.id = cedrus_controls[i].id;
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
index e40180a33951..4099a42dba2d 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
@@ -26,7 +26,7 @@ void cedrus_device_run(void *priv)
{
struct cedrus_ctx *ctx = priv;
struct cedrus_dev *dev = ctx->dev;
- struct cedrus_run run = { 0 };
+ struct cedrus_run run = { NULL };
struct media_request *src_req;
unsigned long flags;
Thanks,
Mauro
