Re: mono progress on mixed-mode assemblies...

2008-05-15 Thread Robert Shearman
Kornél Pál wrote:
 Also note that Mono's Class Library is licensed under MIT/X11 because 
 inlining (done by the runtime) may be incompatible with GPL that would not 
 allow non-GPL programs to be executed within Mono. Would it be possible to 
 have a MIT/X11 licensed msvcrt?

I'm not sure if you realise it, but Wine is licensed under the LGPL, not 
the GPL so I don't think using Wine's msvcrt code would be a problem 
with inlining and using non-GPL programs.

However, I understand that having a uniform license for Mono's Class 
Library is probably highly desirable.

-- 
Rob Shearman





Re: includes: Add vmr9 header

2008-05-13 Thread Robert Shearman
Maarten Lankhorst wrote:
 @@ -0,0 +1,525 @@
 +/*
 + * Copyright 2008 Maarten Lankhorst
 + *
 + * This library is free software; you can redistribute it and/or
 + * modify it under the terms of the GNU Lesser General Public
 + * License as published by the Free Software Foundation; either
 + * version 2.1 of the License, or (at your option) any later version.
 + *
 + * This library is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
 + * Lesser General Public License for more details.
 + *
 + * You should have received a copy of the GNU Lesser General Public
 + * License along with this library; if not, write to the Free Software
 + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
 + */
 +
 +#include unknwn.idl

You should use import here, not #include.

-- 
Rob Shearman





Re: [PATCH] Cache localised number chars

2008-05-13 Thread Robert Shearman
Michael Karcher wrote:
 UDATE *lpUdate)
  static void VARIANT_GetLocalisedNumberChars(VARIANT_NUMBER_CHARS *lpChars, 
 LCID lcid, DWORD dwFlags)
  {
static const VARIANT_NUMBER_CHARS defaultChars = { 
 '-','+','.',',','$',0,'.',',' };
 +  static VARIANT_NUMBER_CHARS lastChars;
 +  static LCID lastLcid = -1;
 +  static DWORD lastFlags = 0;
LCTYPE lctype = dwFlags  LOCALE_NOUSEROVERRIDE;
WCHAR buff[4];
  
 +  /* Asking for default locale entries is very expensive: It is a registry
 + server call. So cache one localy, as Microsoft does it too */
 +  if(lcid == lastLcid  dwFlags == lastFlags)
 +  {
 +memcpy(lpChars, lastChars, sizeof(defaultChars));
 +return;
 +  }
 +
   

This introduces a race condition.

-- 
Rob Shearman





Re: rpcrt4: Fix ndr_marshall test failures, try 2

2008-05-07 Thread Robert Shearman
Maarten Lankhorst wrote:
 -ok(RPC_S_OK == RpcServerUseProtseqEp(ncalrpc, 20, endpoint, NULL), 
 RpcServerUseProtseqEp\n);
 -ok(RPC_S_OK == RpcServerRegisterIf(IFoo_v0_0_s_ifspec, NULL, NULL), 
 RpcServerRegisterIf\n);
 -ok(RPC_S_OK == RpcServerListen(1, 20, TRUE), RpcServerListen\n);
 +status = RpcServerUseProtseqEp(ncalrpc, 20, endpoint, NULL);
 +ok(RPC_S_OK == status, RpcServerUseProtseqEp failed with status 
 %08lx\n, status);
 +status = RpcServerRegisterIf(IFoo_v0_0_s_ifspec, NULL, NULL);
 +ok(RPC_S_OK == status, RpcServerRegisterIf failed with status %08lx\n, 
 status);
 +status = RpcServerListen(1, 20, TRUE);
 +ok(RPC_S_OK == status, RpcServerListen failed with status %08lx\n, 
 status);
   

Since the RPC status values are in decimal in winerror.h, it is more 
user-friendly to output them in decimal in the failure messages.

-- 
Rob Shearman





Re: [PATCH] check for iface-ref to avoid crash

2008-05-07 Thread Robert Shearman
Marcus Meissner wrote:
 @@ -742,6 +742,9 @@ static void write_c_method_def(FILE *header, const type_t 
 *iface)
  
  static void write_c_disp_method_def(FILE *header, const type_t *iface)
  {
 +  if (!iface-ref) {
 +error_loc(write_c_disp_method_def: no reference on interface(%p)\n, 
 iface);
 +  }
do_write_c_method_def(header, iface-ref, iface-name);
  }
   

This can't ever happen. dispinterfaces always derive from IDispatch:
 dispinterfacehdr: attributes dispinterface  { attr_t *attrs;
   is_in_interface = TRUE;
   is_object_interface 
 = TRUE;
   $$ = $2;
   if ($$-defined) 
 error_loc(multiple definition error\n);
   attrs = 
 make_attr(ATTR_DISPINTERFACE);
   $$-attrs = 
 append_attr( check_dispiface_attrs($2-name, $1), attrs );
   $$-ref = 
 find_type(IDispatch, 0);
   if (!$$-ref) 
 error_loc(IDispatch is undefined\n);
   $$-defined = TRUE;
   if (!parse_only  
 do_header) write_forward($$);
 }


-- 
Rob Shearman





Re: winex11.drv: Set the size of returned DEVMODE to least common one as XP does

2008-05-04 Thread Robert Shearman
David Adam wrote:
 Hello

 -devmode-dmSize = sizeof(DEVMODEW);
 -devmode-dmSpecVersion = MAKEWORD(1,4);
 -devmode-dmDriverVersion = MAKEWORD(1,4);
 +devmode-dmSize = FIELD_OFFSET(DEVMODEW, dmICMMethod);

 +devmode-dmSpecVersion = DM_SPECVERSION;
 +devmode-dmDriverVersion = DM_SPECVERSION;
 Wouldn't it be better to write 
   *devmode-dmDeviceName=*dev_name
 instead of

  memcpy(devmode-dmDeviceName, dev_name, sizeof(dev_name));
   

You can't copy whole arrays using one dereference operator. The reason 
for this is that the dereference operator only returns the contents of 
the first element in the array.

-- 
Rob Shearman





Re: credui: Danish translation

2008-05-04 Thread Robert Shearman
Jens Albretsen wrote:
 Changelog:
 Danish translation of credui
   
 +LANGUAGE LANG_DANISH, SUBLANG_DEFAULT
 +
 +IDD_CREDDIALOG DIALOG DISCARDABLE  0, 0, 213, 149
 +STYLE DS_MODALFRAME | DS_NOIDLEMSG | DS_CENTER | WS_POPUP | WS_CAPTION | 
 WS_SYSMENU
 +CAPTION IDS_TITLEFORMAT
 +FONT 8, MS Shell Dlg
 +BEGIN
 +CONTROL IDB_BANNER,IDC_STATIC,Statisk,SS_BITMAP | 
 SS_CENTERIMAGE,0,
 +0,213,37

It's an easy mistake to make, but the third piece of data for the 
CONTROL statement is the name of a Windows class, so it shouldn't be 
translated.

BTW, you can test your translation of credui by running the credui tests 
with the WINETESTINTERACTIVE environment variable to 1.

-- 
Rob Shearman





Re: Fix building tools/widl (for bison 1.75, among others)

2008-05-02 Thread Robert Shearman
Gerald Pfeifer wrote:
 After the following change about a week ago

   Rob Shearman [EMAIL PROTECTED]
   widl: Make the rules for parsing fields in structures, encapsulated unions 
   and non-encapsulated unions more strict.

   Move the rules in fields that handle empty union cases into separate
   union rules so that they can't erroneously be accepted for structures or
   other types of unions.

 I started seeing the following build failure on one older installation:

   bison  -p parser_ -o parser.tab.c -d parser.y
   parser.y:749.2-751.15: type clash (`var' `attr_list') on default action
   parser.y:751.16: parse error, unexpected :, expecting ; or |
   parser.y:752.35-59: $2 of `ne_union_field' has no declared type
   parser.y:757.2-759.7: type clash (`var' `') on default action
   parser.y:759.8: parse error, unexpected :, expecting ; or |
   parser.y:759.45-760.50: $1 of `union_field' has no declared type
   parser.y:759.45-761.23: $2 of `union_field' has no declared type
   gmake: *** [parser.tab.h] Error 1

 It turns out that current versions of bison do not enforce the documented 
 grammer as strictly as older ones such as bison 1.75.  Fixed thusly.
   

Oops, thanks for spotting it. Actually, I was developing on version 1.28 
and didn't get those errors so it doesn't appear to be a deliberate 
change by the bison developers.

 ChangeLog:
 Fix syntax to also work with older versions of bison.

 Index: tools/widl/parser.y
 ===
 RCS file: /home/wine/wine/tools/widl/parser.y,v
 retrieving revision 1.199
 diff -u -3 -p -r1.199 parser.y
 --- tools/widl/parser.y   1 May 2008 18:38:07 -   1.199
 +++ tools/widl/parser.y   2 May 2008 12:07:24 -
 @@ -747,6 +747,7 @@ field:  m_attributes decl_spec declarat
  ne_union_field:
 s_field ';'   { $$ = $1; }
   | attributes ';'{ $$ = make_var(NULL); 
 $$-attrs = $1; }
 +;
  
  ne_union_fields: { $$ = NULL; }
   | ne_union_fields ne_union_field{ $$ = append_var( $1, $2 ); }
 @@ -755,6 +756,7 @@ ne_union_fields:  { $$ = NULL; }
  union_field:
 s_field ';'   { $$ = $1; }
   | ';'   { $$ = NULL; }
 +;
  
  s_field:  m_attributes decl_spec declarator  { $$ = $3-var;
 $$-attrs = 
 check_field_attrs($$-name, $1);

   

Looks good. Please send to wine-patches.

-- 
Rob Shearman





Re: widl: Add a framework for automated testing of IDL files that should succeed or fail to be parsed.

2008-05-02 Thread Robert Shearman
Alexandre Julliard wrote:
 Robert Shearman [EMAIL PROTECTED] writes:
   
 The way I see it, we have a choice between having a framework that uses 
 the makefile to run individual tests of the parser without checking the 
 content or a framework that runs every test in one go, but is capable of 
 checking the output of the generated files. The only technical advantage 
 that I can think of for the former is that it allows the tests to be 
 performed in parallel, but I don't know how much of a benefit that is to 
 Alexandre (who will be the one doing make test the most).
 

 We definitely have to be able to run tests individually from make, so
 there can't be just a single script to run them all.

 Still, it seems to me that most of these tests can just as well be done
 in the existing framework, as part of the rpcrt4 test for instance. This
 way we can not only make sure that the code compiles, but also that the
 generated code builds, and works the way it should.
   

I don't really like the idea of mixing tests of two different components 
into the same file. Also, when developing and trying to debug a 
regression I prefer to work on simpler IDL files (i.e. testing a 
particular type of statement in a few ways) than everything being in one 
file and having to work out what statement broke what.

 The only thing that can't be tested that way is obviously the code that
 is expected to fail to build, and for this something like Rob's
 framework would work fine, even though I'm not quite convinced that we
 care that much about getting the failure cases exactly right.

I see the failure cases being important for two reasons:
1. That the line number is reported correctly.
2. That the error is being triggered by the right part of the statement 
and therefore makes sense to the user.

Of course the failures themselves are important in order for incorrect 
constructs to be detected before:
1. widl crashes during generation of output files.
2. widl generates files that can't be compiled or compile with warnings.
3. widl generates files that compile correctly but crash or raise an 
exception at runtime.

-- 
Rob Shearman





Re: shlwapi: expose the IStream_Read and IStream_Write functions.

2008-05-01 Thread Robert Shearman
Reece Dunn wrote:
 @@ -926,6 +926,13 @@ HRESULT WINAPI 
 SHCreateStreamOnFileEx(LPCWSTR,DWORD,DWORD,BOOL,struct IStream*,s
  
  HRESULT WINAPI SHCreateStreamWrapper(LPBYTE,DWORD,DWORD,struct IStream**);
  
 +#undef IStream_Read
 +#undef IStream_Write
 +
 +HRESULT WINAPI IStream_Read(struct IStream *, LPVOID, ULONG);
 +
 +HRESULT WINAPI IStream_Write(struct IStream *, LPCVOID, ULONG);
 +
  #endif /* NO_SHLWAPI_STREAM */
  
  /* SHAutoComplete flags */
   

You've put this in the section guarded by NO_SHLWAPI_STREAM, but in the 
PSDK header it isn't guarded by this define. If it were the case, then 
you could have fixed the issue in files that include shlwapi.h by 
defining this before including it.

However, I think having to work around not being able to use the 
IStream_Read and IStream_Write macros is a bit ugly.

-- 
Rob Shearman





Re: RFC: detecting wine drivers in the audio tests

2008-04-30 Thread Robert Shearman
Francois Gouget wrote:
 My idea is that 
 some platforms (e.g. Wine) could ask for stricter checks, by using 
 strict_wine a bit like they use todo_wine to request looser checks.

 So for instance you would do:

strict_wine ok(expected_cond || buggy(this_is_a_bug), ...);

 Then on Windows the test would pass if either expected_cond is true or 
 if this_is_a_bug is true. The same would happen for other platforms that 
 this test runs on (e.g. ReactOS).

 However on Wine the strict_wine clause would cause buggy to always 
 return false. So on Wine this test would only succeed if expected_cond 
 is true.

 In addition to buggy I would propose a deprecated(cond) macro which 
 would behave exactly the same but document valid but deprecated Windows 
 behavior that we really don't want to reproduce.
   

I think that is an excellent idea and I can see it being immediately 
useful in rpcrt4, if not other areas.

-- 
Rob Shearman





Re: [PATCH 1/4] widl: Keep const attributes applied to pointers when writing out the type.

2008-04-29 Thread Robert Shearman
Alexandre Julliard wrote:
 --- include/d3d10.h.old   2008-04-29 13:38:38.0 +0200
 +++ include/d3d10.h   2008-04-29 13:39:08.0 +0200
 @@ -3455,12 +3455,12 @@
  virtual void STDMETHODCALLTYPE VSSetConstantBuffers(
  UINT StartSlot,
  UINT NumBuffers,
 -ID3D10Buffer **ppConstantBuffers) = 0;
 +ID3D10Buffer **const ppConstantBuffers) = 0;
  
 It should be ID3D10Buffer * const * instead.

That is due to the tree of types being constructed in the wrong order. 
I'll send a fixed patch in due course.

-- 
Rob Shearman





Re: widl: Add a framework for automated testing of IDL files that should succeed or fail to be parsed.

2008-04-29 Thread Robert Shearman
Dan Hipschman wrote:
 On Mon, Apr 28, 2008 at 08:37:26PM +0100, Robert Shearman wrote:
   
 This should aid in testing more-obscure parts of the parser that aren't 
 necessarily valid when using RPC (and hence don't make sense being put 
 in dlls/rpcrt4/tests/server.idl).
 

 Obviously this is a good idea.  I tried doing something similar a while
 ago, but it didn't get accepted on the first try and I never asked why,
 so I'll ask why now.  See here:

 http://winehq.org/pipermail/wine-patches/2006-September/030438.html

 It has some benefits over the approach you're proposing so it's worth
 bringing up.  One is that it can run the tests against MIDL, so we know
 the tests themselves are correct (ok, MIDL has been known to do things
 differently than the spec, but we may want to copy MIDL's behaviour
 anyway, and in any case it's easier to automatically validate the tests
 on MIDL than to do so by hand or by inspection).
   

The above linked patch of yours is what actually inspired me to add 
automated testing of widl. I don't see any reason why we can't run MIDL 
with the same framework. In fact, I think the framework I am proposing 
could better handle this case due to being able to expect specific 
errors/warnings that differ between MIDL and widl (and being able to 
cope with MIDL failing and widl succeeding, for example, due to bugs in 
MIDL).

 Another is versatility in what it can test.  Since the tests are shell
 scripts, not only can it test the success/failure of a parse, but it can
 check that all the correct files were created, and it can perform
 further tests on the output (e.g., if we have an import in the IDL file,
 we can grep the output to make sure a #include was generated for the
 import).

 It already supports todos, and the test scripts look similar to the
 usual Wine tests written in C.
   

Absolutely. The flexibility of your system being able to test the 
contents of files and do other checks for specific tests is a definite 
bonus.

 On the other hand your method has some advantages over mine.  You
 already provide a make test framework,

I don't see any reason why your framework can't also be plugged into 
make test.

 and including the expected
 result of the compilation in the IDL file is cleaner.
   

Again, that could also be done using your approach. It's just that it 
was necessary with my approach.

 Alexandre may very well disagree with what I see as valid points (like
 testing on MIDL), but these are my suggestions anyway.  Hope they're
 useful.

The way I see it, we have a choice between having a framework that uses 
the makefile to run individual tests of the parser without checking the 
content or a framework that runs every test in one go, but is capable of 
checking the output of the generated files. The only technical advantage 
that I can think of for the former is that it allows the tests to be 
performed in parallel, but I don't know how much of a benefit that is to 
Alexandre (who will be the one doing make test the most).

-- 
Rob Shearman





Re: widl: callback, code, comm_status and in_line are attribute names, not keywords.

2008-04-29 Thread Robert Shearman
Robert Shearman wrote:
 ---
  tools/widl/parser.l |8 
  1 files changed, 4 insertions(+), 4 deletions(-)

Just to note, this patch depends on the patch series that I sent 
yesterday and that I'm about to resend now.

-- 
Rob Shearman





Re: [PATCH 2/7] widl: Keep const attributes applied to pointers when writing out the type.

2008-04-28 Thread Robert Shearman
Alexandre Julliard wrote:
 Robert Shearman [EMAIL PROTECTED] writes:

   
 Use an attribute to store the const qualifier for the pointer and type.

 Allow multiple type-qualifiers to be applied to a type by adding a
 declaration-specifier rule that encompasses type-qualifiers and types.
 

 This seems to drop the const in some cases, for instance:

 --- include/propidl.h.old 2008-04-28 13:52:01.0 +0200
 +++ include/propidl.h 2008-04-28 13:52:11.0 +0200
 @@ -332,7 +332,7 @@
  virtual HRESULT STDMETHODCALLTYPE WritePropertyNames(
  ULONG cpropid,
  const PROPID rgpropid[],
 -const LPOLESTR rglpwstrName[]) = 0;
 +LPOLESTR rglpwstrName[]) = 0;
  
  virtual HRESULT STDMETHODCALLTYPE DeletePropertyNames(
  ULONG cpropid,

Ok, I'll have to investigate why this is happening. Thanks for spotting it.

-- 
Rob Shearman





Re: [PATCH 6/7] widl: Accept integer constant suffixes in the lexer.

2008-04-27 Thread Robert Shearman
Dan Hipschman wrote:
 On Sat, Apr 26, 2008 at 09:51:58AM +0100, Robert Shearman wrote:
   
 +u_suffix(l|L)
 +l_suffix(u|U)
 

 I'm guessing you meant for these to be the other way around.
   

Good spot, thanks!

-- 
Rob Shearman





Re: [PATCH 09/17] widl: Split the expr rule up into multiple rules to fix operator precedence.

2008-04-21 Thread Robert Shearman

Alexandre Julliard wrote:

You shouldn't need that, the %left/%right declarations should define the
correct precedence already. Why doesn't this work for you?


I was attempting to fix the attached case. The result of the expression 
evaluation can be seen in the generated _c.c file by searching for /* 
Corr desc:  constant, val=.


I see that the issue is that the %left and %right declarations aren't in 
the correct order (and some aren't grouped properly either).


So this bug can indeed be fixed without splitting up expr into multiple 
rules.


--
Rob Shearman


[
uuid(7a98c250-6808-11cf-b73b-00aa00b677a7),
version(1.0)
]
interface Tests
{
struct s
{
[size_is(2  2 * 3)] char *array;
};
void Test([in] struct s *s);
}



Re: Help with configure option

2008-04-16 Thread Robert Shearman
Alistair Leslie-Hughes wrote:
   I need to add an option, HAS_TEXTCONCAT_BUG to the configure script 
 to check for a bug in libxml.  What file(s) do I have to change?

 The file attached is an example to test for this issue. (returns -1 if 
 the error exists)

The trouble is that because msxml3 links to a shared library, the 
library that the configure check is done against isn't necessarily the 
version that is used at runtime. (Think .debs for multiple distros and 
versions of those distros.)

Therefore, you may have to do the check at runtime. Where that check is 
implemented depends on what you need to do when you discover the buggy 
version is in use.

-- 
Rob Shearman





Re: fusion: Explicitly check for -1 for a missing table

2008-04-16 Thread Robert Shearman
James Hawkins wrote:
 It works just fine.  -1 is 4294967295 in ULONG (32bit), which is
 exactly the same as offset on error (because we assigned it -1, but
 the representation in memory is the same).

Then use ~0 so that the purpose is clearer. Adding a define for this 
value would probably further increase readability of the code.

 We do this several other
 places in the code base.

They should be changed too.

-- 
Rob Shearman





RFC: Fix for IWineMsiRemotePackage::FormatRecord

2008-04-15 Thread Robert Shearman

Hi,

Currently, the [out] value parameter for 
IWineMsiRemotePackage::FormatRecord doesn't have a level of indirection 
associated with it and so I would be very surprised if the typelib 
marshaller actually does the right thing in this case. Compiling with 
MIDL and with a future update to widl causes an error for this 
parameter. I'm proposing the attached patch to fix things, but I'm not 
able to test that this works correctly.


Thanks,

--
Rob Shearman

From 6e1a75e76dbf402b05b9475eaf74b7b1341919ea Mon Sep 17 00:00:00 2001
From: Robert Shearman [EMAIL PROTECTED]
Date: Tue, 15 Apr 2008 11:33:43 +0100
Subject: msi: Fix the value parameter of IWineMsiRemotePackage::FormatRecord to have the right level of indirection for an [out] parameter.
To: wine-patches [EMAIL PROTECTED]

Remove the redundant size parameter and simplify the client code such that the remote function is only called once, with the value being automatically allocated. Add corresponding code on the server side to automatically allocate said value.
---
 dlls/msi/format.c  |   17 +
 dlls/msi/msiserver.idl |2 +-
 dlls/msi/package.c |   13 +++--
 3 files changed, 13 insertions(+), 19 deletions(-)

diff --git a/dlls/msi/format.c b/dlls/msi/format.c
index f58f517..454142a 100644
--- a/dlls/msi/format.c
+++ b/dlls/msi/format.c
@@ -921,28 +921,13 @@ UINT WINAPI MsiFormatRecordW( MSIHANDLE hInstall, MSIHANDLE hRecord,
 HRESULT hr;
 IWineMsiRemotePackage *remote_package;
 BSTR value = NULL;
-DWORD len;
 awstring wstr;
 
 remote_package = (IWineMsiRemotePackage *)msi_get_remote( hInstall );
 if (remote_package)
 {
-len = 0;
 hr = IWineMsiRemotePackage_FormatRecord( remote_package, hRecord,
- NULL, len );
-if (FAILED(hr))
-goto done;
-
-len++;
-value = SysAllocStringLen( NULL, len );
-if (!value)
-{
-r = ERROR_OUTOFMEMORY;
-goto done;
-}
-
-hr = IWineMsiRemotePackage_FormatRecord( remote_package, hRecord,
- value, len );
+ value );
 if (FAILED(hr))
 goto done;
 
diff --git a/dlls/msi/msiserver.idl b/dlls/msi/msiserver.idl
index d5b47ac..37aa91e 100644
--- a/dlls/msi/msiserver.idl
+++ b/dlls/msi/msiserver.idl
@@ -70,7 +70,7 @@ interface IWineMsiRemotePackage : IUnknown
 HRESULT SetComponentState( [in] BSTR component, [in] INSTALLSTATE state );
 HRESULT GetLanguage( [out] LANGID *language );
 HRESULT SetInstallLevel( [in] int level );
-HRESULT FormatRecord( [in] MSIHANDLE record, [out] BSTR value, [out] DWORD *size );
+HRESULT FormatRecord( [in] MSIHANDLE record, [out] BSTR *value );
 HRESULT EvaluateCondition( [in] BSTR condition );
 }
 
diff --git a/dlls/msi/package.c b/dlls/msi/package.c
index a587a63..f11b068 100644
--- a/dlls/msi/package.c
+++ b/dlls/msi/package.c
@@ -1812,10 +1812,19 @@ static HRESULT WINAPI mrp_SetInstallLevel( IWineMsiRemotePackage *iface, int lev
 }
 
 static HRESULT WINAPI mrp_FormatRecord( IWineMsiRemotePackage *iface, MSIHANDLE record,
- BSTR value, DWORD *size )
+BSTR *value)
 {
+DWORD size = 0;
 msi_remote_package_impl* This = mrp_from_IWineMsiRemotePackage( iface );
-UINT r = MsiFormatRecordW(This-package, record, (LPWSTR)value, size);
+UINT r = MsiFormatRecordW(This-package, record, NULL, size);
+if (r == ERROR_SUCCESS)
+{
+*value = SysAllocStringLen(NULL, size);
+if (!*value)
+return E_OUTOFMEMORY;
+size++;
+r = MsiFormatRecordW(This-package, record, *value, size);
+}
 return HRESULT_FROM_WIN32(r);
 }
 
-- 
1.5.3.8




Re: browseui: Remove unused variables

2008-04-15 Thread Robert Shearman
Andrew Talbot wrote:
 @@ -299,7 +299,6 @@ static HRESULT WINAPI 
 ProgressDialog_StartProgressDialog(IProgressDialog *iface,
  {
  ProgressDialog *This = (ProgressDialog *)iface;
  struct create_params params;
 -HANDLE hThread;
  
  TRACE((%p, %p, %x, %p)\n, iface, punkEnableModeless, dwFlags, 
 reserved);
  if (punkEnableModeless || reserved)
 @@ -321,7 +320,7 @@ static HRESULT WINAPI 
 ProgressDialog_StartProgressDialog(IProgressDialog *iface,
  params.hwndParent = hwndParent;
  params.hEvent = CreateEventW(NULL, TRUE, FALSE, NULL);
  
 -hThread = CreateThread(NULL, 0, dialog_thread, params, 0, NULL);
 +CreateThread(NULL, 0, dialog_thread, params, 0, NULL);
  WaitForSingleObject(params.hEvent, INFINITE);
  
  This-hwndDisabledParent = NULL;

A correct fix is to call CloseHandle(hThread), otherwise the handle is 
leaked.


-- 
Rob Shearman





Re: comctl32: Remove unused variables

2008-04-15 Thread Robert Shearman
Andrew Talbot wrote:
 @@ -967,16 +967,14 @@ static HFONT SYSLINK_SetFont (SYSLINK_INFO *infoPtr, 
 HFONT hFont, BOOL bRedraw)
   */
  static LRESULT SYSLINK_SetText (SYSLINK_INFO *infoPtr, LPCWSTR Text)
  {
 -int textlen;
 -
  /* clear the document */
  SYSLINK_ClearDoc(infoPtr);
 -
 -if(Text == NULL || (textlen = lstrlenW(Text)) == 0)
 +
 +if(Text == NULL || lstrlenW(Text) == 0)
  {
   

A better fix would be to use *Text == 0 instead of lstrlenW(Text) == 0.

-- 
Rob Shearman





Re: dinput: Remove unused variables

2008-04-15 Thread Robert Shearman
Andrew Talbot wrote:
 @@ -142,13 +142,12 @@ static INT find_joystick_devices(void)
  {
  CHAR device_name[MAX_PATH], *str;
  INT len;
 -int fd;
  
  len = sprintf(device_name, %s%d, JOYDEV_NEW, i) + 1;
 -if ((fd = open(device_name, O_RDONLY))  0)
 +if (open(device_name, O_RDONLY)  0)
  {
  len = sprintf(device_name, %s%d, JOYDEV_OLD, i) + 1;
 -if ((fd = open(device_name, O_RDONLY))  0) continue;
 +if (open(device_name, O_RDONLY)  0) continue;
  }
  
  if (!(str = HeapAlloc(GetProcessHeap(), 0, len))) break;

Again, this needs to be fixed in another way as fd is being leaked.

-- 
Rob Shearman





Re: error: `XICCallback' undeclared (Solaris build problem)

2008-04-14 Thread Robert Shearman
Kusanagi Kouichi wrote:
 Does this patch fix the problem?

 -XICCallback destroy = {(XPointer)data, X11DRV_DestroyIC};
 +XIMCallback destroy = {(XPointer)data, (XIMProc)X11DRV_DestroyIC};
   

I think a configure check may be a more appropriate fix.

-- 
Rob Shearman





Re: msdmo: Array parameter is passed to function as pointer so loses size information

2008-04-10 Thread Robert Shearman

Andrew Talbot wrote:

Robert Shearman wrote:
  

This is incorrect. count is the size in bytes of the buffer passed in
(szName) and so should be sizeof(szName) not
sizeof(szName)/sizeof(szName[0]) (i.e. 80).




Are you sure? MSDN says szName: Array of 80 Unicode characters that
receives the name of the DMO.
  


It doesn't matter what MSDN says about szName, RegQueryValueExW still 
takes the size in bytes, not characters. I.e. count should be set to 
NAME_SIZE * sizeof(WCHAR), not NAME_SIZE.



But, in any case, when arrays
are passed as arguments to functions they are converted to pointers, so
sizeof(szName) would represent the size of a pointer to the array, not the
size of the array itself.


I didn't realise this before but the attached test program prints:
$ ./atest
sizeof(array) = 4
So you are right. Still, my point stands that the code is still passing 
in a smaller size than the size of the buffer and so is not correct.


--
Rob Shearman

#include stdio.h

static void arrayfunc(char array[80])
{
	printf(sizeof(array) = %d\n, sizeof(array));
}

int main(int argc, char *argv[])
{
	arrayfunc(NULL);
}



Re: shell32: Implement ParseDisplayName for EntireNetwork in the Network Places shell folder.

2008-04-10 Thread Robert Shearman
Juan Lang wrote:
 Hi Rob,

 +strcpy(pData-u.network.szNames, Entire Network);

 Is this not locale-dependent?

I don't believe so. ParseDisplayName (which isn't implemented yet) will 
do the mapping between the PIDL and the name the user sees, which 
doesn't have to depend on pData-u.network.szNames.

-- 
Rob Shearman





Re: msdmo: Array parameter is passed to function as pointer so loses size information

2008-04-09 Thread Robert Shearman
Andrew Talbot wrote:
 @@ -291,8 +291,9 @@ lend:
   *
   * Get DMP Name from the registry
   */
 -HRESULT WINAPI DMOGetName(REFCLSID clsidDMO, WCHAR szName[80])
 +HRESULT WINAPI DMOGetName(REFCLSID clsidDMO, WCHAR szName[])
  {
 +#define NAME_SIZE   80  /* Size of szName[] */
  WCHAR szguid[64];
  HRESULT hres;
  HKEY hrkey = 0;
 @@ -311,7 +312,7 @@ HRESULT WINAPI DMOGetName(REFCLSID clsidDMO, WCHAR 
 szName[80])
  if (ERROR_SUCCESS != hres)
  goto lend;
  
 -count = sizeof(szName);
 +count = NAME_SIZE;
  hres = RegQueryValueExW(hkey, NULL, NULL, NULL, 
  (LPBYTE) szName, count); 
   

This is incorrect. count is the size in bytes of the buffer passed in 
(szName) and so should be sizeof(szName) not 
sizeof(szName)/sizeof(szName[0]) (i.e. 80).

I see this patch has already been committed, so 
a9200b24014607c4c82fb052b97de88daa804a81 should be reverted.

If you want to pick up errors like passing the wrong size into functions 
then I would suggest using an automatic checker that is able to use 
semantic information, like Microsoft's PREfast.

-- 
Rob Shearman





Re: Wine 1.0 status: 70 days to release, 68 bugs to fix. 28 days to code freeze!

2008-04-05 Thread Robert Shearman
Dan Kegel wrote:
 New (by A.F. and Austin):
 11420  advapi320   service control manager API problem:
 name of named objects might differ (client vs. service process)
   

This should be fixed as a result of the services work that was done in 
the last couple of weeks and the last comment in the bug confirms this.

-- 
Rob Shearman





Re: [PATCH 4/4] configure.ac: Add support for MSVC.

2008-04-04 Thread Robert Shearman
Alexandre Julliard wrote:
 Robert Shearman [EMAIL PROTECTED] writes:
   
 MSVC needs a lot more in the way of rule changes and/or helper scripts
 for it to work properly, but this is a good start.
 

 I'm not sure I see the point. You can't really run configure or make on
 an MSVC setup anyway.

Well, you can. MSVC can be run in wine and you can install a Win32 port 
of sh to run configure on Windows and all of our configure tests work.

 It seems to me that something like msvcmaker is a
 better solution.

Yes, you're right. I wish I'd known about the purpose of this tool 
before I went down the route that I did...

-- 
Rob Shearman





Re: Crossbuild patch, appropriate for wine?

2008-04-04 Thread Robert Shearman
John Klehm wrote:
 diff --git a/include/excpt.h b/include/excpt.h
 index 3369f3b..081fb06 100644
 --- a/include/excpt.h
 +++ b/include/excpt.h
 @@ -46,6 +46,10 @@ typedef enum _EXCEPTION_DISPOSITION
  unsigned long __cdecl _exception_code(void);
  void * __cdecl _exception_info(void);
  int __cdecl _abnormal_termination(void);
 +#elif defined(__GNUC__)  defined(USE_COMPILER_EXCEPTIONS)
 +#define __try
 +#define __except(x) while(0)
 +#define __finally
  #endif /* defined(_MSC_VER)  defined(USE_COMPILER_EXCEPTIONS) */

  #endif /* __WINE_EXCPT_H */
   

Actually, I think the best place to fix this is in 
include/wine/exception.h. I also think it would be better to use 
defined(__MINGW32__)  !defined(USE_COMPILER_EXCEPTIONS). This is 
because that is the platform that can't use the exception macros 
implemented on top of sigsetjmp, and if the developer defines 
USE_COMPILER_EXCEPTIONS then it is telling us that their compiler 
supports __try, __except and __finally (there have been custom builds of 
MinGW that include support for native exception handling).

-- 
Rob Shearman





Re: Crossbuild patch, appropriate for wine?

2008-04-04 Thread Robert Shearman
Alexandre Julliard wrote:
 Robert Shearman [EMAIL PROTECTED] writes:
   
 Actually, I think the best place to fix this is in 
 include/wine/exception.h. I also think it would be better to use 
 defined(__MINGW32__)  !defined(USE_COMPILER_EXCEPTIONS). This is 
 because that is the platform that can't use the exception macros 
 implemented on top of sigsetjmp
 

 I don't think we want to disable exceptions, that will just lead to
 broken builds. It should be possible to make the sigsetjmp variant work
 for MinGW.

You can make it compile by using sigjmp instead of sigsetjmp, but 
there's not much point because the code depends on Wine-only functions 
in ntdll that aren't available on Windows and so would cause the DLLs 
not to load.

-- 
Rob Shearman





Re: msi: Remove tentative definition of static array with no size specifier

2008-04-03 Thread Robert Shearman
Andrew Talbot wrote:
 James Hawkins wrote:

   
 It's ugly.  What warning are you trying to fix?

 

 Although I imagine that gcc doesn't do anything particularly adverse as a
 result of the existing code, if the pedantic switch were applied it would
 cause a message of the following type to be generated.

 action.c:236: error: array size missing in ?StandardActions?

 I believe it is also likely to show up under lint-like tools and I believe
 it is actually an error, though compilers are not required to generate any
 message, apparently. I couldn't say whether the resultant behaviour is
 undefined, implemenation defined, or what. And I don't know whether gcc
 places any surplus baggage in any segment as a result. The fix was just to
 make the code correct and hence more portable.

This also causes problems when compiling with msvc:
 make[1]: Entering directory `/home/rob/wine-msvc/dlls/msi'
 CC action.o
 action.c
 action.c(236) : error C2133: 'StandardActions' : unknown size

It also might affect compilers on other platforms that Wine might be 
ported to. As Andy says, it's non-standard behaviour and so could break 
even in future versions of gcc.

I think the patch could be changed so that very few, if any, forward 
declarations would need to be made simply by moving 
ACTION_HandleStandardAction to after StandardActions is initialised.

-- 
Rob Shearman





Re: Hey all, introducing myself and my GSoC idea...

2008-04-01 Thread Robert Shearman
James Hawkins wrote:
 On Mon, Mar 31, 2008 at 8:11 PM, Juan Lang [EMAIL PROTECTED] wrote:
   
  Personally, I've thought that playing around with the
  native LPC API might be interesting.  I'm sure there are other areas
  of the native API that are sparsely documented, and for which some
  test cases might prove useful.
 

 But is the API in question useful for the Wine project as far as
 fixing apps?  I don't think we should devote SOC resources to a
 project that doesn't progress Wine beyond just implementing more APIs.

It is useful for getting native rpcrt4 to work, which in turn is useful 
for getting other COM DLLs to work, and so is potentially useful for bug 
triage and identifying the source of bugs in builtin COM DLLs.

-- 
Rob Shearman





Re: midl question

2008-03-31 Thread Robert Shearman
Alistair Leslie-Hughes wrote:
 Hi Rob,

 What work would need to be done to get widl to support __stdcall?

The issue is that widl doesn't support functions in typedefs. Further 
issues are that widl doesn't support writing out the function typedef 
once parsed and widl doesn't store the calling convention for the 
function and so it currently can't be written out correctly.

-- 
Rob Shearman





Re: amstream: IAMMultiMediaStream::EnumMediaStreams

2008-03-28 Thread Robert Shearman
Nikolay Sivov wrote:
 @@ -144,9 +144,18 @@
  {
  IAMMultiMediaStreamImpl *This = (IAMMultiMediaStreamImpl *)iface;
  
 -FIXME((%p/%p)-(%ld,%p) stub!\n, This, iface, Index, ppMediaStream); 
 +TRACE((%p/%p)-(%d,%p)\n, This, iface, Index, ppMediaStream);
  
 -return E_NOTIMPL;
 +/* check out of range */
 +if(Index  0 || Index = This-nbStreams)
 +return S_FALSE;
 +/* NULL pointer */
 +if(!(*ppMediaStream))
 +return E_POINTER;
 +
 +*ppMediaStream = This-pStreams[Index];
 +
 +return S_OK;
  }
   

You need to call IMediaStream_AddRef on the returned stream.


-- 
Rob Shearman





Re: [PATCH 3/7] services: Start a local RPC server.

2008-03-28 Thread Robert Shearman
Alexandre Julliard wrote:
 Robert Shearman [EMAIL PROTECTED] writes:

   
 +handle_t __RPC_USER MACHINE_HANDLEW_bind(MACHINE_HANDLEW MachineName)
 +{
 +WCHAR transport[] = SVCCTL_TRANSPORT;
 +WCHAR endpoint[] = SVCCTL_ENDPOINT;
 +LPWSTR server_copy = NULL;
 +RPC_WSTR binding_str;
 +RPC_STATUS status;
 +handle_t rpc_handle;
 +
 +/* unlike Windows we start services.exe on demand. We start it always as
 + * checking if this is our address can be tricky */
 +if (!check_services_exe())
 +return NULL;
 

 I think it would be OK to assume that services.exe is started by
 wineboot and is always running, since we'll have at least the mountmgr
 service running all the time.
   

I'm not sure that is a valid assumption, as we could have an option to 
disable wineboot in the future due to performance reasons.

 +/* Not the Windows event name - if needed the true one can be found in 
 Inside Windows */
 +cpp_quote(#define SVCCTL_STARTED_EVENT (const 
 WCHAR[]){'_','_','w','i','n','e','_','S','v','c','c','t','l','S','t','a','r','t','e','d',0})
 

 That's gcc-specific syntax, it would need some #ifdefs.

Well spotted. I'll fix that.

-- 
Rob Shearman





Re: amstream: IMediaStream::GetMultiMediaStream

2008-03-28 Thread Robert Shearman
Nikolay Sivov wrote:
 @@ -109,9 +109,14 @@
  {
  IMediaStreamImpl *This = (IMediaStreamImpl *)iface;
  
 -FIXME((%p/%p)-(%p) stub!\n, This, iface, ppMultiMediaStream); 
 +TRACE((%p/%p)-(%p)\n, This, iface, ppMultiMediaStream);
  
 -return S_FALSE;
 +if(!(*ppMultiMediaStream))
 +return E_POINTER;
 +
 +*ppMultiMediaStream = This-Parent;
 +
 +return S_OK;
  }
   

Although I haven't looked at the context, you also likely need to add a 
reference to the returned COM object here.

-- 
Rob Shearman





Re: midl question

2008-03-27 Thread Robert Shearman
Alistair Leslie-Hughes wrote:
 Hi,
   
 In an IDL file I need to define the following
 typedef HRESULT (__stdcall *FExecuteInAppDomainCallback) (void* cookie);

 How do i get midl to understand what __stdcall is defined to?

midl or widl? AFAIK midl supports the __stdcall keyword.


-- 
Rob Shearman





Re: widl: Use is_string_type for detecting strings in write_typeformatstring_var to make it consistent with write_remoting_arg.

2008-03-27 Thread Robert Shearman
Alexandre Julliard wrote:
 Robert Shearman [EMAIL PROTECTED] writes:

   
 Fix the is_string_type function used for detecting strings by only
 examining aliases instead of both aliases and pointers. This is due to
 the requirement that pointers to strings be handled as pointers and so
 not detected as strings.
 

 This breaks qmgr:

 ../../../tools/runtest -q -P wine -M qmgr.dll -T ../../.. -p qmgr_test.exe.so 
 file.c  touch file.ok
 file.c:127: Test failed: Got incorrect remote name
 file.c:144: Test failed: Got incorrect local name
 make: *** [file.ok] Error 2

Thanks, there was a typo in the patch. I'll resend it with the typo fixed.

I believe the patches titled widl: Support using context handles as the 
binding handle in client functions. and widl: Add support for generic 
binding handles. will still apply without this patch being committed first.

-- 
Rob Shearman





Re: GSOC proposal - control panel

2008-03-27 Thread Robert Shearman
[EMAIL PROTECTED] wrote:
 No dice.
 CPls are DLLs that export a function - CPlApplet(). 
 winegcc coughs on dllexport. - not implemented.
   

__declspec(dllexport) is a MSVC-only feature. Hence, when compiling with 
gcc (even using winegcc) it won't actually export the function. As has 
been stated before, you need to use a .spec or .def file to export it.

 I've been hacking at it for days on end now. 
 Result: CPl can be compiled under windows and run under wine. CPls compiled 
 with winegcc don't export that function = not treated as control panel 
 applets.
 software to aid anyone: dllexp.exe. Download and run on any dll/cpl of 
 windows, you get exports in it; run on wine dlls - nada. (hence, No Template 
 in wine's source. Twain DLL exports some methods though.)

 MICROSOFT ISSUE: EASY TO DISTINGUISH BETWEEN WINE DLLS AND WINDOWS DLLS. 

 If dllexp.exe can do it, so can they. Solution: M$ implements a code that 
 *checks for a dll version* (based on what the DLL exports). Doesn't find 
 export = coughs and dies with an Incompatible DLL version. Happened before 
 with OS2 / Windows. Maybe has happened now with all M$ that cough on wine.

 Not enough intimate knowledge = can a soft cough and die based on a call to 
 a 
 DLL export which isn't there, without being designed to do so?

A DLL compiled as a winelib DLL won't be parsed by a tool that reads PE 
DLLs, because winelib DLLs are Elf shared objects. Use winedump instead.

-- 
Rob Shearman





Re: wininet: Implement chunked reads.

2008-03-26 Thread Robert Shearman
Hans Leidekker wrote:
  static DWORD HTTPREQ_ReadFile(WININETHANDLEHEADER *hdr, void *buffer, DWORD 
 size, DWORD *read)
  {
  WININETHTTPREQW *req = (WININETHTTPREQW*)hdr;
 +static WCHAR encoding[20];
 +DWORD buflen = sizeof(encoding);
 +static const WCHAR szChunked[] = {'c','h','u','n','k','e','d',0}; 
   

encoding shouldn't be static.

-- 
Rob Shearman





Re: Version of Windows

2008-03-19 Thread Robert Shearman
Chris Teague wrote:
 This is probably trivial, but I'm having a tough time figuring out the
 current version of Windows that Wine is mimicking.  I have a situation
 where I need to behave differently if the version is NT (0x0500)
 versus when it is XP (0x0600).  I see the WINVER #define, but that
 doesn't seem right to me since this must be a runtime variable.  Can
 someone point me at an example of how to tell what the current version
 of Windows is?  Thanks,
   

Errm, GetVersion()?

If you're a developer for a Windows application it is a much better 
policy to check for features rather than checking the Windows version, 
since that feature may be implemented in a later service pack for that 
version of Windows.

-- 
Rob Shearman





Re: Alexandre Julliard : winex11: Fix bug report address

2008-03-19 Thread Robert Shearman
Groeschel, Volker wrote:
 The Patch points to wine-cvs. Shouldn't it point to wine-devel.

Not in the version of the patch I'm looking at:
  if (! is_tablet_cursor(target-name, device_type))
  {
 -WARN(Skipping device %d [name %s|type %s]; not apparently a 
 tablet cursor type device.  If this is wrong, please report to 
 http://forums.winehq.org\n;,
 +WARN(Skipping device %d [name %s|type %s]; not apparently a 
 tablet cursor type device.  If this is wrong, please report it to [EMAIL 
 PROTECTED],
   loop, devices[loop].name, device_type ? device_type : 
 );
  XFree(device_type);
  cursor_target --;
   

-- 
Rob Shearman





Re: user32: Test destroying the cursor of a parent process.

2008-03-18 Thread Robert Shearman
Saulius Krasuckas wrote:
 * On Tue, 8 Jan 2008, Eric Pouech wrote:
   
 * Andrew Riedi a écrit :
 
  dlls/user32/tests/cursoricon.c |  201 
 
   
 ...
   
 +static void do_child(void)
 +{
 +WNDCLASS class;
 +MSG msg;
 +BOOL ret;
 +
 +/* Register a new class. */
 +class.style = CS_GLOBALCLASS;
 +class.lpfnWndProc = callback_child;
 +class.cbClsExtra = 0;
 +class.cbWndExtra = 0;
 +class.hInstance = GetModuleHandle(NULL);
 +class.hIcon = NULL;
 +class.hCursor = NULL;
 +class.hbrBackground = NULL;
 +class.lpszMenuName = NULL;
 +class.lpszClassName = cursor_child;
 +
 +SetLastError(0xdeadbeef);
 +ret = RegisterClass(class);
 +ok(ret, Failed to register window class. Error: %d\n, 
 GetLastError());
   
   ...
   
 IMO, the ok() tests in the child process are a bad idea (they won't be 
 counted, nor returned as errors, by the parent process)
 

 Was Wine test framework architecture done such way on a purpose?  Why 
 would it be a bad idea to take into account a child output also?

The architecture has been fixed since the introduction of the 
winetest_wait_child_process function in include/wine/test.h.

-- 
Rob Shearman





Re: services.exe/advapi32[3/7]: start a local RPC server

2008-03-17 Thread Robert Shearman
Mikołaj Zalewski wrote:
 @@ -231,6 +247,101 @@ static inline LPWSTR SERV_dupmulti(LPCSTR str)
  }
  
  
 /**
 + * RPC connection with servies.exe
 + */
 +
 +static BOOL check_services_exe()
 +{
   

Put void in the brackets here and in many other places.

 +static SvcCtlRpcHandle connect_to_server(LPCWSTR server)
 +{
 +WCHAR transport[] = SVCCTL_TRANSPORT;
 +WCHAR endpoint[] = SVCCTL_ENDPOINT;
 +LPWSTR server_copy = NULL;
 +RPC_WSTR binding_str;
 +RPC_STATUS status;
 +SvcCtlRpcHandle rpc_handle;
 +
 +/* unlike Windows we start services.exe on demand. We start it always as
 + * checking if this is our address can be tricky */
 +if (!check_services_exe())
 +return NULL;
 +
 +if (server) /* parameters of RpcStringBindingComposeW are not const */
 +{
 +server_copy = HeapAlloc(GetProcessHeap(), 0, 
 sizeof(WCHAR)*(strlenW(server) + 1));
 +strcpyW(server_copy, server);
 +}
   

RpcStringBindingComposeW doesn't change server if you pass it in, so 
there is no need to make a copy.

 +status = RpcStringBindingComposeW(NULL, transport, server_copy, 
 endpoint, NULL, binding_str);
 +HeapFree(GetProcessHeap(), 0, server_copy);
 +if (status != RPC_S_OK)
 +{
 +ERR(RpcStringBindingComposeW failed (%d)\n, (DWORD)status);
 +return NULL;
 +}
   

 diff --git a/include/wine/svcctl.idl b/include/wine/svcctl.idl
 new file mode 100644
 index 000..fefd12f
 --- /dev/null
 +++ b/include/wine/svcctl.idl
 @@ -0,0 +1,68 @@
 +/*
 + * svcctl interface definitions - exported by services.exe to access the
 + * services database
 + *
 + * Copyright 2007 Google (Mikolaj Zalewski)
 + *
 + * This library is free software; you can redistribute it and/or
 + * modify it under the terms of the GNU Lesser General Public
 + * License as published by the Free Software Foundation; either
 + * version 2.1 of the License, or (at your option) any later version.
 + *
 + * This library is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
 + * Lesser General Public License for more details.
 + *
 + * You should have received a copy of the GNU Lesser General Public
 + * License along with this library; if not, write to the Free Software
 + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
 + */
 +
 +import wtypes.idl;
 +
 +/* 
 + * some defined for the C code
 + */
 +cpp_quote(#define SVCCTL_TRANSPORT {'n','c','a','c','n','_','n','p',0});
 +cpp_quote(#define SVCCTL_ENDPOINT 
 {'','p','i','p','e','','s','v','c','c','t','l',0});
 +
 +/* Not the Windows event name - if needed the true one can be found in 
 Inside Windows */
 +cpp_quote(#define SVCCTL_STARTED_EVENT (const 
 WCHAR[]){'_','_','w','i','n','e','_','S','v','c','c','t','l','S','t','a','r','t','e','d',0});
 +
 +
 +/* Based on the Samba IDL. Some functions are compatible with Windows but 
 some
 + * aren't and many are missing (thus function numbers are different) so we
 + * don't use the Windows uuid which is 367abb81-9844-35f1-ad32-98f038001003 .
 + */
 +[ uuid(78e2d9e8-d2a8-40d8-9bbe-7328cc5a9c32),
 +  version(2.0),
 +  pointer_default(unique),
 +  endpoint(ncacn_np:[\\pipe\\svcctl]),
 +  helpstring(Service Control)
 +] interface svcctl
 +{
 +typedef handle_t SvcCtlRpcHandle;
 +
 +typedef struct _POLICY_HANDLE
 +{
 +DWORD dwHandleType;
 + GUID uuid;
 +} POLICY_HANDLE;
   

There's no need to use Samba's concept of POLICY_HANDLE. We fully 
support the DCE/RPC context_handle type which is what this actually is.

You can find out more about how to use context handles here:
http://msdn2.microsoft.com/en-us/library/aa373605(VS.85).aspx

 +
 +/* Compatible with Windows function 0x00 */
 +DWORD svcctl_CloseServiceHandle(
 +[in] SvcCtlRpcHandle rpc_handle,
 +[in,out] POLICY_HANDLE *handle
 +);
   

There's no need for the rpc_handle here once you make the second 
parameter into a context handle.

 +
 +/* Compatible with Windows function 0x0f */
 +DWORD svcctl_OpenSCManagerW(
 +SvcCtlRpcHandle rpc_handle,
 +[in,unique] LPCWSTR MachineName,
 +[in,unique] LPCWSTR DatabaseName,
 +[in] DWORD dwAccessMask,
 +[out] POLICY_HANDLE *handle
 +);
 +
 +}
   

-- 
Rob Shearman





Re: services.exe/advapi32[5/7]: Move QueryServiceConfig to services.exe

2008-03-17 Thread Robert Shearman
In the next patch you've found a problem:

Mikołaj Zalewski wrote:
 +#if 0 /* for some reason (rpcrt4 bug?) QueryServiceConfig for a non-NULL 
 lpLoadOrder crashes Wine */
   

The issue is to do with this code:

   LPQUERY_SERVICE_CONFIGW lpServiceConfig,
   DWORD cbBufSize, LPDWORD pcbBytesNeeded)
  {
 -WCHAR str_buffer[ MAX_PATH ];
 -LONG r;
 -DWORD type, val, sz, total, n;
 -LPBYTE p;
 -HKEY hKey;
 +QUERY_SERVICE_CONFIGW config;
  struct sc_service *hsvc;
 +DWORD total;
 +DWORD err;
 +BYTE *bufpos;
  
  TRACE(%p %p %d %p\n, hService, lpServiceConfig,
 cbBufSize, pcbBytesNeeded);
 @@ -1886,58 +1908,21 @@ QueryServiceConfigW( SC_HANDLE hService,
  SetLastError( ERROR_INVALID_HANDLE );
  return FALSE;
  }
 -hKey = hsvc-hkey;
 -
 -/* TODO: Check which members are mandatory and what the registry types
 - * should be. This should of course also be tested when a service is
 - * created.
 - */
 -
 -/* calculate the size required first */
 -total = sizeof (QUERY_SERVICE_CONFIGW);
  
 -sz = sizeof(str_buffer);
 -r = RegQueryValueExW( hKey, szImagePath, 0, type, (LPBYTE) str_buffer, 
 sz );
 -if( ( r == ERROR_SUCCESS )  ( type == REG_SZ || type == REG_EXPAND_SZ 
 ) )
 +if ((err = svcctl_QueryServiceConfigW(hsvc-hdr.rpc_handle, 
 hsvc-hdr.server_handle, config)) != 0)
   

The problem is that QUERY_SERVICE_CONFIGW contains pointers and the 
DCE/RPC programming model ensures that non-NULL pointers that are being 
unmarshalled into are used (presumably to reduce memory allocations). So 
the issue here is that you're not initialising config before passing it 
into svcctl_QueryServiceConfigW and it is blowing up just by chance on 
the pointer occupying the lpLoadOrderGroup; it could just as well have 
been lpBinaryName, lpServiceStartName or lpDisplayName.

 +
 +/* Windows function 0x11 must be using a different prototype - not 
 compatible */
 +/* Robert Shearman thinks there should be a byte_count attribute but (as 
 of Sep 2007)
 + * this isn't supported by widl nor by rpcrt4 */
 +DWORD svcctl_QueryServiceConfigW(
 +SvcCtlRpcHandle rpc_handle,
 +[in] POLICY_HANDLE *handle,
 +[out] QUERY_SERVICE_CONFIGW *config);
 +
  }
   

The byte_count attribute is officially deprecated by Microsoft and it's 
not part of the DCE/RPC standard, plus you've already done the work and 
it's wire compatible (I think) without the attribute, so you can remove 
the comment about it.

-- 
Rob Shearman





Re: services.exe/advapi32[7/7]: move GetServiceDisplayName to services.exe and implement GetServiceKeyName

2008-03-17 Thread Robert Shearman
Mikołaj Zalewski wrote:
 +/* Windows function 0x14 must be using a different prototype - not 
 compatible */
 +DWORD svcctl_GetServiceDisplayNameW(
 +SvcCtlRpcHandle rpc_handle,
 +[in] POLICY_HANDLE *hSCManager,
 +[in] LPCWSTR lpServiceName,
 +[in,out,size_is(cchBufSize)] WCHAR lpBuffer[],
 +[in] DWORD cchBufSize,
 +[out] DWORD *cchLength);
 +
 +/* Windows function 0x15 must be using a different prototype - not 
 compatible */
 +DWORD svcctl_GetServiceKeyNameW(
 +SvcCtlRpcHandle rpc_handle,
 +[in] POLICY_HANDLE *hSCManager,
 +[in] LPCWSTR lpServiceDisplayName,
 +[in,out,size_is(cchBufSize)] WCHAR lpBuffer[],
 +[in] DWORD cchBufSize,
 +[out] DWORD *cchLength);
  }
   

The lpBuffer parameter should be out-only instead of in and out. I 
believe this will make the prototype compatible.

-- 
Rob Shearman





Re: Running services in WINE?

2008-03-14 Thread Robert Shearman
Paul Vriens wrote:
 James Hawkins wrote:
   
 On Fri, Mar 14, 2008 at 8:03 AM, Paul Vriens [EMAIL PROTECTED] wrote:
 
 Christopher wrote:
   Maarten Lankhorst wrote:
   Hi Christopher,
  
   2008/3/12, Christopher [EMAIL PROTECTED]:
  
   I've been trying to get MozyHome working on WINE, and have made a 
 little
progress. However, it seems that the service which Mozy uses won't
start. Searching a bit I found a thread from a year ago stating that
services don't work in WINE, but that it was being worked on. Are
services implemented in WINE now, or if they're not is someone working
on it who I could help out?
  
   Services most definitely work now. Some of them you can start manually
   with wine net start 'servicename', however this shouldn't be needed.
  
   Cheers,
   Maarten.
  
  
  
   Thanks Maarten!
   I tried that, but didn't have anymore luck. The service exits almost
   immediately, after giving a few fixme messages. Time to do some
   debugging I guess :)
  
   Christopher
  
  
  
  I've been looking into this and found an issue in Wine that I'm not sure 
 who to
  test or solve yet.

  MozyHome creates a key in the Services branch:

  System\\CurrentControlSet\\Services\\mozybackup

  It than adds a 'Description' value to this key.

  A bit later it checks for the service by doing an OpenService. This 
 succeeds on
  Wine but shouldn't.

  I've created a simple test that creates the key and some value in it. 
 Doing an
  OpenService fails every time on Windows.

  I took another approach whereby I created a proper service (CreateService) 
 and
  checked whether I could do an OpenService. This succeeded every time even 
 after
  deleting the whole key.

  What it boils down to is that the service control manager keeps track of 
 the
  services. Only proper service calls can influence the service control 
 manager
  (or a reboot).

  So as long as we don't implement some service control manager functionality
  (services.exe?) we could have some issues as with MozyHome.

  One approach we could take while we don't have the full functionality is to
  check for the minimal required parameters (value) we need for a service. 
 Would
  that be acceptable?

  I think Maarten's patch for CreateService was along those same lines
  
 (http://source.winehq.org/git/wine.git/?a=commit;h=284f86183cce638ff8fce4023cb76273e979aa09).
  We need to find a better way of checking whether a service is truly 
 installed.
  (and not just some registry keys).

   
 http://bugs.winehq.org/show_bug.cgi?id=11651#c3

 but generally comments 2-4.

 
 It's good we came to same conclusion.

 If we wanted to implemented a SCM where would that reside? wineboot?
   

programs/services

It could then be started on demand by wineboot and/or the advapi32 
services functions.
I was planning to work on this after my endpoint mapper patches are 
accepted.

-- 
Rob Shearman





Re: Hans Leidekker : wininet: Make sure not to overwrite any caller supplied authorization header.

2008-03-12 Thread Robert Shearman
Alexandre Julliard wrote:
 Module: wine
 Branch: master
 Commit: b069ef4268e7056856ce6714714d929165f24fc8
 URL:
 http://source.winehq.org/git/wine.git/?a=commit;h=b069ef4268e7056856ce6714714d929165f24fc8

 Author: Hans Leidekker [EMAIL PROTECTED]
 Date:   Fri Feb  1 14:40:15 2008 +0100

 wininet: Make sure not to overwrite any caller supplied authorization header.

 ---

  dlls/wininet/http.c   |   36 ++--
  dlls/wininet/tests/http.c |5 -
  2 files changed, 14 insertions(+), 27 deletions(-)

 diff --git a/dlls/wininet/http.c b/dlls/wininet/http.c
 index 65b86fb..a339c86 100644
 --- a/dlls/wininet/http.c
 +++ b/dlls/wininet/http.c
 @@ -1187,9 +1187,11 @@ static UINT HTTP_DecodeBase64( LPCWSTR base64, LPSTR 
 bin )
   *
   *   Insert or delete the authorization field in the request header.
   */
 -static BOOL HTTP_InsertAuthorizationForHeader( LPWININETHTTPREQW lpwhr, 
 struct HttpAuthInfo *pAuthInfo, LPCWSTR header )
 +static BOOL HTTP_InsertAuthorization( LPWININETHTTPREQW lpwhr, LPCWSTR 
 header, BOOL first )
  {
  WCHAR *authorization = NULL;
 +struct HttpAuthInfo *pAuthInfo = lpwhr-pAuthInfo;
 +DWORD flags;
  
  if (pAuthInfo  pAuthInfo-auth_data_len)
  {
 @@ -1222,35 +1224,17 @@ static BOOL HTTP_InsertAuthorizationForHeader( 
 LPWININETHTTPREQW lpwhr, struct H
  
  TRACE(Inserting authorization: %s\n, debugstr_w(authorization));
  
 -HTTP_ProcessHeader(lpwhr, header, authorization,
 -   HTTP_ADDHDR_FLAG_REPLACE | HTTP_ADDHDR_FLAG_REQ);
 +/* make sure not to overwrite any caller supplied authorization header */
 +flags = HTTP_ADDHDR_FLAG_REQ;
 +flags |= first ? HTTP_ADDHDR_FLAG_ADD_IF_NEW : HTTP_ADDHDR_FLAG_REPLACE;
  
 -HeapFree(GetProcessHeap(), 0, authorization);
 +HTTP_ProcessHeader(lpwhr, header, authorization, flags);
  
 +HeapFree(GetProcessHeap(), 0, authorization);
  return TRUE;
  }
  
  /***
 - *  HTTP_InsertAuthorization
 - *
 - *   Insert the authorization field in the request header
 - */
 -static BOOL HTTP_InsertAuthorization( LPWININETHTTPREQW lpwhr )
 -{
 -return HTTP_InsertAuthorizationForHeader(lpwhr, lpwhr-pAuthInfo, 
 szAuthorization);
 -}
 -
 -/***
 - *  HTTP_InsertProxyAuthorization
 - *
 - *   Insert the proxy authorization field in the request header
 - */
 -static BOOL HTTP_InsertProxyAuthorization( LPWININETHTTPREQW lpwhr )
 -{
 -return HTTP_InsertAuthorizationForHeader(lpwhr, lpwhr-pProxyAuthInfo, 
 szProxy_Authorization);
 -}
 -
 -/***
   *   HTTP_DealWithProxy
   */
  static BOOL HTTP_DealWithProxy( LPWININETAPPINFOW hIC,
 @@ -2621,8 +2605,8 @@ BOOL WINAPI HTTP_HttpSendRequestW(LPWININETHTTPREQW 
 lpwhr, LPCWSTR lpszHeaders,
 lpwhr-hdr.dwFlags  
 INTERNET_FLAG_KEEP_CONNECTION ? szKeepAlive : szClose,
 HTTP_ADDHDR_FLAG_REQ | HTTP_ADDHDR_FLAG_REPLACE);
  
 -HTTP_InsertAuthorization(lpwhr);
 -HTTP_InsertProxyAuthorization(lpwhr);
 +HTTP_InsertAuthorization(lpwhr, szAuthorization, !loop_next);
 +HTTP_InsertAuthorization(lpwhr, szProxy_Authorization, !loop_next);
  
  /* add the headers the caller supplied */
  if( lpszHeaders  dwHeaderLength )
 diff --git a/dlls/wininet/tests/http.c b/dlls/wininet/tests/http.c
 index 8bb7cca..88369cf 100644
 --- a/dlls/wininet/tests/http.c
 +++ b/dlls/wininet/tests/http.c
 @@ -1502,7 +1502,10 @@ static void test_header_handling_order(int port)
  request = HttpOpenRequest(connect, NULL, /test3, NULL, NULL, types, 
 INTERNET_FLAG_KEEP_CONNECTION, 0);
  ok(request != NULL, HttpOpenRequest failed\n);
  
 -ret = HttpSendRequest(request, authorization, ~0UL, NULL, 0);
 +ret = HttpAddRequestHeaders(request, authorization, ~0UL, 
 HTTP_ADDREQ_FLAG_ADD);
 +ok(ret, HttpAddRequestHeaders failed\n);
 +
 +ret = HttpSendRequest(request, NULL, 0, NULL, 0);
  ok(ret, HttpSendRequest failed\n);
  
  status = 0;
   

This patch simultaneously breaks proxy authentication and NTLM 
authentication. Proxy authentication is broken because the helper 
function is removed that passes lpwhr-pProxyAuthInfo into 
HTTP_InsertAuthentication. NTLM authentication is broken because first 
is false when we try to send the authenticate message (i.e. the 3rd leg) 
and so the data for the negotiate message (i.e. the first leg) is sent 
instead as the field is not overridden.

Is this trying to fix a real app? If not, why isn't the code that is 
using the behaviour using INTERNET_FLAG_NO_AUTH?

-- 
Rob Shearman





Re: compile fail on Linux Mint

2008-03-11 Thread Robert Shearman
Chad Harris wrote:
 Just tried to compile wine.. ( Yes, I know theres a package )  but 
 sometimes I'm sadistic enough to want to compile somethin.. and it 
 just keeps failing..  perhaps you can shed some light on what's goin' 
 wrong?

You need to install glibc and associated development libraries.

-- 
Rob Shearman





Re: a patch to fix the font in bottons and so on.

2008-03-10 Thread Robert Shearman
彭思 wrote:
 Hi everyone,
 I have a patch in ubuntu China forum, which fix the font in bottons 
 and so on to display correctly using the right font. This patch 
 changes only one source file: dlls/gdi32/freetype.c. The main function 
 is: Default and ansi charset is translated to 
 getenv(WINE_DEFAULT_CODEPAGE). This patch was written by *windowssux 
 javascript:pn('windowssux'); in ubunt china forum: 
 http://forum.ubuntu.org.cn/*


 The patched freetype.c is attached. It's the first time for me to send 
 you guys a patch to wine. Please let know if something is wrong. Thanks.

You need to send the patch, not the whole file. It's nearly impossible 
to review the changes if you add an attachment with several thousand 
lines of code.

-- 
Rob Shearman





Re: Make winecfg DPI interval to be the same as in Windows

2008-03-07 Thread Robert Shearman
L. Rahyen wrote:
 @@ -37,7 +37,7 @@ WINE_DEFAULT_DEBUG_CHANNEL(winecfg);
  
  #define RES_MAXLEN 5 /* the maximum number of characters in a screen 
 dimension. 5 digits should be plenty, what kind of crazy person runs their 
 screen 10,000 pixels across? */
  #define MINDPI 96
 -#define MAXDPI 160
 +#define MAXDPI 480
  #define DEFDPI 96
  
  static const char logpixels_reg[] = System\\CurrentControlSet\\Hardware 
 Profiles\\Current\\Software\\Fonts;
 @@ -243,9 +243,12 @@ static void on_d3d_pshader_mode_clicked(HWND dialog) {
  }
  static INT read_logpixels_reg(void)
  {
 +static const WCHAR szLogPixels[] = {'L', 'o', 'g', 'P', 'i', 'x', 'e', 
 'l', 's', 0};
  DWORD dwLogPixels;
 -char *buf  = get_reg_key(HKEY_LOCAL_MACHINE, logpixels_reg,
 - LogPixels, NULL);
 +WCHAR *buf;
 +WCHAR szLogPixelsReg[MAXBUFLEN];
 +MultiByteToWideChar(GetACP(), 0, logpixels_reg, -1, szLogPixelsReg, 
 sizeof(szLogPixelsReg)/sizeof(szLogPixelsReg[0]) );
 +buf = get_reg_keyW(HKEY_LOCAL_MACHINE, szLogPixelsReg, szLogPixels, 
 NULL);
  dwLogPixels = buf ? *buf : DEFDPI;
  HeapFree(GetProcessHeap(), 0, buf);
  return dwLogPixels;
   

Don't convert a constant string using MultiByteToWideChar.

-- 
Rob Shearman





Re: [3/6] qmgr: Add a mutex to BackgroundCopyJob.

2008-03-07 Thread Robert Shearman
Dan Hipschman wrote:
 Pretty much the same as yesterday but I changed to using mutexes instead
 of critical sections since I need to wait on two at once (one for the
 job and the other for the file when updating progress).
   

I'm not sure this is a correct reason for mutexes instead of critical 
sections. Either way, you need to have a strict ordering so that you 
don't deadlock and critical sections use mutexes underneath. Unless 
you're going to be using a named mutex to synchronise across processes 
or you're going to have threads quit whilst holding the mutex, then the 
only difference will be that the code is slower when using mutexes 
because of the necessary server call, which more often than not can be 
avoided with critical sections.

-- 
Rob Shearman





Re: msi: Fix cast-qual warnings

2008-03-03 Thread Robert Shearman
James Hawkins wrote:
  module = LoadLibraryExW( file-TargetPath, NULL, 
 LOAD_LIBRARY_AS_DATAFILE );
  if (module)
  {
 -LPCWSTR guid;
 -guid = MSI_RecordGetString(row,1);
 -CLSIDFromString((LPWSTR)guid, tl_struct.clsid);
 +LPWSTR guid;
 +guid = strdupW(MSI_RecordGetString(row, 1));
 +CLSIDFromString(guid, tl_struct.clsid);
 +msi_free(guid);
  tl_struct.source = strdupW( file-TargetPath );
  tl_struct.path = NULL;
I can guarantee you that CLSIDFromString does not change the input string. We 
should not be adding extra performance costs in the form of extra memory 
allocations for the sake of getting rid of a fairly harmless warning. We should 
fix the ones that don't require an extra allocations or other performance 
penalty and leave the warning off.


-- 
Rob Shearman





Re: ntdll: Added support for Windows 2008

2008-03-03 Thread Robert Shearman
Alistair Leslie-Hughes wrote:
  sizeof(RTL_OSVERSIONINFOEXW), 6, 0, 0x1770, VER_PLATFORM_WIN32_NT,
  {' ',0},
  0, 0, VER_SUITE_SINGLEUSERTS, VER_NT_WORKSTATION, 0
 +},
 +/* WIN2K8 */
 +{
 +sizeof(RTL_OSVERSIONINFOEXW), 6, 0, 0x1771, VER_PLATFORM_WIN32_NT,
 +{'S','e','r','v','i','c','e',' ','P','a','c','k',' ','1',0},
 +0, 0, VER_SUITE_SINGLEUSERTS, VER_NT_SERVER, 0
  }
  };
   

I see Microsoft are using even more sneaky tactics these days to 
increase sales. They are releasing new versions of Windows with the 
first service pack already applied to satisfy the customers who always 
wait for the first service pack of a version of Windows to come out 
before upgrading to it.

Or could this just be a copy and paste mistake?

-- 
Rob Shearman





Re: Building dlls/oleaut32/tests/olepicture.c under Cygwin?

2008-03-03 Thread Robert Shearman
Dan Kegel wrote:
 7. Try compiling again.  Now you fail with a handful of errors like
 /cygdrive/c/DOCUME~1/Liz/LOCALS~1/Temp/ccXL7SF3.o:olepicture.c:(.text+0x18e5):
  u
 ndefined reference to `_IPictureDisp_Invoke'

 Seems like a portability problem in tests/olepicture.c.

 From ocidl.h:
#define 
IPictureDisp_Invoke(This,dispIdMember,riid,lcid,wFlags,pDispParams,pVarResult,pExcepInfo,puArgErr)
 
(This)-lpVtbl-Invoke(This,dispIdMember,riid,lcid,wFlags,pDispParams,pVarResult,pExcepInfo,puArgErr)

Which headers are you using?

-- 
Rob Shearman





Re: [PATCH] shlwapi: Handle PathCanonicalize with too long paths

2008-02-29 Thread Robert Shearman
Marcus Meissner wrote:
 lpszPath)
{
  WCHAR szPath[MAX_PATH];
  WCHAR szBuff[MAX_PATH];
 -MultiByteToWideChar(CP_ACP,0,lpszPath,-1,szPath,MAX_PATH);
 +DWORD le = GetLastError();
 +int ret = MultiByteToWideChar(CP_ACP,0,lpszPath,-1,szPath,MAX_PATH);
 +
 +if (!ret) {
 + FIXME(Failed to convert string to widechar (too long?), LE %d.\n, 
 GetLastError());
 + SetLastError (le); /* failure does not change Lasterror, see testcase. 
 */
 + return FALSE;
 +}
   

Hi Marcus,

FIXME should be used for unimplemented functionality, not for 
highlighting application bugs that will happen on Windows too. WARN 
would be more appropriate here.

-- 
Rob Shearman





Re: dxdiagn: Fix the variable that the result of GetFileVersionInfoW is assigned to in DXDiag_AddFileDescContainer.

2008-02-29 Thread Robert Shearman
Paul Vriens wrote:
 Robert Shearman wrote:
 ---
  dlls/dxdiagn/provider.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)


 



 Is there a need to have it assigned, as the next line overwrites 
 boolret again?

When somebody bothers to actually do some error checking in the function 
it will be a better starting point.

-- 
Rob Shearman





Re: kernel32 winelib: Don't test string size in CompareStringW for null terminated strings

2008-02-29 Thread Robert Shearman
Adam Strzelecki wrote:
 During playing with installing Visual Studio 2005 with WINE I found 
 out that WINE's MSI is spending lot of time inside lstrcmpW. With Mac 
 OS X process sampler I checked that actually 60-70% of CompareStringW 
 is wine_compare_string, rest is rest of function body, which is in 
 case 2 strlenW calls (inline).
 I believe this isn't necessary, moreover most of compares in MSI are 
 non-matching on first few characters, however with current 
 implementation we go trough all strings with strlenW regardless of 
 anything, which is waste in CPU cucles.
 I modified both CompareStringW and wine_compare_string so they work 
 with -1 strings without strlenW checking before.
 This saves in my testing about 20-30% of CPU cycles.

You introduce a lot of complexity into the low-level helper routines. We 
don't want to put Win32 quirks, like -1 meaning until the null 
terminator being put into the low-level functions unless we have to. In 
this case, if this is the sole cause of the slowdown then you would be 
better off caching the length at the MSI level since the strings are 
bound to be accessed more than once.

However, I think you'll find that this is an algorithm problem in MSI 
rather than a performance issue in the functions you mention. By that, I 
mean it is likely that one MSI query is being performed again and again 
and that the results should be cached.

 Note also that WINE's strlenW and lstrcmpW functions are far from 
 perfect. Normal msvcrt or glibc string functions are far more 
 optimized at machine code level. This makes strong impact on WINE 
 parts strongly relaying on string manipulation which is in this case MSI.

How do you know that the compiler isn't generating the same assembly for 
the functions as that used by msvcrt / glibc? Theories like these need 
numbers to back them up.

-- 
Rob Shearman





Re: kernel32 winex11: WaitForMultipleObjectsEx should ignore NULL handles

2008-02-29 Thread Robert Shearman
Adam Strzelecki wrote:
 Hi,

 In MSI dialog.c: msi_dialog_check_messages function it often happens 
 that is sends:
 MsgWaitForMultipleObjects( 1, handle, 0, INFINITE, QS_ALLINPUT );
 Where handle = NULL. I'm not sure if it is correct behavior. But 
 tracing it down I found out that kernel32 and winex11 sync functions 
 are not checking at all if the passes handles are not NULLs.

 So I'm not sure whether this patch is right or not, but I believe we 
 shouldn't bother wineserver with NULL handles!?
 Yet maybe I don't understand clearly the way interserver communication 
 works, and sending WaitFor with NULL handle makes sense. Does it?

No, but this is the wrong fix.

You're assuming that because MSI code does action A and that causes bad 
effect when it does action B that the correct action is to fix B so that 
it doesn't have a bad effect. However, the bug could also be that action 
A shouldn't be done.

In this case, MSI should not be calling MsgWaitForMultipleObjects with 
an invalid handle and in fact that barring memory corruption, this can't 
happen:
 while (1)
 {
 msi_process_pending_messages( NULL );

 if( !handle )
 break;

 /*
  * block here until somebody creates a new dialog or
  * the handle we're waiting on becomes ready
  */
 r = MsgWaitForMultipleObjects( 1, handle, 0, INFINITE, 
 QS_ALLINPUT );
 if( r == WAIT_OBJECT_0 )
 break;
 }


-- 
Rob Shearman





Re: marshalled ITypeComp

2008-02-26 Thread Robert Shearman
Guillermo Winkler wrote:
 Hi,
 I'm having some problems regarding ITypeInfo/ITypeComp implementation.

 IDispatch* pdisp;
 ITypeInfo* ptinfo;

 HRESULT hr = pdisp-GetTypeInfo(0, LOCALE_SYSTEM_DEFAULT, typeinfo);
 hr = ptinfo-GetTypeComp(ptcomp);


 pdisp is created from

  hr = CoCreateInstance(clsid,
   NULL,
   CLSCTX_SERVER,
   IID_IDispatch,
   (void**)pdisp);

 That is, the server is an outprocess exe.

 In this case marshalling of ITypeComp fails in first instance because

 ITypeComp_fnQueryInterface (oleaut32\typelib.c)
 resolves on - ITypeInfo_QueryInterface (oleaut32\typelib.c)

 That is not considering IID_ITypeComp as a valid alternative(but it is)

if(IsEqualIID(riid, IID_IUnknown) ||
  IsEqualIID(riid,IID_ITypeInfo)||
  IsEqualIID(riid,IID_ITypeInfo2))
  *ppvObject = This;

 When I fixed this, I have a different problem now:

 fixme:ole:DllGetClassObject
   CLSID: {00020425---C000-0046}
   IID:  {D5F569D0-593B-101A-B569-08002B2DBF7A}

 That is PSTypeComp to be marshalled asking for factory buffer 
 IID_IPSFactoryBuffer.

Yes, CLSID_PSTypeComp should be added to the cases in which 
OLEAUTPS_DllGetClassObject is called in 
dlls/oleaut32/oleaut.c:DllGetClassObject.

-- 
Rob Shearman





Re: [3/4].Wbemprox.dll. Main modules for then library.

2008-02-25 Thread Robert Shearman
Ivan Sinitsin wrote:
 --- /dev/null 2007-03-10 18:36:24 +0300
 +++ Makefile.in   2008-02-21 15:57:24 +0300
 @@ -0,0 +1,23 @@
 +TOPSRCDIR = @top_srcdir@
 +TOPOBJDIR = ../..
 +SRCDIR= @srcdir@
 +VPATH = @srcdir@
 +MODULE= wbemprox.dll
 +IMPORTLIB = libwbemprox.$(IMPLIBEXT)
 +IMPORTS   = ole32 oleaut32 user32 advapi32 kernel32 shell32
 +EXTRALIBS = -luuid
 +
 +C_SRCS = \
 +   main.c \
 +   regsvr.c \
 +   i_numwbemclassobject.c \
 +   i_wbemservices.c \
 +   mainclass.c \
 +   factory.c
   

The files i_numwbemclassobject.c and i_wbemservices.c aren't included 
with this patch, so Wine would not compile if this patch was applied. 
Please also reconsider the naming of these files - I presume i_ 
standards for interface, but you aren't declaring an interface in these 
files but implementing an object.

 +
 +
 +IDL_I_SRCS = wbemprox.idl
 +
 [EMAIL PROTECTED]@
 +
 [EMAIL PROTECTED]@  # everything below this line is overwritten by make depend
   


-- 
Rob Shearman





Re: [2/4]try2.Wbemprox.dll. Idl file.

2008-02-25 Thread Robert Shearman
Ivan Sinitsin wrote:
 --- /dev/null 2007-03-10 18:36:24 +0300
 +++ wbemprox.idl  2008-02-21 16:09:20 +0300
 @@ -0,0 +1,260 @@
   

Then doesn't appear to be a wbemprox.idl file in any version of the 
Platform SDK that I have access to. The interfaces in your file are in 
wbemcli.idl instead.

Also, please diff from the toplevel wine directory.

-- 
Rob Shearman





Re: dmband: Indirection levels fix

2008-02-21 Thread Robert Shearman
Andrew Talbot wrote:
 diff --git a/dlls/dmband/band.c b/dlls/dmband/band.c
 index 891fb5a..8b89573 100644
 --- a/dlls/dmband/band.c
 +++ b/dlls/dmband/band.c
 @@ -173,7 +173,7 @@ static HRESULT WINAPI 
 IDirectMusicBandImpl_IDirectMusicObject_SetDescriptor (LPD
   This-pDesc-ftDate = pDesc-ftDate;
   if (pDesc-dwValidData  DMUS_OBJ_MEMORY) {
   This-pDesc-llMemLength = pDesc-llMemLength;
 - memcpy (This-pDesc-pbMemData, pDesc-pbMemData, sizeof 
 (pDesc-pbMemData));
 + memcpy (This-pDesc-pbMemData, pDesc-pbMemData, sizeof 
 (pDesc-pbMemData));
   }
   if (pDesc-dwValidData  DMUS_OBJ_STREAM) {
   /* according to MSDN, we copy the stream */

This isn't correct. Judging by the surrounding code, this should be 
allocating a block of memory of This-pDesc-pbMemData and then passing 
pDesc-llMemLength into memcpy, possibly validating that 
pDesc-llMemLength isn't greater than UINT_MAX to avoid an overflow.

-- 
Rob Shearman





Re: Make.rules.in: Pass the --nounistd flag to flex since unistd.h is a header that might not be present on all systems.

2008-02-19 Thread Robert Shearman
Alexandre Julliard wrote:
 Robert Shearman [EMAIL PROTECTED] writes:

   
 If we do need it, we can include it in the .l file after config.h and
 with an appropriate include guard.
 

 You should instead put a %option in the files that need it.

The point is that all of the files need it. Only tools/widl/parser.l 
needs to include unistd.h and it already does so manually.

-- 
Rob Shearman





Re: Make.rules.in: Pass the --nounistd flag to flex since unistd.h is a header that might not be present on all systems.

2008-02-19 Thread Robert Shearman
Alexandre Julliard wrote:
 Robert Shearman [EMAIL PROTECTED] writes:

   
 The point is that all of the files need it. Only tools/widl/parser.l
 needs to include unistd.h and it already does so manually.
 

 Then you should put the option in all the files. That sort of thing
 doesn't belong in the global make rules.

I disagree. The global make rules, among other things, should enforce 
policies whether this be telling the C compiler to error on certain 
non-portable constructs like declarations after statements or telling 
flex not to output code that includes headers that might not be present, 
like unistd.h.

However, I've sent a patch that turns the option on individually for 
each file so that you can choose which route to take.

-- 
Rob Shearman





Re: slc: Initial stub DLL (try 2)

2008-02-19 Thread Robert Shearman
Alistair Leslie-Hughes wrote:
 slc: Initial stub dll

I'm not sure we want to go down the path of implementing the functions 
in this DLL. Furthermore, this file isn't present on an XP SP2 installation.

Can you give some information as to why this is needed and how the 
application depends on this DLL?

-- 
Rob Shearman





Re: iTunes

2008-02-18 Thread Robert Shearman
Maarten Lankhorst wrote:
 On a slightly related note: Do you know if the hardware drivers are
 installed somewhere? I would be interested in getting iTunes syncing
 to my ipod touch (similar to iphone). I didn't see anything in my
 wineprefix after installation.

I don't know about the iPod Touch and iPhone, but iPods are USB mass 
storage devices in Windows and so don't require any device drivers.

iTunes 4.x communicated with an iPod using DeviceIoControl(..., 
IOCTL_SCSI_PASS_THROUGH, ...) and DeviceIoControl(..., 
IOCTL_SCSI_PASS_THROUGH_DIRECT, ...), but only after doing a whole bunch 
of low-level calls using setupapi and other ioctls to find the device.

The 4.x versions of iTunes did install a driver for the CD burning 
functionality, however.

Good luck, but bear in mind that any hard work done on iTunes may need 
to re-done in 3-6 months time when the next version of iTunes comes out.

-- 
Rob Shearman





Re: include: Add support for executing finally code for exceptions without using a separate function.

2008-02-18 Thread Robert Shearman
Alexandre Julliard wrote:
 Robert Shearman [EMAIL PROTECTED] writes:

   
 The first goal is accomplished by changing the previous two-state
 variable of __first into a three-state variable where the third state
 is the state in which the finally code is executed in the normal
 case. The second goal is accomplished by calling sigsetjmp in
 __wine_finally_handler to give the finally code a place to jump back
 to after it has finished executing.
 

 That can't work, once you longjmp into the finally block you can't
 longjmp back, since you are no longer inside the function that did the
 setjmp.

Ah, yes. I thought there might be some caveat like that on the use of 
longjmp, but I couldn't break it with any of the tests I did. I can see 
why it wouldn't work on all platforms or in all circumstances.

-- 
Rob Shearman





Re: [PATCH] Environment variable to specify X11 window for desktop

2008-02-15 Thread Robert Shearman
Timothy Lee wrote:
 The attached patch allows the WINE desktop to be embedded within an 
 existing X11 window, and is similar to the windowed mode in WINE.

 I've tested this feature using MS PowerPoint Viewer 2007, so that a 
 fullscreen slideshow can be fitted inside any X11 window.

I'm struggling to imagine a case where the behaviour you've implemented 
is useful.

-- 
Rob Shearman





Re: [PATCH #1] wininet: Added beginning support for HTTP cache files.

2008-02-13 Thread Robert Shearman
Jacek Caban wrote:
 +/* FIXME: Better check, when we have to create the cache file */
 +if(bSuccess  (lpwhr-hdr.dwFlags  INTERNET_FLAG_NEED_FILE)) {
 +WCHAR url[INTERNET_MAX_URL_LENGTH];
 +WCHAR cacheFileName[MAX_PATH+1];
 +BOOL b;
 +
 +b = HTTP_GetRequestURL(lpwhr, url);
 +if(!b) {
 +WARN(Could not get URL\n);
 +goto lend;
 +}
 +
 +b = CreateUrlCacheEntryW(url, lpwhr-dwContentLength  0 ? 
 lpwhr-dwContentLength : 0, NULL, cacheFileName, 0);
 +if(b) {
 +lpwhr-lpszCacheFile = WININET_strdupW(cacheFileName);
 +lpwhr-hCacheFile = CreateFileW(lpwhr-lpszCacheFile, 
 GENERIC_WRITE, FILE_SHARE_READ|FILE_SHARE_WRITE,
 +  NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
 +if(lpwhr-hCacheFile == INVALID_HANDLE_VALUE) {
 +WARN(Could not create file: %u\n, GetLastError());
 +lpwhr-hCacheFile = NULL;
 +}
 +}else {
 +WARN(Could not create cache entry: %08x\n, GetLastError());
 +}
 +}

   

It may be beyond the scope of your patch, but you're creating the cache 
file without committing the entry elsewhere so that it can be found at a 
later time. According to MSDN, this should be done in HTTP_FinishedReading.

-- 
Rob Shearman





Re: systray[3/4]: Better validate icon owner

2008-02-07 Thread Robert Shearman
Kirill K. Smirnov wrote:
 I don't get why you need this change. PostMessage should correctly
 handle the case where icon-owner has been destroyed and adding a call
 to IsWindow just introduces a race condition.
 

 Just a testcase:
 1) run any wine app (to be sure that explorer is running), e.g winecfg
 2) run taskmgr
 3) select taskmgr process in tasklist and kill it. taskmgr window will 
 dissapear but tray icon not.
 4) move mouse cursor over taskmgr tray icon. Before this patch it will not 
 dissapear, but should.
   

This is pretty much the same test I did when I originally wrote the code.

 The reason is that ERROR_INVALID_HANDLE is not the only value which indicates 
 the absence of owner window (ERROR_INVALID_WINDOW_HANDLE too). I think that 
 using robust and straightforward IsWindow() function is better than verify 
 last error.
   

The error value was changed in commit 
19e7fab981a75cdf0a339db35d36d9c1a6e64494. The code should be changed to 
check for ERROR_INVALID_WINDOW_HANDLE now.

 Is it acceptable to check return value of PostMessage only and do not check 
 last error?
   

Possibly, although I can think of other errors such as out-of-memory 
that probably shouldn't indicate that the parent has disappeared and the 
icon destroyed.

 And what race condition introduces IsWindow() here?
   

In general, you shouldn't check whether something can be done before you 
try doing it. Just try doing it and cope with it failing.

In this case, you're checking if the window is valid before you post a 
message to it. The window could be destroyed between IsWindow and 
PostMessage, in which case you'd have to detect the error by checking 
the result of the PostMessage call. After you've done this, the IsWindow 
call is redundant.

-- 
Rob Shearman





Re: systray[3/4]: Better validate icon owner

2008-02-07 Thread Robert Shearman
Reece Dunn wrote:
 On 06/02/2008, Robert Shearman [EMAIL PROTECTED] wrote:
   
 PostMessage should correctly
 handle the case where icon-owner has been destroyed and adding a call
 to IsWindow just introduces a race condition.
 

 Is there a test case in the PostMessage tests to verify that behaviour?
   

No - I don't think the tests will tell us anything we don't already 
know, but don't let me stop you adding a test for 
PostMessage((HWND)0xdeadbeef, WM_USER, 0, 0).

-- 
Rob Shearman





Re: systray[3/4]: Better validate icon owner

2008-02-06 Thread Robert Shearman
Kirill K. Smirnov wrote:
 @@ -131,15 +130,18 @@ static LRESULT WINAPI adaptor_wndproc(HW
  case WM_LBUTTONDBLCLK:
  case WM_RBUTTONDBLCLK:
  case WM_MBUTTONDBLCLK:
 -/* notify the owner hwnd of the message */
 -WINE_TRACE(relaying 0x%x\n, msg);
 -ret = PostMessage(icon-owner, icon-callback_message, (WPARAM) 
 icon-id, (LPARAM) msg);
 -if (!ret  (GetLastError() == ERROR_INVALID_HANDLE))
 +if (!IsWindow(icon-owner))
  {
  WINE_WARN(application window was destroyed without removing 
 
notification icon, removing automatically\n);
  delete_icon_directly(icon);
  }
 +else
 +{
 +/* notify the owner hwnd of the message */
 +WINE_TRACE(relaying 0x%x\n, msg);
 +PostMessage(icon-owner, icon-callback_message, (WPARAM) 
 icon-id, (LPARAM) msg);
 +}
  break;
  
  case WM_NCDESTROY:

I don't get why you need this change. PostMessage should correctly 
handle the case where icon-owner has been destroyed and adding a call 
to IsWindow just introduces a race condition.


-- 
Rob Shearman





Re: systray[2/4]: correctly handle icon addition/deletion

2008-02-06 Thread Robert Shearman
Kirill K. Smirnov wrote:
 @@ -71,6 +71,12 @@ struct icon
  static struct tray tray;
  static BOOL hide_systray;
  
 +static BOOL add_icon(NOTIFYICONDATAW *nid);
 +static BOOL modify_icon(NOTIFYICONDATAW *nid);
 +static BOOL delete_icon(const NOTIFYICONDATAW *nid);
 +static BOOL delete_icon_directly(struct icon *icon);
 +static BOOL display_icon(struct icon *icon, BOOL hide);
 +
  /* adaptor code */
  
  #define ICON_SIZE GetSystemMetrics(SM_CXSMICON)
   

I don't see any reason why you need to add forward declarations for all 
of these functions.

-- 
Rob Shearman





Re: help with typelib support

2008-01-23 Thread Robert Shearman
Alistair Leslie-Hughes wrote:
 Hi,

 This msxml bug requires the TYPELIB resource to be added.
 http://bugs.winehq.org/show_bug.cgi?id=11257

 I modified the followng file as follows.

 Makefile.in
 IDL_TLB_SRCS = msxml3_v1.idl

 msxml3_v1.idl(new File, with a single line)
 #include msxml2.idl

 version.rc
 1 TYPELIB LOADONCALL DISCARDABLE msxml3_v1.tlb

 The issue I am seing is the msxml3_v1.tlb is not being created.

 Output.
 ../../tools/widl/widl -I. -I. -I../../include -I../../include 
 -I/usr/include/libxml2 
-I/usr/include/libxml2   -D__WINESRC__ -DCOM_NO_WINDOWS_H  -t -T 
 msxml3_v1.tlb msxml3_v1.idl
 ../../tools/wrc/wrc --nostdinc -I. -I. -I../../include -I../../include 
 -I/usr/include/libxml2 
-I/usr/include/libxml2   -D__WINESRC__ -DCOM_NO_WINDOWS_H  -foversion.res 
 version.rc
 version.rc:26:47: Error: Cannot open file msxml3_v1.tlb

 Any ideas on what is wrong?
   

Either you didn't remake the Makefile by running make from the toplevel 
dir or the dependencies weren't picked up, in which case a make depend 
should fix things.

-- 
Rob Shearman





Re: kernel32: Intialise 16-bit stack to zero.

2008-01-17 Thread Robert Shearman
Alexandre Julliard wrote:
 Robert Shearman [EMAIL PROTECTED] writes:
   
 Otherwise dbghelp could sometimes get confused and think it should
 switch to 16-bit mode.
 This results in messages such as the following when warn or tracing
 for the heap channel is turned on:
 fixme:dbghelp:Failed to linearize address : ...
 

 Initializing the first frame should be enough.

I can't work out what the size of the first frame is though. Is it 
sizeof(STACK16FRAME)?

-- 
Rob Shearman





Re: gdi32: Do not allow to create too large device dependent bitmaps like Windows does

2008-01-15 Thread Robert Shearman
Dmitry Timoshkov wrote:
 +SetLastError(0xdeadbeef);
 +hbmp = CreateBitmap(0x7ff, 9, 1, 1, NULL);
 +ok(!hbmp, CreateBitmap should fail\n);
 +ok(GetLastError() == ERROR_NOT_ENOUGH_MEMORY,
 +   expected ERROR_INVALID_PARAMETER, got %u\n, GetLastError());
   

The string here doesn't match the test.

-- 
Rob Shearman





Re: GDI32.dll.NamedEscape

2008-01-14 Thread Robert Shearman
Luis C. Busquets Pérez wrote:
 I am having problems with a bunch of three games that came together in 
 the same pack. They all abort qhen asking for installation with the 
 following line

 wine: Call from 0x73ba1895 to unimplemented function 
 GDI32.dll.NamedEscape, aborting

 Does anybody know what GDI32.dll.NamedEscape does?
   

It appears to be undocumented. Are you using some native DLLs by any chance?

-- 
Rob Shearman





Re: [hlink] Skip tests if hlink couldn't be created

2008-01-14 Thread Robert Shearman
Dan Kegel wrote:
 This makes the test slightly more robust to operator error.
   

If we're striving to get zero test failures then converting a crash to 
some failures isn't really much of an improvement.

-- 
Rob Shearman





Re: SVN usage for Wine

2008-01-13 Thread Robert Shearman
James McKenzie wrote:
 Because git is unstable on the Mac platform, I've been utilizing a 
 little time investigating the use of either SVN or CVS to download .git 
 updates.  This appears to not be working per the Wiki pages for SVN.
   

I've been using git releases on my Mac for the last year or so (and I'm 
currently using 1.5.3.8) and I haven't noticed any instability.

-- 
Rob Shearman





Re: Advapi32: Service Control RPC

2008-01-10 Thread Robert Shearman
Rolf Kalbermatter wrote:
 Robert Shearman [mailto:[EMAIL PROTECTED] wrote:
   
 No. Everything (except multi-dimensional arrays, possibly) 
 can already be done by the code we output already. Adding 
 code to output -Oicf proc format strings in widl would be 
 possible, but unnecessary.
 

 Ok, but I have made some parameter protocols to NdrClientCall2
 from native advapi32 and believe that using those MIDL format
 informations could allow to make an idl file that would result
 in a fully wire compatible protocol to the MS services.exe one.
 It's not exactly trivial to decode the MIDL_ProcFormatString
 but by verifying the generated MIDL generated code against those
 protocolled parameters would certainly help a lot.

We really don't want to do development in that way. It would be much 
better to use the information that Samba have gained in a clean way 
combined with documentation with the equivalent Win32 function to figure 
out the compatible IDL file.

 A first not
 through check seems to indicate that almost every service control
 API has its own services.exe rpc message, so no ANSII to Widechar
 translation in the Advapi32.dll! Probably because there was a
 services.exe implementation before the widechar APIs got introduced.
 Also the protocol message numbers seem to have some holes, probably
 some undocumented APIs here.
   

This kind of work has already been done and the interface fairly well 
documented, so I'll point you at this documentation:
http://www.hsc.fr/ressources/articles/win_net_srv/msrpc_svcctl.html

-- 
Rob Shearman





Re: Advapi32: Service Control RPC

2008-01-09 Thread Robert Shearman
Rolf Kalbermatter wrote:
 The current service control API directly accesses the registry for most
 things.
 In native those fucntions are simple wrappers around RPC calls to the actual
 service.exe application. Mikolaj Zalewski offered about 3 months ago a
 possible
 aproach to implement the service.exe program.
   

Yes, we need to dig that out again. I forget what was blocking it being 
committed.

 In there the proxy code generated through widl directly calls various RPC
 functions
 to communicate with the service manager executable. When doing some tests
 with native
 advapi32 under Wine I found that those fucntions seem to be implemented by
 simply
 calling more or less directly NdrCallClient2. Widl obviously doesn't
 generate code
 yet using this API and I'm sure wire compatibility can be achieved without
 using
 NdrCallClient2.
   

Exactly. The whole point of IDL in DCE/RPC is that it is independent of 
the DCE/RPC implementation as long as the generated code conforms to the 
NDR rules. With Microsoft's implementation, most of the NDR rules are 
implemented in rpcrt4 and all the IDL compiler as to do is output code 
to call the buffer size, marshall, unmarshall and free functions in the 
right order. widl should be correct in this respect. (There are some 
shortcuts that midl/widl take for marshalling base types and the 
alignments would have to be correct for these, but these are simple to 
get right.)

There is no reason to use NdrClientCall2 for any purpose in Wine.

 But a few questions that came up here:
 - How good or complete is Wine's NdrCallClient(2) implementation?

Pretty good, IMHO. We use exactly the code that's in Wine here at 
CodeWeavers for connecting to Exchange servers (the RPC code is 
MIDL-compiled code in DLLs used by Outlook).

 Would it
 be feasable
   to use it for the implementation of the service rpc calls?
   

No. Everything (except multi-dimensional arrays, possibly) can already 
be done by the code we output already. Adding code to output -Oicf proc 
format strings in widl would be possible, but unnecessary.

It also adds problems because gcc and MSVC have differing opinions on 
how to return unions from functions. For this reason, we have 
implemented NdrClientCall2 in Wine by pretending it returns a LONG_PTR 
instead. widl would have to decide on whether it wanted to be compatible 
with our signature of the function or the MS one.

 - Assuming it would be not a whole lot of work to make NdrCallClient(2) work
 for this,
   would it be desirable to use this API instead of the code generated by
 widl currently
   from complexity and/or performance view?
   

It reduces code size (and on modern processors that is supposed to lead 
to performance gains), but that is the only advantage that it gives us.

 - Of course this would mean that widl would have to support (some of) the
 {Oi | Oic | Oif | Oicf}
   flags to generate the proxy code but as it seems the necessary code
 generator at least
   for the client side would be fairly trivial as it is mostly just a wrapper
 around
   NdrClientCall(2) and most of the nitty gritty work is done in in that
 function.
   A midl generated proxy code using -Oicf looks at least quite trivial. 

 Any opinions about this anyone?

-- 
Rob Shearman





Re: dlls/kernel32/task.c -- minor improvement

2008-01-07 Thread Robert Shearman
Alexandre Julliard wrote:
 Gerald Pfeifer [EMAIL PROTECTED] writes:

   
 Index: dlls/kernel32/task.c
 ===
 RCS file: /home/wine/wine/dlls/kernel32/task.c,v
 retrieving revision 1.2
 diff -u -3 -p -r1.2 task.c
 --- dlls/kernel32/task.c 13 Oct 2006 10:27:19 -  1.2
 +++ dlls/kernel32/task.c 3 Jan 2008 21:33:44 -
 @@ -57,7 +57,7 @@ typedef struct
  WORD  magic;  /* Thunks signature */
  WORD  unused;
  WORD  free;   /* Head of the free list */
 -WORD  thunks[4];  /* Each thunk is 4 words long */
 +WORD  thunks[];   /* Each thunk is 4 words long */
  } THUNKS;
 

 That syntax is not portable.

We have the ANYSIZE_ARRAY macro in include/winnt.h for this exact purpose.

-- 
Rob Shearman





Re: ole32: Fix a failed call to DllGetClassObject while retrieving class object interface pointer.

2008-01-06 Thread Robert Shearman
Huang, Zhangrong wrote:
 Hi,
 Sorry for the delay.
 Here is a test, pls import the attached registry files, and copy
 native activeds.dll, adsldpc.dll,
 adsldp.dll to your windows system32 dir, and patch
 wine/dlls/ole32/tests/moniker.c

 Then run the test:
 $ ../../../wine ole32_test.exe.so moniker
 err:ole:apartment_getclassobject DllGetClassObject returned error 0x80004002
 err:ole:create_server class {228d9a81-c302-11cf-9aa4-00aa004a5691} not
 registered
 fixme:ole:CoGetClassObject CLSCTX_REMOTE_SERVER not supported
 err:ole:CoGetClassObject no class object
 {228d9a81-c302-11cf-9aa4-00aa004a5691} could be created for context
 0x15
 moniker.c:783: Test failed: LDAP**
 moniker: 2 tests executed (0 marked as todo, 1 failure), 0 skipped.

 after my patch:
 $ ../../../wine ole32_test.exe.so moniker
 moniker.c:783: Test failed: LDAP**
 moniker: 2 tests executed (0 marked as todo, 1 failure), 0 skipped.

 Although the test failed, but loading class
 {228d9a81-c302-11cf-9aa4-00aa004a5691} success.
   

I've just sent a patch for this. Thanks for reporting it and going to 
the trouble of describing how to reproduce it and proposing a patch.

-- 
Rob Shearman





Re: comdlg32: Remove unneeded casts (Resend)

2008-01-02 Thread Robert Shearman
Andrew Talbot wrote:
if (fodInfos-ofnInfos-Flags  OFN_ALLOWMULTISELECT)
   size += 1;
/* return needed size in first two bytes of lpstrFile */
 -  *(WORD *)fodInfos-ofnInfos-lpstrFile = size;
 +  *fodInfos-ofnInfos-lpstrFile = size;
FILEDLG95_Clean(hwnd);
ret = EndDialog(hwnd, FALSE);
COMDLG32_SetCommDlgExtendedError(FNERR_BUFFERTOOSMALL);
 @@ -763,7 +763,7 @@ static BOOL PRINTDLG_SetUpPaperComboBoxW(HWND hDlg,
  NrOfEntries = DeviceCapabilitiesW(PrinterName, PortName,
fwCapability_Names, Names, dm);
  NrOfEntries = DeviceCapabilitiesW(PrinterName, PortName,
 -   fwCapability_Words, (LPWSTR)Words, dm);
 +  fwCapability_Words, Words, dm);
  
  /* reset any current content in the combobox */
  SendDlgItemMessageW(hDlg, nIDComboBox, CB_RESETCONTENT, 0, 0);
   

I think these two casts should stay as LPWSTR is compatible with WORD * 
only by coincidence - one is intended to hold characters and the other 
numbers.

-- 
Rob Shearman





Re: [wintab32] Refine how we determine cursor types; use that to discard non tablet devices.

2007-12-27 Thread Robert Shearman
Jeremy White wrote:
 +/* FIXME:  strcasestr is not available on all platforms; implement a simple 
 version */
 +static char * poormans_strcasestr(const char *haystack, const char *needle)
   

This is something that can be put in include/wine/port.h.

-- 
Rob Shearman





Re: winex11.drv: driver.c: GET_USER_FUNC

2007-12-21 Thread Robert Shearman
Adam Rimon wrote:
 Is there any reason for the /do { .. } while(0);/ in the 
 GET_USER_FUNC macro?

It's a standard way of making sure that the macro is safe if you put it 
after an if statement or similar without opening a new block.

-- 
Rob Shearman





Re: rpcrt4: Fix regression in NdrConformantStringUnmarshall by doing the memcpy check in safe_copy_from_buffer

2007-12-20 Thread Robert Shearman
Maarten Lankhorst wrote:
 @@ -665,7 +665,8 @@ static inline void 
 safe_copy_from_buffer(MIDL_STUB_MESSAGE *pStubMsg, void *p, U
  if ((pStubMsg-Buffer + size  pStubMsg-Buffer) || /* integer overflow 
 of pStubMsg-Buffer */
  (pStubMsg-Buffer + size  pStubMsg-BufferEnd))
  RpcRaiseException(RPC_X_BAD_STUB_DATA);
 -memcpy(p, pStubMsg-Buffer, size);
 +if (p != pStubMsg-Buffer)
 +memcpy(p, pStubMsg-Buffer, size);
  pStubMsg-Buffer += size;
  }
  
 @@ -890,8 +891,7 @@ unsigned char *WINAPI NdrConformantStringUnmarshall( 
 PMIDL_STUB_MESSAGE pStubMsg
*ppMemory = NdrAllocate(pStubMsg, memsize);
}
  
 -  if (*ppMemory != pStubMsg-Buffer)
 -safe_copy_from_buffer(pStubMsg, *ppMemory, bufsize);
 +  safe_copy_from_buffer(pStubMsg, *ppMemory, bufsize);
  
if (*pFormat == RPC_FC_C_CSTRING) {
  TRACE(string=%s\n, debugstr_a((char*)*ppMemory));
   

Good work in spotting and fixing the mistake I made, but I think I'd 
prefer to fix it by making the caller of safe_copy_from_buffer do the 
incrementing of the buffer. This is to avoid confusion with the name of 
the function and to avoid the possibility that the buffer is incremented 
twice.

-- 
Rob Shearman





Re: rpcrt4: Fix regression in NdrConformantStringUnmarshall by doing the memcpy check in safe_copy_from_buffer

2007-12-20 Thread Robert Shearman
Maarten Lankhorst wrote:
 Hi Rob,

 Robert Shearman schreef:
   
 Maarten Lankhorst wrote:
 
 @@ -665,7 +665,8 @@ static inline void
 safe_copy_from_buffer(MIDL_STUB_MESSAGE *pStubMsg, void *p, U
  if ((pStubMsg-Buffer + size  pStubMsg-Buffer) || /* integer
 overflow of pStubMsg-Buffer */
  (pStubMsg-Buffer + size  pStubMsg-BufferEnd))
  RpcRaiseException(RPC_X_BAD_STUB_DATA);
 -memcpy(p, pStubMsg-Buffer, size);
 +if (p != pStubMsg-Buffer)
 +memcpy(p, pStubMsg-Buffer, size);
  pStubMsg-Buffer += size;
  }
  
   
 Good work in spotting and fixing the mistake I made, but I think I'd
 prefer to fix it by making the caller of safe_copy_from_buffer do the
 incrementing of the buffer. This is to avoid confusion with the name
 of the function and to avoid the possibility that the buffer is
 incremented twice.
 
 If you want to remove that, you might as well remove that whole inline,
 since even when the two areas are equal the check that needs to be
 performed at overflowing still needs to be done even if the areas are
 the same.
   

That's what safe_buffer_increment does.

-- 
Rob Shearman





Re: kernel32: Remove unneeded casts

2007-12-20 Thread Robert Shearman
Andrew Talbot wrote:
/* All local heap allocations are aligned on 4-byte boundaries */
  #define LALIGN(word)  (((word) + 3)  ~3)
  
 -#define ARENA_PTR(ptr,arena)   ((LOCALARENA *)((char*)(ptr)+(arena)))
 +#define ARENA_PTR(ptr,arena)   ((LOCALARENA *)((ptr)+(arena)))

Don't remove casts from macros like this as they then won't be safe if 
the type that is passed in is changed to something else (like void *).

-- 
Rob Shearman





Re: ole32: Fix a failed call to DllGetClassObject while retrieving class object interface pointer.

2007-12-20 Thread Robert Shearman
Huang, Zhangrong wrote:
 See dlls/ole32/compobj.c,

 static HRESULT apartment_getclassobject(struct apartment *apt, LPCWSTR 
 dllpath,
 BOOL apartment_threaded,
 REFCLSID rclsid, REFIID riid,
 void **ppv)
 {
 ...
 if (SUCCEEDED(hr))
 {
 ..
 TRACE(calling DllGetClassObject %p\n,
 apartment_loaded_dll-dll-DllGetClassObject);
 /* OK: get the ClassObject */
 hr = apartment_loaded_dll-dll-DllGetClassObject(rclsid, riid, ppv);
   
 if (hr != S_OK)
 ERR(DllGetClassObject returned error 0x%08x\n, hr);
 }
 ..
 }

 Reference to http://msdn2.microsoft.com/en-us/library/ms680760.aspx,

 riid

 [in] Reference to the identifier of the interface that the caller
 is to use to communicate with the class object. Usually, this is
 IID_IClassFactory (defined in the OLE headers as the interface
 identifier for IClassFactory).
   

This doesn't mean anything other than CoCreateInstance is usually used 
instead of CoGetClassObject (and CoCreateInstance always passes in 
IID_IClassFactory for the IID).

 We can't pass riid to DllGetClassObject directly sometimes, causes some dlls'
 DllGetClassObject (like native adsldp.dll) can only return reference-counted
 pointer to class factory via IID_IClassFactory, for retrieving the
 pointer to the
 class object interface requested in riid, we must try another way.
   

Thank you for your patch, but this behaviour seems a little strange and 
needs a little more investigation before I am happy that it is correct. 
What parameters are being passed in to CoGetClassObject to cause the 
call to fail for you?

-- 
Rob Shearman





Re: [PATCH] Implement BindIoCompletionCallback

2007-12-19 Thread Robert Shearman
Andrey Turkin wrote:
 Robert Shearman wrote:
   
 Andrey Turkin wrote:
 
  
 /**

   *BindIoCompletionCallback (KERNEL32.@)
   */
 +extern NTSTATUS WINAPI
 RtlSetIoCompletionCallback(HANDLE,LPOVERLAPPED_COMPLETION_ROUTINE,ULONG);

   
   
 This should go in winternl.h.
 
 winternl.h may not depend on winbase.h for some reason, so this would
 mean either changing prototype to (HANDLE,LPVOID,ULONG), which seems
 wrong, or redefining LPOVERLAPPED_COMPLETION_ROUTINE and
 OVERLAPPED/LPOVERLAPPED, which seems to be too much duplication.
   

I think you should guard the function with an #ifdef __WINE_WINBASE_H 
statement, but I would wait for some other developers to chime in to get 
a consensus before accepting this though.

-- 
Rob Shearman





Re: gdi32: Add a GdiConvertToDevmodeW test, make it pass under Wine

2007-12-18 Thread Robert Shearman
Dmitry Timoshkov wrote:
  DEVMODEW *dmW;
  WORD dmW_size;
  
 -dmW_size = dmA-dmSize + CCHDEVICENAME;
 -if (dmA-dmSize = (const char *)dmA-dmFormName - (const char *)dmA + 
 CCHFORMNAME)
 +dmW_size = dmA-dmSize;
 +if (dmW_size  sizeof(DEVMODEA))
 +dmW_size = sizeof(DEVMODEA);
   

Shouldn't this be sizeof(DEVMODEW)?

 +
 +dmW_size += CCHDEVICENAME;
   

Shouldn't this be CCDEVICENAME * sizeof(WCHAR)?

 +if (dmA-dmSize = FIELD_OFFSET(DEVMODEA, dmFormName) + CCHFORMNAME)
  dmW_size += CCHFORMNAME; 
   

Shouldn't this be CCHFORMNAME * sizeof(WCHAR)?

-- 
Rob Shearman





Re: Today's git won't hunt. Server crash, lots of other problems?

2007-12-18 Thread Robert Shearman
Dan Kegel wrote:
 On Dec 18, 2007 10:29 AM, Susan Cragin [EMAIL PROTECTED] wrote:
   
 My git won't install DNS9P any more, as of today's git.
 Any suggestions?

 [EMAIL PROTECTED]:~/DNS9$ wine setup
 fixme:advapi:LookupAccountNameW (null) Lsusan (nil) 0x34b944 (nil) 
 0x34b948 0x34b93c - stub
 fixme:advapi:LookupAccountNameW (null) Lsusan 0x1463f0 0x34b944 0x146998 
 0x34b948 0x34b93c - stub
 fixme:msi:ITERATE_DuplicateFiles We should track these duplicate files as 
 well
 fixme:msi:msi_unimplemented_action_stub RemoveFolders - 2 ignored 
 LCreateFolder table values
 fixme:advapi:LookupAccountNameW (null) Lsusan (nil) 0x34f7fc (nil) 
 0x34f800 0x34f7f4 - stub
 fixme:advapi:LookupAccountNameW (null) Lsusan 0x1423a0 0x34f7fc 0x13ce80 
 0x34f800 0x34f7f4 - stub
 fixme:atl:AtlModuleInit SEMI-STUB (0x40eb40 0x40c040 0x40)
 wineserver crashed, please enable coredumps (ulimit -c unlimited) and 
 restart.
 

 I just ran into that myself.ALVIN!

 But it doesn't seem easy to repeat; I keep getting other errors now.

 Roy next to me says he ran into another regression, he's doing a bisect,
 maybe it's the same thing.  He should be done shortly.

I don't think DNS9 uses the console API, and the only other recent 
changes in the server are to do with I/O completion ports. If you can't 
reproduce it very easily, then it might be worth just trying to confirm 
that it does use I/O completion ports.

-- 
Rob Shearman





Re: [PATCH] Implement BindIoCompletionCallback

2007-12-18 Thread Robert Shearman
Andrey Turkin wrote:
  
 /**
   *   BindIoCompletionCallback (KERNEL32.@)
   */
 +extern NTSTATUS WINAPI 
 RtlSetIoCompletionCallback(HANDLE,LPOVERLAPPED_COMPLETION_ROUTINE,ULONG);
   

This should go in winternl.h.

  BOOL WINAPI BindIoCompletionCallback( HANDLE FileHandle, 
 LPOVERLAPPED_COMPLETION_ROUTINE Function, ULONG Flags)
  {
 -FIXME(%p, %p, %d, stub!\n, FileHandle, Function, Flags);
 -SetLastError(ERROR_CALL_NOT_IMPLEMENTED);
 +NTSTATUS status;
 +
 +TRACE((%p, %p, %d)\n, FileHandle, Function, Flags);
 +
 +status = RtlSetIoCompletionCallback( FileHandle, Function, Flags );
 +if (status == STATUS_SUCCESS) return TRUE;
 +SetLastError( RtlNtStatusToDosError(status) );
  return FALSE;
  } 
   


-- 
Rob Shearman





Re: New bugzilla components.

2007-12-16 Thread Robert Shearman
Dmitry Timoshkov wrote:
 Rename wine-ole - ole32

 Add oleaut32, rpcrt4
   

Currently, all of the above are filed in wine-ole and I don't have a 
problem with that. Since a common question by our bugzilla advocates is 
does installing DCOM fix the bug? then this makes sense.

Care needs to be taken with creating other categories too so that any 
new categories you create are useful to the developers that work on that 
area and to the people who triage bugs. (For example, consult Jacek 
before creating separate mshtml, shdocvw and urlmon components.)

-- 
Rob Shearman





Re: Clear padding in the buffer due to alignment.

2007-12-16 Thread Robert Shearman
Dmitry Timoshkov wrote:
 Robert Shearman [EMAIL PROTECTED] wrote:

 This isn't done in MIDL, but I think it's desirable both for 
 silencing Valgrind warnings and for security purposes (fixes 
 potential for leaking information to remote computers or other 
 processes).

 +print_file(file, indent, memset(_StubMsg.Buffer, 0, 
 ((long)_StubMsg.Buffer)  0x%x);\n, alignment - 1);

 Is it possible to make it 64-bit and Wine/Windows compatible by using
 ULONG_PTR instead of long?

At the moment, widl doesn't support generating 64-bit compatible client, 
server and proxy code so doing so would be useless. FWIW, MIDL outputs 
__int64 here instead of long when targeting 64-bit.

Getting support for the Microsoft 64-bit calling convention into 
upstream gcc is the most important requirement for a 64-bit Wine.

-- 
Rob Shearman





Re: Clear padding in the buffer due to alignment.

2007-12-16 Thread Robert Shearman
Dmitry Timoshkov wrote:
 Robert Shearman [EMAIL PROTECTED] wrote:

 Is it possible to make it 64-bit and Wine/Windows compatible by using
 ULONG_PTR instead of long?

 At the moment, widl doesn't support generating 64-bit compatible 
 client, server and proxy code so doing so would be useless. FWIW, 
 MIDL outputs __int64 here instead of long when targeting 64-bit.

 If it doesn't take too much effort to write a 64-bit clean code, why 
 avoid
 that, especially all that's required is using an appropriate cast?

1. Both of these lines are correct using Unix compilers since long is 
64-bit when pointers are 64-bit:
print_file(file, indent, memset(_StubMsg.Buffer, 0, 
((long)_StubMsg.Buffer)  0x%x);\n, alignment - 1);
print_file(file, indent, _StubMsg.Buffer = (unsigned char 
*)(((long)_StubMsg.Buffer + %u)  ~0x%x);\n,
alignment - 1, alignment - 1);

2. It's independent of my patch (even if long is 32-bits, we're not 
going to have alignments above 4Gb).
3. Supporting 64-bit pointers in widl is a lot more complicated than 
just casting using the correct type (it involves making anything with 
embedded pointers into a complex type).

-- 
Rob Shearman





Re: Tracking down missing controls (drawn)

2007-12-14 Thread Robert Shearman
Alistair Leslie-Hughes wrote:
 Hi,
   I've manage to get MS Money 2008 loading there sample file, (with some 
 help from Juan).
   

Nice work.

 The next issue is trying to get the information to display correctly.

 The first image is the one currently running in WINE, and the second is 
 within WinXP
 http://members.dodo.com.au/~lesliehughes/MS_Money2008.png
 http://members.dodo.com.au/~lesliehughes/Msmoney2008real.PNG

 As you can see the control at the bottom doesnt display as it should.

 Where can I start to look to workout how to get this control(s) to display 
 correctly?
   

It looks like a webpage to me, so mshtml is probably being used.

-- 
Rob Shearman





  1   2   3   4   5   6   7   8   9   10   >