| Message ID | 20221021090126.28626-1-quic_rbhattac@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | wifi: ath11k: Fix qmi_msg_handler data structure initialization | expand |
On 10/21/22 2:01 AM, Rahul Bhattacharjee wrote: > }, > + {/* end of list */} > }; Do you want to add a comma after that last list element? Actually, I normally see the last list element simply being > + {}, ... with no comment necessary.
"Joseph S. Barrera III" <joebar@chromium.org> writes: > On 10/21/22 2:01 AM, Rahul Bhattacharjee wrote: > >> }, >> + {/* end of list */} >> }; > > Do you want to add a comma after that last list element? I can add that in the pending branch. > > Actually, I normally see the last list element simply being > >> + {}, > > ... with no comment necessary. I would prefer to have a comment to make it more visible that an empty element is needed at the end, but I would add that outside of braces? /* end of list */ {}, Thoughts? I can change this in the pending branch.
On 10/28/2022 4:14 PM, Kalle Valo wrote: > "Joseph S. Barrera III" <joebar@chromium.org> writes: > >> On 10/21/22 2:01 AM, Rahul Bhattacharjee wrote: >> >>> }, >>> + {/* end of list */} >>> }; >> Do you want to add a comma after that last list element? > I can add that in the pending branch. > >> Actually, I normally see the last list element simply being >> >>> + {}, >> ... with no comment necessary. > I would prefer to have a comment to make it more visible that an empty > element is needed at the end, but I would add that outside of braces? > > /* end of list */ > {}, > > Thoughts? I can change this in the pending branch. LGTM!
On 10/28/22 3:44 AM, Kalle Valo wrote: > "Joseph S. Barrera III" <joebar@chromium.org> writes: >> Do you want to add a comma after that last list element? > > I can add that in the pending branch. I think that would be good. > I would prefer to have a comment to make it more visible that an empty > element is needed at the end, but I would add that outside of braces? > > /* end of list */ > {}, Ending a list with {}, is such a common idiom that adding a comment is kind of just noise. And if you look at other such instances of ending with an empty element, you'll find no comment, just the {},. When making changes I prefer to stick to the existing coding style as much as possible, so in this case I would definitely omit the comment. But if you do feel like you need to add a comment, I would keep it on the same line as the empty element, probably like {}, // end of list
Rahul Bhattacharjee <quic_rbhattac@quicinc.com> wrote: > qmi_msg_handler is required to be null terminated by QMI module. > There might be a case where a handler for a msg id is not present in the > handlers array which can lead to infinite loop while searching the handler > and therefore out of bound access in qmi_invoke_handler(). > Hence update the initialization in qmi_msg_handler data structure. > > Tested-on: IPQ8074 hw2.0 AHB WLAN.HK.2.5.0.1-01100-QCAHKSWPL_SILICONZ-1 > > Signed-off-by: Rahul Bhattacharjee <quic_rbhattac@quicinc.com> > Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com> Patch applied to ath-next branch of ath.git, thanks. ed3725e15a15 wifi: ath11k: Fix qmi_msg_handler data structure initialization
diff --git a/drivers/net/wireless/ath/ath11k/qmi.c b/drivers/net/wireless/ath/ath11k/qmi.c index 145f20a681bd..bda4921208cc 100644 --- a/drivers/net/wireless/ath/ath11k/qmi.c +++ b/drivers/net/wireless/ath/ath11k/qmi.c @@ -3090,6 +3090,7 @@ static const struct qmi_msg_handler ath11k_qmi_msg_handlers[] = { sizeof(struct qmi_wlfw_fw_init_done_ind_msg_v01), .fn = ath11k_qmi_msg_fw_init_done_cb, }, + {/* end of list */} }; static int ath11k_qmi_ops_new_server(struct qmi_handle *qmi_hdl,
qmi_msg_handler is required to be null terminated by QMI module. There might be a case where a handler for a msg id is not present in the handlers array which can lead to infinite loop while searching the handler and therefore out of bound access in qmi_invoke_handler(). Hence update the initialization in qmi_msg_handler data structure. Tested-on: IPQ8074 hw2.0 AHB WLAN.HK.2.5.0.1-01100-QCAHKSWPL_SILICONZ-1 Signed-off-by: Rahul Bhattacharjee <quic_rbhattac@quicinc.com> --- drivers/net/wireless/ath/ath11k/qmi.c | 1 + 1 file changed, 1 insertion(+) base-commit: 087c436cbc8b1bf3d3bc7ea94d6757d74ea2f470