How to report code errors?

[Andrew Talbot]

How should one report minor code errors that don't necessarily produce known
bad behaviour? To give a concrete example, in msacm32:driver.c
acmDriverPriority(), formal parameter dwPriority is of type DWORD, which is
unsigned, but it contains a line that reads:

if (dwPriority < 0) dwPriority = -1;

... which can never happen.

Bugzilla seems inappropriate, so does wine-devel. If Coverity has abandoned
us, would it be worth having a web/wiki page for posting any such code
anomalies, so that the knowledgable might consider fixing them?

Thanks,

-- Andy.

[Alex Villací­s Lasso]

Andrew Talbot escribió:
> How should one report minor code errors that don't necessarily produce known
> bad behaviour? To give a concrete example, in msacm32:driver.c
> acmDriverPriority(), formal parameter dwPriority is of type DWORD, which is
> unsigned, but it contains a line that reads:
> if (dwPriority < 0) dwPriority = -1;
>
> ... which can never happen.
>
> Bugzilla seems inappropriate, so does wine-devel. If Coverity has abandoned
> us, would it be worth having a web/wiki page for posting any such code
> anomalies, so that the knowledgable might consider fixing them?
>
> Thanks,
>
> -- Andy.

From windowssdk.msdn.microsoft.com:
> /dwPriority/
>
> New priority for a global ACM driver identifier. A zero value 
> specifies that the priority of the driver identifier should remain 
> unchanged. A value of 1 specifies that the driver should be placed as 
> the highest search priority driver. A value of - 1 specifies that the 
> driver should be placed as the lowest search priority driver. 
> Priorities are used only for global drivers.

So -1 is actually a documented value for dwPriority. However, you are 
right in that a DWORD (a typedef for an unsigned long) won't ever return 
true for a comparison such as dwPriority < 0. I will prepare a patch 
shortly. The wine-devel list might be actually a good place to post code 
anomalies, as illustrated by the fact that it caught my attention 
(because I put that code in the first place :-) ).

Alex Villacís Lasso

[Andrew Talbot]

Alex Villací­s Lasso wrote:
> Andrew Talbot escribió:
> > How should one report minor code errors that don't necessarily produce
> > known bad behaviour? To give a concrete example, in msacm32:driver.c
> > acmDriverPriority(), formal parameter dwPriority is of type DWORD, which
> > is unsigned, but it contains a line that reads:
> > if (dwPriority < 0) dwPriority = -1;
> >
> > ... which can never happen.
> So -1 is actually a documented value for dwPriority. However, you are
> right in that a DWORD (a typedef for an unsigned long) won't ever return
> true for a comparison such as dwPriority < 0.

Yes, I didn't highlight where the problem really lay, there.
> I will prepare a patch 
> shortly. The wine-devel list might be actually a good place to post code
> anomalies, as illustrated by the fact that it caught my attention
> (because I put that code in the first place :-) ).
>
> Alex Villacís Lasso

Thank you for your kind and quick response, Alex.

-- Andy.

[Jan Zerebecki]

On Sat, Sep 16, 2006 at 04:41:04PM +0100, Andrew Talbot wrote:
> How should one report minor code errors that don't necessarily produce known
> bad behaviour?
> Bugzilla seems inappropriate, so does wine-devel. If Coverity has abandoned
> us, would it be worth having a web/wiki page for posting any such code
> anomalies, so that the knowledgable might consider fixing them?

Sending a proper patch to wine-patches seems to be the best way
to solve this. Most of the time it's faster to fix those little
bugs than to report them.

For questions about how to fix the problem -devel (or
#winehackers on freenode irc) seems to be the appropriate place.

If you are for whatever reason not willing to come up with a fix,
then I think a bugreport is appropriate if it is one specific
code error (if no one is interested in fixing this a message to
-devel will be forgotten). For generic things that occur all over
the code we have http://wiki.winehq.org/JanitorialProjects .

However to catch generic bugs it might be also good to extend our
smatch scripts (see http://wiki.winehq.org/TodoList about smatch)
to detect them (so we don't depend on Coverity for such stuff).

Jan

[Andrew Talbot]

Thanks, Jan. All good thoughts.

-- Andy.