Re: [PATCH v2 4/7] qmi: Implement QMI service request rate limiting in 'can_write_data'.
Denis Kenzior <[email protected]> Fri, 14 Feb 2025 09:23:19 -0600
| Newsgroups | dev.linux.lists.ofono |
|---|---|
| Message-ID | <[email protected]> |
Hi Grant, > > There are two separate actions to be taken: > > 1. Performing the rate limit check before the request is dequeued and sent. > 2. Setting the last sent time after the request is dequeued and successfully sent. I'd rearrange the whole thing such that 2 isn't needed. If you update the last sent time in 1, the worst that can happen is that the write() will fail. Which will probably trigger a socket close shortly afterward anyway. > > So, there need to be two, separate conditional blocks for each. > Only if you want to deal with compiler warnings ;) >> Also, don't you need last_req_sent_time_us to be initialized to be non-zero somehow? Otherwise it is dumb luck that last_req_sent_time_us is set to anything the first time (probably garbage data) > > I thought about zero-initializing it in ‘qmi_qmux_device_new’; however, diving into the That is not what I'm talking about. l_new does indeed zero-initialize everything. Explicit init to 0 is not needed (or wanted). See my reply to v3. implementation of ‘l_new’, it appears as though all memory is zeroed out, so the zero-initialization seemed redundant. > Regards, -Denis