Messages in this thread | | | From | Chaitanya Kulkarni <> | Subject | Re: [PATCH v2 0/2] remove workarounds for gcc bug wrt unnamed fields in initializers | Date | Wed, 24 Jun 2020 22:40:21 +0000 |
| |
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.
| |