lpotter at trolltech.com
Sat Jul 26 22:52:38 CEST 2008
Holger Freyther wrote:
> On Saturday 26 July 2008 20:58:26 Lorn Potter wrote:
>>> My approach:
>>> - I do clean up the incosistent usage of 1, 3, 4 spaces mixed with tabs
>>> and placing braces and #ifdef's randomly. If you write code like that you
>>> want to hide something and the reason I have to touch this code is
>>> because there is something hiding.
>> We don't take patches for 'code cleanups'. There's a reason for this.
>> Customers get patches, and they don't like unnecessary lines of code
>> being changed. It can make it difficult to see what the real changes are.
> You should write clean code from the start. At WebKit.org we have clear Coding
> Style Guidelines and most of it is enforced by review and pre-commit scripts.
> Maybe adopting such things for Qtopia would make sense?
I wish you would stop using the word 'you' when you mean Trolltech.
'You' makes it seem like it is personal to me. I only work on one small
part of Qtopia, and direct people to view your commit log.
Maybe webkit.org isn't in the same position as us.
Perhaps you should stop caring if the code has differing white spaces
and get on with the real bugs.
>>> - I care little about BC, if I need to add a parameter to a method I do
>>> so, the commit messages state so though.
>> well, we do. or try to. Especially in a .x release mode.
> Sure. this is why I write this into the commit message...
and why we do not like to commit code that breaks it.
>>> - So I decided to solely rely on %CPI information for our code. One way
>>> would have been to just remove the timers from the libraries and be
>>> happy. If I did so you could have claimed that I don't think about other
>>> devices. So as I care about the consequences for other devices where
>>> timers might be needed I added methods to make starting of timers
>>> optional. So just because some modems are worse than the TI Calypso you
>>> shouldn't punish modems that are better....
>> So our customers buggy modems are just supposed to not work?
> Please reread the paragraph. As I said I made starting of timers optional. So
> what is the default? Default is still the old and error prone (from my point
> of view and every OpenSource/FreeSoftware engineer I have talked to)
It's a fine line between whats right, what's correct, and what's reality.
> First of all I doubt that you need to start any timers to guess when something
> is done. CLCC should always work (if %CPI or such vendor extensions do not
> exist). So instead of guessing when a call might or might not be missed, when
> all information arrived one could start your missed timer and then check the
> state of the call... So I think the "buggy modem" is just an excuse for
> writing fragile software.
This is what I was told by the lead comms guy. He has many real world
years experience (including large projects with large amounts of
engineers such as Qtopia) as a software developer, including open source
and I take his word for it.
> Basing everything on timers is IMO stupid but race free. You will present
> something consistent (but slightly wrong) to the user.
> Basing everything on %CPI is the right thing to do (if it is provided and
> reliable). You will present something consistent to the user.
> Mixing both will create funny and all sort of race conditions. You get a %CPI
> shortly after one of your timers fired...
> So with the patches I created you are able to disable the timer guessing
> completely. This fixed at least five different bugs QA reported. I left the
> default to be broken to keep your "customers" happy.
> Another funny race comes from the fact when a %CPI arrvies after sending a
> wakeup command and QAtChat just ignores these... E.g. the approach mickeyl
> has taken for FSO is to send a command that will never echo/print something
> so you don't need to throw away stuff from the modem in a certain window... a
> much saner approach to the same problem.
> And really you should not guess. If you send ATA to a modem it might fail.
> Just because you are going to queue the ATA does not mean that your call is
> connected. One has to wait for the response of the modem and can do the state
> transition after one knows which state the modem is in. Anything else is by
> any engineering standard error prone and just asking for random failures. A
> modem is a separate system, it runs software too, you should not guess about
> the internal state of this unknown modem firmware. You _have_ to wait for the
> modem to tell you the result of a command. So these class of issues with the
> code is not about style it is about engineering practices to create reliable
>> And again I fix this kind of things because it failed in reality..
>>> not because I have fun thinking about all the possible race conditions
>>> available in the Qtopia source.
>>> - I think the approach and implementation is reasonable. If you don't
>>> think so open up and start discussing it.
>> we do internally. we can't discuss this publicly because it involves
>> customers that do not want to be open/discussed or have their
>> competitors know what they are doing/what hardware/software bugs they
>> are seeing.
> So discussing my patches involves customers that are not interested in sharing
> improved versions with us?
I was meaning general internal discussions. People choose to view your
patches or not, depending on their interest. Many do not have time to
wade through 300 patches of code clean up for those few real bugs.
The fact is, Trolltech _is_ looking at your patches, regardless whether
any one person chooses to discuss them.
Lorn 'ljp' Potter
Software Engineer, Systems Group, Trolltech, a Nokia company
More information about the community