From: Hendrik S. <po...@he...> - 2010-12-08 12:31:55
|
Hi Johan, Zitat von "Johan Hedberg" <joh...@gm...>: > On Wed, Dec 08, 2010, Hendrik Sattler wrote: >> Zitat von "Johan Hedberg" <joh...@gm...>: >> > The patches have been pushed upstream. I didn't actually test them in >> > practice, so I hope you've done that thoroughly ;) >> >> I'm sure that there are some bugs left, I am not perfect, after all :) >> I tested with my obexpushd and obexftp, once with my USB AT modem >> emulation custom transport (obexftp -t /dev/ttyACM0) and with local >> TCP connection (obexftp -n 127.0.0.1). I haven't figured out how to >> get a Bluetooth local loop (that actually uses the bluetooth stack). >> Also using dummy_hcd.ko with g_serial use_acm=0 use_obex=1 randomly >> crashes, so I am not using it. > > We're gonna do a test package based on current git and run our usual > test set on it (the one we use internally at Nokia/MeeGo). obexd and > obex-client (also from the obexd package) are used for those. Hopefully > we'll catch any regressions. Great :) > I hope you at least agree with changes like: > > * not having unnecessary whitespace at the end of lines (in fact your > patches introduces some) Sorry, then. Sometimes, I forget to check with the emacs whitespace-mode. > * if (foo) instead of if(foo) Ack. > * if (foo) { > instead of > if (foo) > { I usually use the second case when the if condition spans more than one line. > * foo(); instead of foo (); Ack. > As well as getting rid of unnecessarily deep nesting, right? Ack. >> But please do NOT do the following: >> - int type = SOCK_STREAM; >> - int proto = 0; >> + int type = SOCK_STREAM, proto = 0; >> >> Please do not insist on some kernel coding style stupidities, I may as >> well insist on MISRA rules, then ;) > > Actually there's no kernel rule for this (at least I didn't manage to > find any). I was just following the usual convention (from my other > projects) of grouping same type variables together. But you're right, > in the case that assignment is done simultaneously to declaration it's > probably better to have them on separate lines. Actually in this case > the type variable isn't needed at all since the value could be used > directly in the subsequent socket call. It is done for the sake of one-time-definition. It's optimized away. >> The above syntax: >> * is usually done wrong then using pointer types > > This is really only a problem if you don't follow the coding style rule > of having the * part of the variable name without whitespace in between. Or if you use a bad types e.g. typedef struct { ... } *pFoo Windows has lots of bad types like this. We don't use that in openobex, though. HS |