Hi Hendrik,

On Wed, Dec 08, 2010, Hendrik Sattler wrote:
> Zitat von "Johan Hedberg" <johan.hedb...@gmail.com>:
> > 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.

> > I also pushed a code/coding style cleanup patch on top of these, but to
> > be fair a lot of the cleanup (maybe even most) wasn't related to issues
> > introduced by your patches but existed in the code from before. While
> > going through the code I noticed that it could still use quite a lot of
> > refactoring in order to get rid of deeply nested if-statements, etc.
> > It's by far the most common cause of it being almost impossible to
> > adhere to the "max 79 columns" rule everywhere.
> 
> I don't like the style but I don't care enough, either.

No matter what the style is it should still be consistent throughout the
project.

I hope you at least agree with changes like:

* not having unnecessary whitespace at the end of lines (in fact your
  patches introduces some)

* if (foo) instead of if(foo)

*       if (foo) {
  instead of
        if (foo)
        {

* foo(); instead of foo ();

As well as getting rid of unnecessarily deep nesting, right?

> 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.

> 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.

Johan

------------------------------------------------------------------------------
What happens now with your Lotus Notes apps - do you make another costly 
upgrade, or settle for being marooned without product support? Time to move
off Lotus Notes and onto the cloud with Force.com, apps are easier to build,
use, and manage than apps on traditional platforms. Sign up for the Lotus 
Notes Migration Kit to learn more. http://p.sf.net/sfu/salesforce-d2d
_______________________________________________
Openobex-users mailing list
Openobex-users@lists.sourceforge.net
http://lists.sourceforge.net/lists/listinfo/openobex-users

Reply via email to