Re: [Bluez-devel] [PATCH] Updated sendto

[Bastien Nocera <hadess@hadess.net>]

On Mon, 2008-02-25 at 21:36 +0100, Marcel Holtmann wrote:
> Hi Bastien,
> 
> > > So current patch is not acceptable. It is actually bad. So first  
> > > action must be to remove all these useless comments. An example is this:
> > > 
> > > +       /* Go into main loop */
> > >          gtk_main();
> > > 
> > > Put comments where the code is unclear and not were everybody knows  
> > > what it is doing. This is a perfect example of wrongly commenting code.
> > 
> > Done locally.
> 
> do you have an updated patch or do I have to do it by myself.

It was minimal changes, so I was waiting for response on the other
issues.

> > > Second of all, I am unhappy with all this usage of gtk_main_quit() in  
> > > various functions. Can we not just structure the code a lot more  
> > > cleaner to avoid multiple calls of it.
> > 
> > I don't understand what you mean there.
> 
> What I currently got from code review is that we simply call
> gtk_main_quit() instead of having a little bit better structured code.
> Just stopping the mainloop at more then 1 or 2 places doesn't seem right
> to me. Feel free to convince me otherwise.

All the uses of gtk_main_quit() could be "exit(0)" instead, but we want
to finish the mainloop and finish the cleaning up afterwards. It makes
debugging memory leaks easier, as the program can exit cleanly after
having mopped up after itself as hard as it could.

There's one instance of gtk_main_quit() that we could move (the one in
send_one shouldn't be needed, we should exit as soon as we know the
filelist is empty).

> > >  Besides the signal handling, I  
> > > would expect one extra call in case we automatically wanna close the  
> > > progress dialog.
> > 
> > Why? If there's no errors, why would you want to see it's finished? What
> > information would be in the dialogue that could be useful?
> 
> That behavior is fine with me. However that was not what I said. So I
> expect two gtk_main_quit() inside the code. One for SIGTERM and one when
> we are actually done with the transfer.

Do you just want us to exit() on error/user cancellation paths? It makes
no practical differences apart from the one mentioned above.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel