Re: Zaptel Pseudo TDM Bus

[Francois Romieu <romieu@cogenit.fr>]

Greetings,

Mark Spencer <markster@linux-support.net> :
[...]
> 	Over the past year, Linux Support Services, Inc.  and Zapata
> Telephony, Inc. have been working together on building the "Zaptel" pseudo
> TDM bus architecture, and having at least 7 supported boards in a variety
> of roles (T1, E1, multi-port T1, E1, FXS and FXO with USB, PCI, ISA, and
> Ethernet interfaces), we are now interesting in getting comments on the
> driver architecture and moving towards integration into the 2.5 kernel.

A few remarks after a quick glance at zaptel-0.2.0 
- copy_{to/from}_user() may sleep. Don't call it with spinlock held
  (see Documentation/DocBook/kernel-locking.tmpl)
- avoid unchecked copy_{to/from}_user() and friends.
- the failure paths aren't consistent: sometime goto, sometime not.
- the failure paths don't always do their job regarding kmalloc/kfree balance
- fcstab[] declaration is duplicated
- please put blank lines after variables declarations in function body
- zt_common_ioctl/zt_ctl_ioctl() on less than 14/24 screens would be nice

[...]
> 	I am very interested in seeking comments both on our driver
> framework, and on how to go about submitting this for kernel inclusion if
> appropriate.

The kernel parts are appropriate imho (hint, hint: split these from userspace
code in zaptel package). Cosmetic/CodingStyle issues apart, bugs are lurking
in the code.

-- 
Ueimor