Re: [Bluez-devel] new sco flowcontrol patch

[Marcel Holtmann <marcel@holtmann.org>]

Hi Brad,

> > The biggest thing however is hci_sched_sco(). This function is ugly as
> > hell and basically unreadable. Please use break or continue instead of
> > cascaded if clauses. I am not gonna review this piece of the patch until
> > that is fixed.
> 
> I fixed a few whitespace problems and changed hci_sched_sco so it didn't 
> get deep into nested if's. How does it look now?

please also fix the other coding style issues. Some comments and a lot
of if clauses are violating the whitespace requirements. It is "if ("
and not "if(". It is also "(char *) &val" and not "(char *)&val".

And the code in sco_sock_queue_rcv_skb() is bad. We don't initialize
variables if not really needed. In this case if skb_queue_len() check
fails simply call return -ENOMEM. Strip the label and call return 0 at
the end of the function. Don't make the code more complex than it
actually is.

And no forward declaration of sco_sock_queue_rcv_skb() if not really
needed.

Why are we using SCO_TXBUFS and not reuse SO_SNDBUF since the value is
directly mapped to sk_sndbuf. Same applies to sk_rcvbuf.

And why do we have to do "c->tx_timer.function = hci_sco_tx_timer" over
and over again. Isn't it enough if we set the timer function once after
init of the timer.

Regards

Marcel



-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel