Re: [PATCH v2 0/2] remove workarounds for gcc bug wrt unnamed fields in initializers

From: Chaitanya Kulkarni
Date: Wed Jun 24 2020 - 18:40:26 EST


Christoph, Sagi and Keith,

On 6/24/20 9:44 AM, Christoph Hellwig wrote:
> This looks good to me, but I'd rather wait a few releases to
> avoid too mush backporting pain.
>

Here is a summary, for longer explanation please have a look at the
end [1] :-

Pros:
1. Code looks uniform and follows strict policy.

Cons:
1. Adds a tab + more char [1] which can lead to line breaks and that can
be avoided without following declare-init pattern, less bugs and
no pressure to fit the initializer in ~72 char given that we do have
some long names and who knows what is in the future.
2. Issue with older version can lead to adding additional braces which
does not look good.
3. Writing a new code becomes inflexible and pressure to fit initializer
will not allow users to use meaningful names in the nested structures
and anon unions.
4. Future patches will be needed for backward compatibility.

Also code is perfectly readable as it is so why change ?

If everyone is okay with above cons I'm fine adding this.

Regards,
Chaitanya

[1] Explanation :-

I'm not against unifying the code. This will enforce struct
initialization to be done at the time of declaration and is inflexible
given that we have different transports and meaningful structure names.
Also, no one knows how many new structures will be coming since protocol
still has a room for improvement.

Consider following :-

e.g. 1

static void nvme_xxx_func()
{
struct nvme_XXX_YYY_ZZZ abcde {
.member1 = line of the initializer calculation = X,
.member2 = AAAAAAAAAAAAA + BBBBBBBBB + CCCCCC + DDDD +
EEEEE,
.member3 = AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAa,
}
}

e.g. 2

From code :-
+ struct rdma_conn_param param = {
+ .rnr_retry_count = 7,
+ .flow_control = 1,
+ .initiator_depth = min_t(u8, p->initiator_depth,
+ here ->>>queue->dev->device->attrs.max_qp_init_rd_atom),
+ .private_data = &priv,
+ .private_data_len = sizeof(priv),
+ };

In above case (e.g.1, e.g.2) we loose 8 character = 1 tab for every
declaration-initialization, now if we have a member to be initialized
with complex calculations then it comes down to the next line and again
we loose 8 char of tab + (number of characters = name) of the member
(member2 in nvme_xx_func()) and whole things looks ugly, in contrast if
we do it outside of the declaration we still get 8 more characters
before we reach line limit. With 80 char limit we should avoid line
breaks and tabs as and when possible, this policy goes against it.