On Fri, 14 Aug 2009 21:23:46 +0200, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
I don't like the approach taken in these two patches. You should leave the existing file_storage.c alone and create new source files for the composite version of the driver.
Do you think it's a better approach? My line of thinking was that there is no point in keeping two gadgets which do the same work and thus maintaining two copies of similar (but in many cases not identical) code. In particular, are there any big drawbacks of using composite framework? If no then IMO it'll be better to have all gadgets made using it (as then creating composite gadgets would be somehow trivial) instead of maintaining some gadgets in either of the two versions and some in both.
Also, you seem to have gone wild with configuration options.
Come to think of it, it may have gone out of hand. ;) I'll remove those in next patch.
IMO you should leave out the FILE_STORAGE_TEST config option entirely from the composite version.
If we were to keep the original FSG then I agree, however, personally, I'm still not convinced that's the better way to do this.
There were one or two other odd things. For example, what's the point of this business about making fsf_bufflen an alias so that it can't be assigned to? If you don't want it assigned to, then don't assign anything to it! If you like, declare it as "const" -- everybody will understand what that means. There's no need to obscure the code.
It could not be declared const because it was a module parameter and thus had to be modifiable. However, I didn't want to allow accidental assignments so constructed the macro in such a way that made assignment impossible. If FILE_STORAGE_TEST (or FSF_TEST) were to be removed altogether, this will be no longer necessary.
And on top of everything else, you based your work on an old version of file_storage.c. I hope you'll redo it based on the current version.
I did? O_o And all this time I thought I was working on code from 2.6.30. I'll look into it. Of course, I'll take into consideration all other comments. Thanks. -- Best regards, _ _ .o. | Liege of Serenly Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał "mina86" Nazarewicz (o o) ooo +-<m.nazarewicz@xxxxxxxxxxx>-<mina86@xxxxxxxxxx>-ooO--(_)--Ooo-- -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
