Re: [PATCH] Updated sendto

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.

lets use gtk_main_quit() since the exit code doesn't really matter and  
show me an updated patch. I am still working through the whole thing.  
We need some simplifications here and there, because the code looks  
too complicated to me.

Regards

Marcel


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