[PATCH 00/11] staging: comedi: cleanup asynchronous buffer functions

[Ian Abbott]

On 2013-01-08 23:22, H Hartley Sweeten wrote:
> On Thursday, January 03, 2013 5:26 AM, Ian Abbott wrote:
>> On 2012-12-21 16:36, H Hartley Sweeten wrote:
>>> The functions used to interact with the asynchronous buffer currently are
>>> located in the drivers.c source file. The only function drivers.c uses is
>>> comedi_buf_alloc().
>>>
>>> For aesthetic reasons and to and help maintainability, separate all the
>>> asynchronous buffer functions out to a new comedi_buf.c source file.
>>>
>>> Do a bit of cleanup of the comedi_buf_* functions to clarify them.
>>>
>>> H Hartley Sweeten (11):
>>>     staging: comedi: separate out comedi_buf_* functions
>>>     staging: comedi: comedi_buf: factor out common comedi_buf_write_alloc_* code
>>>     staging: comedi: comedi_buf: factor out common code to free the buf_page_list
>>>     staging: comedi: comedi_buf: factor out the new buffer allocation code
>>>     staging: comedi: comedi_buf: consolidate the buffer deallocation code
>>>     staging: comedi: comedi_buf: rename comedi_reset_async_buf()
>>>     staging: comedi: comedi_buf: clarify comedi_buf_write_free()
>>>     staging: comedi: comedi_buf: clarify comedi_buf_read_free()
>>>     staging: comedi: comedi_buf: clarify comedi_buf_read_alloc()
>>>     staging: comedi: comedi_buf: clarify __comedi_buf_write_alloc()
>>>     staging: comedi: comedi_buf: remove comedi_buf_write_alloc_strict()
>>>
>>>    drivers/staging/comedi/Makefile          |   3 +-
>>>    drivers/staging/comedi/comedi_buf.c      | 402 +++++++++++++++++++++++++++++++
>>>    drivers/staging/comedi/comedi_fops.c     |   4 +-
>>>    drivers/staging/comedi/comedi_internal.h |   3 +-
>>>    drivers/staging/comedi/comedidev.h       |   2 -
>>>    drivers/staging/comedi/drivers.c         | 398 ------------------------------
>>>    6 files changed, 408 insertions(+), 404 deletions(-)
>>>    create mode 100644 drivers/staging/comedi/comedi_buf.c
>>
>> I need to check more carefully, but my current view is that patches 01
>> to 06 and 11 are okay, but 07 to 10 are wrong as they've effectively
>> replaced signed comparisons with unsigned comparisons leading to
>> problems when the unsigned integers wrap around modulo 2^32.
>
> Ian,
>
> I just reposted this series based on next-20130108. I think I addressed
> all your previous concerns with the int/unsigned int usage.
>
> There could still be a modulo 2^32 issue but I think that problem also
> exists with the signed comparisons in the existing code. I think the
> changes in the new series (patches 8, 10, and 11) at least make the code
> a bit more understandable. If you still have issues with them we can drop
> patches 8-11 for now.

I think they're mostly okay apart from the barrier removal in patch 3 
(which I should have spotted the first time around).

(Also, being really nit-picky, I guess the comments should should really 
use the verb "ensure" instead of "insure", but that was a problem with 
the original comments too!)

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti at mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-