Code quality (was Re: comctl32: Fix invalid syntax)

[Bang Jun-young]

On 2/7/08, Michael Stefaniuc  wrote:
> Bang Jun-young wrote:
> > On 2/7/08, Alexandre Julliard  wrote:
> > > "Bang Jun-young"  writes:
> > > > 18 hours passed, and it looks like Alexandre decided to ignore this...why?
> > > >
> > > > This fix is required for Wine to be built with VC2005/2008 (although
> > > > Alexandre doesn't seem to care about it).
> > >
> > > There's no need to count the hours, or to ask every day, especially not
> > > with a mail to wine-patches. I have lots of pending patches from my
> > > vacation, I'll get to it eventually, and if I don't you should resubmit
> > > it after a week has passed.
> >
> > That's because you wanted yourself to be the only committer in the
> > project. Why don't you give commit privilege to other Wine
> > contributors as well?
>
> Huh? Why would you need that?

Because I'm concerned with the code quality of Wine. When Alexandre
commits what he doesn't understand, it's a red sign to the project.
Months ago I posted a patch similar to this one to wine-patches, and
he committed it:

typedef struct _OLEOBJECTVTBL {
-    void CALLBACK *(*QueryProtocol)(_LPOLEOBJECT,LPCOLESTR16);
+    void *         (CALLBACK *QueryProtocol)(_LPOLEOBJECT,LPCOLESTR16);

How did he know what the patch does from the same message "Fix invalid
syntax?" IIRC, he didn't tell anything about what he doesn't
understand.

In fact, this is a well known mistake many newbie Win32 developers
make (and fix in minutes). It shouldn't have been in the tree in the
first place if he actually have read the patch. There are a lot of
easily catchable bugs in the tree, for example, potential security
holes like buffer overrun, meaningless comparison of unsigned < 0 (or
> 0), misuse of BOOL vs. HRESULT, misuse of functions such as

strcasecmp(), use of non-portable syntax, etc. Most of them (if not
all) could be filtered out if he have read the patches carefully.

That's the main reason why Wine keeps crashing every time I give it a
try with my Windows apps. Since 1993, Wine has never gotten to the
point where everybody could rely on it for his daily work. It has as
awfully many bugs as Win95. I see something fundamentally wrong with
development process.
> Wine uses git, a distributed SCM. You are
> free to publish your own Wine tree without waiting for Alexandre.

99% of the world won't benefit from what I have done in my own local
repository. People usually don't have a much time to keep track of
repositories of hundreds of developers around the world. That's why
every open source project has the main repository even if developers
usually maintain their own repostories at home.
> People prefer to follow Alexandre's tree but nobody forces them to use it.

If winehq.org is Alexandre's own property, this is something
fundametally wrong again.

[Dmitry Timoshkov]

"Bang Jun-young"  wrote:
> > People prefer to follow Alexandre's tree but nobody forces them to use it.
>
> If winehq.org is Alexandre's own property, this is something
> fundametally wrong again.

You may try to learn how things work in the Wine project by reading
the following thread:

http://www.winehq.org/pipermail/wine-devel/2006-September/050915.html

and I'd suggest you very carefully read what Mike said:

http://www.winehq.org/pipermail/wine-devel/2006-September/051163.html

[Marcel Partap]

hi base,
> > If winehq.org is Alexandre's own property, this is something
> > fundametally wrong again.

Well.. then 'something' must be wrong with the linux kernel aswell.. how comes noone forked? hmmm..
> You may try to learn how things work in the Wine project by reading
> the following thread:
> http://www.winehq.org/pipermail/wine-devel/2006-September/050915.html

Although time consuming, it was rather a good read. While digesting the follow up 'governance ideas'
thread I had a little idea that might help, though I doubt the problems discussed at that time are
still that relevant. IMHO most of the wine-patches-black-hole problem is complexity of submitted
patches, making them hard to review and rather lead to those patches being silently skipped in favor
of the infinitly queuing later patches. One simple fix for this would be to recommend a maximum size
for diffs on the developer wiki (additionally to the recommendation to focus on a small context per
patch) and set up a wine-patches auto-reply for patches beyond that size, warning the submitter that
because of sheer size (=complexity), the patch is likely to be skipped because of merge pressure of
the following patches. That way, new contributors would get another visible warning to submit
patches that are easier to review.
Again, I can't estimate how much of an issue this is atm. I see a lot of patches commit or discussed
on -devel, so it might not be at all. Anyways..

Another thing: after my important exam in march, I will probably try setting up a drupal site and
see how it can be merged/ integrated with the wineforum, mailing lists and bugzilla.. there are some
cool modules and regexp power waiting to be explored :)

Last note: Jesse's DIB engine stuff, my work on printing aswell as Detlef's progress on sanitizing
the spooler will hopefully show some results, too. Jesse I'll discuss that stuff with you next month
be sure to be ready for some wine-patches resubmit cycles ;D
regards, marcel.

[Francois Gouget]

On Fri, 8 Feb 2008, Bang Jun-young wrote:
[...]
> In fact, this is a well known mistake many newbie Win32 developers
> make (and fix in minutes). It shouldn't have been in the tree in the
> first place if he actually have read the patch.

News flash!!!

You too are allowed to review patches posted to wine-patches and to 
point out their flaws. Some people do so regularly and thus help 
Alexandre. If bad patches are accepted it's partly your fault.

(and mine too cause I don't review patches enough)
> There are a lot of easily catchable bugs in the tree, for example, 
> potential security holes like buffer overrun, meaningless comparison 
> of unsigned < 0 (or > 0), misuse of BOOL vs. HRESULT, misuse of 
> functions such as strcasecmp(), use of non-portable syntax, etc.

You are welcome to review the Wine code and submit patches too. Again 
some others do so on a regular basis (me included this time :-). But if 
you don't want Wine to get better, then sure, just bitch about it.
> That's the main reason why Wine keeps crashing every time I give it a
> try with my Windows apps.

<sarcasm>
Riiiight. It must be the *main* cause of crashes. All the stub functions 
and unimplemented undocumented features must really be secondary causes 
of crashes in comparison.
</sarcasm>

[L. Rahyen]

On Friday February 8 2008 04:41:28 Bang Jun-young wrote:
> That's the main reason why Wine keeps crashing every time I give it a
> try with my Windows apps.
> ...
> I see something fundamentally wrong with  development process.

I think that current development process isn't a problem at all. In fact AJ 
is very good at what he is doing!
    As far as I understand your patch ("comctl32: Fix invalid syntax.") was 
rejected just because you forgot to add proper (descriptive) changelog entry.
    What is the real problem is the lack of testers (who report regressions and 
bugs) and developers. This is why WINE still has a lot of bugs, regressions 
are quite common thing to happen, etc. To fix this, more people should read 
wine-patches and test patches before they are committed, more people should 
write bug reports, more people should be involved in the development, and so 
on. Unfortunately, not all people have enough time for such tasks.
    However, WINE is pretty usable today.
> Since 1993, Wine has never gotten to the
> point where everybody could rely on it for his daily work. It has as
> awfully many bugs as Win95.

WINE can work reliably even with very complex programs (such as Photoshop 
CS - I use it pretty often). And if you don't see BIG improvement in last 
years you either tried very few Windows programs or you are very unlucky...
    In my practice WINE run most of the programs I try (well, I didn't tried 
thousands of Windows programs and there is no "hardcore" gamers in my family 
so my statistic may be biased). In fact, it is so good that I typically can 
rely on it to run any program I'm downloading from the Internet (success 
rate for me is more than 85% for "small" and "average" programs/games 
downloadable for free from the Internet). And if Windows program(s) work 
correctly on WINE, cases of "random" crashes are very rare. They (generally) 
do exist but most of Windows programs aren't affected by such bugs - much 
more often you find repeatable crashes after specific sequence(s) of steps.
    And your statement "Wine has never gotten to the point where everybody could 
rely on it for his daily work" is strange. In fact it is true for Windows too 
(Windows never gotten to the point where everybody could rely on it for his 
daily work). Maybe you just mean that WINE doesn't work well for you and some 
(or maybe even most) other people? But working well for some people or for 
nobody is very different things... There is a lot of people who use WINE for 
their daily tasks.
    For example, I and whole my family use Linux and WINE on daily basis (because 
of dependency on some Windows software and Windows games). There is no 
Windows installed on our computers (only I have Windows XP in VMWare for my 
very specific purposes to run Autodesk products).
    In my practice WINE and Linux are absolutely stable (if no bugs in WINE 
triggered by the program of course). In fact, I have trading station for 
Windows working 24 hours per day on my Linux server with WINE (if trading 
station fail or crash, I potentially can lose real money). And for this 
purpose (which by definition requires high stability) WINE+Linux works 
MUCH better than Windows XP.
    And your comparison of WINE with Windows 95 isn't true at all. Did you 
actually ever tried to use Windows 95? It will fail MUCH more often than 
WINE, and WINE can run more Windows programs than Windows 95 (I have it in 
VMWare so I really tested this with some programs year ago or so, "just for 
fun"). Even Windows 98 cannot run many important programs such as Photoshop 
(it require at least Windows 2000). And Windows 95/98 have a LOT of "random" 
crashes; WINE is much better - for many programs it can work for months 24 
hours per day without problems, and even if it crashes in some cases, other 
programs aren't affected (especially if they are launched from different 
prefix).
    Everything above is my personal experience, and for some users it may be 
worse. But for me, WINE work good enough for daily use, even for very 
important applications. So you can consider my story as 
yet-another-success-story-of-using-WINE.
    Obviously, this doesn't mean that WINE is good enough for everyone... But at 
least it is good enough for me, my family and some of my friends. There is 
some minor problems (for example, my brother have some games that don't work 
on WINE at all but he doesn't care very much about this) so WINE isn't 
perfect of course... But I just want to say that it is good enough for daily 
work and gaming at least for some people, and a lot of Windows software is 
usable on Linux with WINE.
    Personally, I think that AJ and all other WINE developers are doing very 
great and important work! Big thanks to all of them...

[Stefan Dösinger]

Am Samstag, 9. Februar 2008 02:13:34 schrieb L. Rahyen:
> On Friday February 8 2008 04:41:28 Bang Jun-young wrote:
> > That's the main reason why Wine keeps crashing every time I give it a
> > try with my Windows apps.
> > ...
> > I see something fundamentally wrong with  development process.
>
> I think that current development process isn't a problem at all. In fact
> AJ is very good at what he is doing!
>    As far as I understand your patch ("comctl32: Fix invalid syntax.") was
> rejected just because you forgot to add proper (descriptive) changelog
> entry. What is the real problem is the lack of testers (who report
> regressions and bugs) and developers. This is why WINE still has a lot of
> bugs, regressions are quite common thing to happen, etc. To fix this, more
> people should read wine-patches and test patches before they are committed,
> more people should write bug reports, more people should be involved in the
> development, and so on. Unfortunately, not all people have enough time for
> such tasks. However, WINE is pretty usable today.

I'd say we need more tests. Any patch that breaks a test from our testsuite 
when AJ runs them prior to committing is rejected