Re: [PATCH v2 4/7] qmi: Implement QMI service request rate limiting in 'can_write_data'.

[Denis Kenzior <denkenz@gmail.com>]

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