Re: [PATCH] cabinet: Turn some ERR into WARN and improved or fix messages.

[Nikolay Sivov]

On 11/27/2012 11:14, Christian Costa wrote:

---
  dlls/cabinet/fdi.c |   15 ++-
  1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/dlls/cabinet/fdi.c b/dlls/cabinet/fdi.c
index 0690b38..809a5c0 100644
--- a/dlls/cabinet/fdi.c
+++ b/dlls/cabinet/fdi.c
@@ -710,7 +710,7 @@ static BOOL FDI_read_entries(
  }
  
  /***

- * FDIIsCabinet (CABINET.21)
+ * FDIIsCabinet (CABINET.21)
   *
   * Informs the caller as to whether or not the provided file handle is
   * really a cabinet or not, filling out the provided PFDICABINETINFO
@@ -725,7 +725,7 @@ static BOOL FDI_read_entries(
   *   be filled out with information about the cabinet
   *   file indicated by hf if, indeed, it is determined
   *   to be a cabinet.
- *
+ *
   * RETURNS
   *   TRUE  if the file is a cabinet.  The info pointed to by pfdici will
   * be provided.
@@ -736,10 +736,7 @@ static BOOL FDI_read_entries(
   * INCLUDES
   *   fdi.c
   */
-BOOL __cdecl FDIIsCabinet(
-   HFDIhfdi,
-   INT_PTR hf,
-   PFDICABINETINFO pfdici)
+BOOL __cdecl FDIIsCabinet(HFDI hfdi, INT_PTR hf, PFDICABINETINFO pfdici)
  {
BOOL rv;
FDI_Int *fdi = get_fdi_ptr( hfdi );
@@ -749,13 +746,13 @@ BOOL __cdecl FDIIsCabinet(
if (!fdi) return FALSE;
  
if (!hf) {

-ERR((!hf)!\n);
+WARN(No file handle\n);
  SetLastError(ERROR_INVALID_HANDLE);
  return FALSE;
}
  
if (!pfdici) {

-ERR((!pfdici)!\n);
+WARN(No cab info pointer\n);
  SetLastError(ERROR_BAD_ARGUMENTS);
  return FALSE;
}
This call already has a TRACE earlier, I don't see a point in tracing 
input parameters multiple times.

@@ -2532,7 +2529,7 @@ BOOL __cdecl FDICopy(
  
/* check if it's really a cabfile. Note that this doesn't implement the bug */

if (!FDI_read_entries(fdi, cabhf, fdici, (CAB(mii {
-ERR(FDIIsCabinet failed: %u.\n, fdi-perf-erfOper);
+WARN(FDI_read_entries failed: %u\n, fdi-perf-erfOper);
  fdi-free(decomp_state);
  fdi-close(cabhf);
  return FALSE;

Re: winex11: Add a few 'fall through' comments.

[Dmitry Timoshkov]

Huw Davies h...@codeweavers.com wrote:

  case DirectColor:
   X11DRV_PALETTE_PaletteFlags |= X11DRV_PALETTE_VIRTUAL;
 -case GrayScale:
 +case GrayScale: /* fall through */

case DirectColor:
X11DRV_PALETTE_PaletteFlags |= X11DRV_PALETTE_VIRTUAL;
/* fall through */
case GrayScale:

would be more clear IMHO. Right now it's not obvious to which case statement
'fall through' belongs to.

Same for other changes.

-- 
Dmitry.

Re: winex11: Add a few 'fall through' comments.

[Huw Davies]

On Tue, Nov 27, 2012 at 06:44:35PM +0800, Dmitry Timoshkov wrote:
 case DirectColor:
   X11DRV_PALETTE_PaletteFlags |= X11DRV_PALETTE_VIRTUAL;
 /* fall through */
 case GrayScale:
 
 would be more clear IMHO. Right now it's not obvious to which case statement
 'fall through' belongs to.

Resent, thanks.

Huw.

Re: msvcp60: Remove superfluous semicolons

[Jacek Caban]

On 11/26/12 22:35, Andrew Talbot wrote:
 Jacek Caban wrote:

 It's probably better to change the macro to require the semicolon.

 Jacek
 The reason I did it that way was because there are two variants of the
 DEFINE_CXX_DATA macro, surrounded by an #ifndef construct: one comprising
 three struct declarations, all ending in semicolons, the other comprised
 three struct declarations (ending in semicolons) and one function definition
 (ending in a curly brace). I suppose I could move the function above the
 structs, but that would require forward declarations.

Ah, sorry, missed the function there. Both solution seem equally fine then.

Jacek

Re: [PATCH] cabinet: Turn some ERR into WARN and improved or fix messages.

[Christian Costa]

 -BOOL __cdecl FDIIsCabinet(
 -   HFDIhfdi,
 -   INT_PTR hf,
 -   PFDICABINETINFO pfdici)
 +BOOL __cdecl FDIIsCabinet(HFDI hfdi, INT_PTR hf, PFDICABINETINFO pfdici)
   {
 BOOL rv;
 FDI_Int *fdi = get_fdi_ptr( hfdi );
 @@ -749,13 +746,13 @@ BOOL __cdecl FDIIsCabinet(
 if (!fdi) return FALSE;
   if (!hf) {
 -ERR((!hf)!\n);
 +WARN(No file handle\n);
   SetLastError(ERROR_INVALID_**HANDLE);
   return FALSE;
 }
   if (!pfdici) {
 -ERR((!pfdici)!\n);
 +WARN(No cab info pointer\n);
   SetLastError(ERROR_BAD_**ARGUMENTS);
   return FALSE;
 }

 This call already has a TRACE earlier, I don't see a point in tracing
 input parameters multiple times.



Indeed. That would be better to remove them. Thanks.

Wine Gecko versioning

[Jacek Caban]

Hi all,

Since Wine Gecko 1.9 branching, the tip should bump to 1.10, but I think
it's time to clean up the situation a bit. Such things usually don't
bring much attention, but it should be decided in public. Right now we
have three versioning schemes involved in Wine Gecko:
- Wine version that applies to Gecko
- Gecko version (since Firefox 5 and Mozilla's rapid releases it's the
same as Firefox version)
- Wine Gecko version, which is just a growing number, not really
connected with any of above. These numbers currently have no meaning
other that being different for different Wine Gecko releases.

The idea is that Wine Gecko version could be just something based on
other versions, that are more informative. It's not really possible to
use Wine version, because the first version of Wine that will use new
Gecko is not ultimately when Wine Gecko branches. Also multiple Wine
versions use the same Wine Gecko. That leaves us with Gecko (Firefox
version). We don't release on every Firefox release (every 6 weeks), so
if we just used Firefox version, that would look strange (like Wine
Gecko 18 followed by Wine Gecko 20). That can be mitigated by using it
as a minor version. So the next few release would look like:
- 1.9 (that's already in beta and will be the last release using old scheme)
- 2.20 (assuming the next update will be 3 months from 1.9, which means
Firefox 20)
- 2.22 (assuming another 3 moths for the update).

Any suggestions/comments welcomed.

Cheers,
Jacek

Re: Wine Gecko versioning

[André Hentschel]

Am 27.11.2012 13:32, schrieb Jacek Caban:
 The idea is that Wine Gecko version could be just something based on
 other versions, that are more informative. It's not really possible to
 use Wine version, because the first version of Wine that will use new
 Gecko is not ultimately when Wine Gecko branches. Also multiple Wine
 versions use the same Wine Gecko. That leaves us with Gecko (Firefox
 version). We don't release on every Firefox release (every 6 weeks), so
 if we just used Firefox version, that would look strange (like Wine
 Gecko 18 followed by Wine Gecko 20). That can be mitigated by using it
 as a minor version. So the next few release would look like:
 - 1.9 (that's already in beta and will be the last release using old scheme)
 - 2.20 (assuming the next update will be 3 months from 1.9, which means
 Firefox 20)
 - 2.22 (assuming another 3 moths for the update).

I like the idea, but why move to 2.x? 1.20, 1.22 and so on would perfectly fit.
And if the gap between 1.9 and 1.20 is confusing someone, then he really has 
issues.
I'm  also fine with 2.x, i just think a new version scheme shouldn't 
necessarily lead to a version increment.

-- 

Best Regards, André Hentschel

Re: Wine Gecko versioning

[Rosanne DiMesio]

On Tue, 27 Nov 2012 13:32:02 +0100
Jacek Caban ja...@codeweavers.com wrote:

 The idea is that Wine Gecko version could be just something based on
 other versions, that are more informative. 
 
Users don't care what version of Firefox the latest gecko is based on; most 
don't even realize it is based on Firefox.  What does often confuse users, and 
which your suggested scheme doesn't address, is keeping track of which version 
of Wine uses which version of wine-gecko. 

-- 
Rosanne DiMesio dime...@earthlink.net

Re: Wine Gecko versioning

[Nikolay Sivov]

On 11/27/2012 17:26, Rosanne DiMesio wrote:

On Tue, 27 Nov 2012 13:32:02 +0100
Jacek Caban ja...@codeweavers.com wrote:


The idea is that Wine Gecko version could be just something based on
other versions, that are more informative.


Users don't care what version of Firefox the latest gecko is based on; most 
don't even realize it is based on Firefox.  What does often confuse users, and 
which your suggested scheme doesn't address, is keeping track of which version 
of Wine uses which version of wine-gecko.
It's described here http://wiki.winehq.org/Gecko. I guess failure 
message loading gecko (in load_gecko()) could be improved adding 
GECKO_VERSION to it so user clearly see what version he needs to have if 
it's somehow not downloaded already.

Re: Wine Gecko versioning

[Jacek Caban]

On 11/27/12 14:53, André Hentschel wrote:
 Am 27.11.2012 13:32, schrieb Jacek Caban:
 The idea is that Wine Gecko version could be just something based on
 other versions, that are more informative. It's not really possible to
 use Wine version, because the first version of Wine that will use new
 Gecko is not ultimately when Wine Gecko branches. Also multiple Wine
 versions use the same Wine Gecko. That leaves us with Gecko (Firefox
 version). We don't release on every Firefox release (every 6 weeks), so
 if we just used Firefox version, that would look strange (like Wine
 Gecko 18 followed by Wine Gecko 20). That can be mitigated by using it
 as a minor version. So the next few release would look like:
 - 1.9 (that's already in beta and will be the last release using old scheme)
 - 2.20 (assuming the next update will be 3 months from 1.9, which means
 Firefox 20)
 - 2.22 (assuming another 3 moths for the update).
 I like the idea, but why move to 2.x? 1.20, 1.22 and so on would perfectly 
 fit.
 And if the gap between 1.9 and 1.20 is confusing someone, then he really has 
 issues.
 I'm  also fine with 2.x, i just think a new version scheme shouldn't 
 necessarily lead to a version increment.

You're right, 1.20 would fit, but I also don't see anything wrong with
version increment. That said, I don't have strong preference here.

Thanks,
Jacek

Re: [PATCH] d3dx9_36/tests: Fix broken line test

[Matteo Bruni]

2012/11/27 Detlef Riekenberg wine@web.de:
 While inspecting a test failure, i found only
 3 possible results for the line test:
 - d3dx9_36.dll not present
 - all tests skipped
   line.c:146: Tests skipped: Failed to create IDirect3DDevice9 object 
 0x8876086c
 - refcount test failed
   line.c:113: Test failed: Got 5 references to device 0023EA60, expected 2
   (of course with different device pointer)

 See: http://test.winehq.org/data/tests/d3dx9_36:line.html

 I prefer to mark the refcount as implementation detail and remove the 
 refcount test.

 My second way to fix the test failure is the attached patch.
 Since the test was accepted to wine, there must be a machine somewhere
 with a refcount of 2, so i marked that result as broken.


I think the test was accepted simply because it passed on Wine while
it wasn't actually executed on WineTestBot. So yeah, removing the
refcount test or adding a todo_wine is probably the way to go, adding
a broken(ref == 2) doesn't seem to make much sense.

Thanks for bringing this up.


 d3d is not my area, so please comment.

 --
 By by ... Detlef
 ---
  dlls/d3dx9_36/tests/line.c |4 +++-
  1 files changed, 3 insertions(+), 1 deletions(-)

 diff --git a/dlls/d3dx9_36/tests/line.c b/dlls/d3dx9_36/tests/line.c
 index dc0c747..6ed508b 100644
 --- a/dlls/d3dx9_36/tests/line.c
 +++ b/dlls/d3dx9_36/tests/line.c
 @@ -110,7 +110,9 @@ static void test_create_line(IDirect3DDevice9* device)
  expect_mat(world, result);

  ref = IDirect3DDevice9_Release(return_device);
 -ok(ref == 2, Got %x references to device %p, expected 2\n, ref, 
 return_device);
 +todo_wine
 +ok(broken(ref == 2) || ref == 5,
 +Got %x references to device %p, expected 5\n, ref, return_device);

  ref = ID3DXLine_Release(line);
  ok(ref == 0, Got %x references to line %p, expected 0\n, ref, line);
 --
 1.7.5.4

Re: Wine Gecko versioning

[Jacek Caban]

On 11/27/12 16:33, Nikolay Sivov wrote:
 On 11/27/2012 17:26, Rosanne DiMesio wrote:
 On Tue, 27 Nov 2012 13:32:02 +0100
 Jacek Caban ja...@codeweavers.com wrote:

 The idea is that Wine Gecko version could be just something based on
 other versions, that are more informative.

 Users don't care what version of Firefox the latest gecko is based
 on; most don't even realize it is based on Firefox.  What does often
 confuse users, and which your suggested scheme doesn't address, is
 keeping track of which version of Wine uses which version of wine-gecko.

As Nikolay said, we have a Wiki page for that. It's not something we can
improve by different Gecko versioning.

 It's described here http://wiki.winehq.org/Gecko. I guess failure
 message loading gecko (in load_gecko()) could be improved adding
 GECKO_VERSION to it so user clearly see what version he needs to have
 if it's somehow not downloaded already.

Sounds like a good idea, although we'll probably want to fix it by
including version information in Gecko downloader.

Thanks,
Jacek

Re: Wine Gecko versioning

[Rosanne DiMesio]

On Tue, 27 Nov 2012 18:33:20 +0300
Nikolay Sivov bungleh...@gmail.com wrote:

 It's described here http://wiki.winehq.org/Gecko. I guess failure 
 message loading gecko (in load_gecko()) could be improved adding 
 GECKO_VERSION to it so user clearly see what version he needs to have if 
 it's somehow not downloaded already.
 
I know where the information is; in fact, I had to correct that page myself 
once because the information originally posted was wrong. 

Adding the version needed to the error message would definitely be helpful. 
-- 
Rosanne DiMesio dime...@earthlink.net