Hi!

> > This converts comments into C-style,
> 
> You missed a lot of them.

Yes, I know, at one point I realized you probably liked it that
way. Shall I resent just the "remove the initializer" part?

> What's the reason for this prejudice against C++-style comments in the
> kernel?  I've never understood it...  They seem so obviously useful,
> saving 3 characters of typing at the end of each comment line.

I've never seen a piece of non-crappy code using them... that's
probably it. They tend to come from C++ and C++ code tends to be ****.

> >  and removes unused
> > initializers. I'd like it applied...
> > 
> > On a related note -- how well does it work, particulary with different
> > hosts?
> 
> It has been quite reliable and it works with a wide variety of hosts.  The 
> main difficulties people have encountered have been related to the 
> gadget's USB controller driver (problems with halting bulk endpoints), 
> nothing to do with the host.

Thanks.

> >  It uses 'volatile' quite heavily, and that scares me a lot.
> 
> Alan Cox didn't like it either, but he wasn't able to give a clear 
> explanation why not.  He just mumbled something about C compilers 
> generating bad code for volatiles...

It probably does not what you want it to. For example:

static int fsg_setup(struct usb_gadget *gadget,
                const struct usb_ctrlrequest *ctrl)
{
        struct fsg_dev          *fsg = get_gadget_data(gadget);
        int                     rc;
        int                     w_length = le16_to_cpu(ctrl->wLength);

        ++fsg->ep0_req_tag;             // Record arrival of a new request
        fsg->ep0req->context = NULL;
        fsg->ep0req->length = 0;
        dump_msg(fsg, "ep0-setup", (u8 *) ctrl, sizeof(*ctrl));

Do you realize that it is okay for compiler to move assignment into
context before increase of req_tag? What you probably want is

        ++fsg->ep0_req_tag;             // Record arrival of a new request
        mb();
        fsg->ep0req->context = NULL;
        fsg->ep0req->length = 0;
        dump_msg(fsg, "ep0-setup", (u8 *) ctrl, sizeof(*ctrl));

...or something like that. Or perhaps atomic_inc(), that should have
some barriers built-in.

> Probably a lot of them could be eliminated; mainly they are there to mark
> quantities which can be changed by interrupt handlers.  I think all of the
> important usages are protected by a spinlock anyway.  There may be a few
> places that aren't; IIRC none of them really matter.

Ok. I hope above explanation helps.
                                                                Pavel
-- 
Thanks, Sharp!


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to