On Tue, 19 Oct 1999, Jeff Garzik wrote:

> Comments on your procfs patch:  I like it.  :)
> 
> It looks like many xxx_read_proc operation in fs/proc/proc_array.c
> contains the _exact_ same code for adjusting 'len', 'start', etc. 
> Cleanup should take care of that.
> 
> Putting that stuff into an inline or plain macro would save a lot of
> [source] code.  But I think proc_array.c can be made even smaller. 
> Maybe 80% of the functions could be condensed into something like:
> 
> DECLARE_SIMPLE_READ_PROC(filesystems_read_proc, get_filesystem_list)
> DECLARE_SIMPLE_READ_PROC(dma_read_proc, get_dma_list)

Yes, but it's _not_ a good thing. See comment in the beginning of
proc_misc.c - the thing you are talking about is the wrapper turning
->info-ish stuff into ->read_proc one. It's OK for the small things, but
for anything large... Look into drivers/pci/proc.c - I did a proper
conversion there and IMO the same should be done for the rest of them.
 
> Still other functions in proc_array look like they can be similarly
> simplified:
> 
> DECLARE_READ_PROC(ksyms_read_proc, get_ksyms_list)
> 
> IMHO it would be even better if proc_array didn't have to exist at all,
> and the /proc hook would be 100% declared in the module where
> get_xxx_list() exists.

I don't think so. proc_array is for per-process stuff. I've moved the rest
into proc_misc to have them together when the conversion to ->read_proc
will come. For anything that can be longer then one page get_foo_list() is
not an option - check the code and you will see.

There are several areas outside of proc that also need a cleanup - MCA and
SCSI stuff. Could you look at them?

Reply via email to