Re: [3/3] ws2_32: Always return the source address from WSAAccept.
On Tue, 2013-10-01 at 09:32 -0300, Bruno Jesus wrote:
> On Tue, Oct 1, 2013 at 6:37 AM, Hans Leidekker wrote:
> > ---
> > dlls/ws2_32/socket.c | 9 +
> > dlls/ws2_32/tests/sock.c | 6 +++---
> > 2 files changed, 4 insertions(+), 11 deletions(-)
> >
> > diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c
> > index c34de4b..6855298 100644
> > --- a/dlls/ws2_32/socket.c
> > +++ b/dlls/ws2_32/socket.c
> > @@ -6546,12 +6546,8 @@ SOCKET WINAPI WSAAccept( SOCKET s, struct
> > WS_sockaddr *addr, LPINT addrlen,
> > TRACE("Socket %04lx, sockaddr %p, addrlen %p, fnCondition %p,
> > dwCallbackData %ld\n",
> > s, addr, addrlen, lpfnCondition, dwCallbackData);
> >
> > -
> > - size = sizeof(src_addr);
> > - cs = WS_accept(s, &src_addr, &size);
> > -
> > + cs = WS_accept(s, addr, addrlen);
>
> The parameter addr may be NULL in WSAAccept, I can't check the source
> code right now but what will WS_accept do in this case?
It will skip the WS_getpeername call. Good point, I'll add some tests for this
case
and resubmit the series.
Re: wininet: Don't assume that end of chunk means end of stream. (try 2)
On Thu, 2013-09-19 at 17:38 +0200, Jacek Caban wrote: > I was hoping for a test like the attached one, which shows that there is > still more work to do. But it's a step in the right direction, so I'm > fine with your patch. The issue here is that HTTP_ReceiveRequestData calls refill_read_buffer, which calls generic read_http_stream with a read size of 8192 (the read buffer is still empty). In non-chunked mode this should read the minimum of that size and the content length. In chunked mode we don't know the content length, and we should read the minimum of chunk size and read buffer size, as shown by your test. Maybe we should add a refill_read_buffer method to the backends and call that instead of read_http_stream?
Re: wininet: Don't assume that end of chunk means end of stream.
On Wed, 2013-09-11 at 17:53 +0200, Jacek Caban wrote: > > Network traces tell me that native also performs a read on the first call to > > InternetQueryDataAvailable. > > Are you sure it's not an asynchronous read from a separated thread that > is ordered by InternetQueryDataAvailable if no data is available? This was a synchronous request.
Re: wininet: Don't assume that end of chunk means end of stream.
On Wed, 2013-09-11 at 16:38 +0200, Jacek Caban wrote:
> Hi Hans,
>
> On 09/11/13 13:50, Hans Leidekker wrote:
> > static DWORD chunked_get_avail_data(data_stream_t *stream, http_request_t
> > *req)
> > {
> > -/* Allow reading only from read buffer */
> > +chunked_stream_t *chunked_stream = (chunked_stream_t*)stream;
> > +DWORD res;
> > +
> > +if(!chunked_stream->chunk_size || chunked_stream->chunk_size == ~0u) {
> > +res = start_next_chunk(chunked_stream, req);
> > +if(res != ERROR_SUCCESS)
> > +return 0;
> > +}
>
> start_next_chunk may block and we don't want chunked_get_avail_data to
> block.
We can't avoid that. If this is the first chunk or if the current chunk has
been consumed we need to read a couple of bytes to find out how much we can
expect.
Network traces tell me that native also performs a read on the first call to
InternetQueryDataAvailable.
Re: [2/2] rpcrt4: Clear the connection pool when closing an http connection.
On Wed, 2013-09-04 at 18:16 +0200, Jacek Caban wrote: > On 09/04/13 15:36, Hans Leidekker wrote: > > On Wed, 2013-09-04 at 15:28 +0200, Jacek Caban wrote: > >> On 09/04/13 15:01, Hans Leidekker wrote: > >>> + /* don't allow this connection to be reused */ > >>> + InternetSetOptionW(NULL, INTERNET_OPTION_SETTINGS_CHANGED, NULL, 0); > >> There must be a better way to do that than removing all existing wininet > >> cached connections to all hosts. Removing INTERNET_FLAG_KEEP_CONNECTION > >> from HttpOpenRequest call would be a good start... > > We can't do that, it breaks NTLM and Negotiate. > > Hmm, right. Thinking more about this, I don't see how this connection > could be reused. RPC over HTTP shouldn't reach the end of the stream, so > it can't be reused once the connection is established, even if client > closes the handle. If it is, that's probably a bug in wininet. When did > you see this to happen? It happens when Outlook connects to Exchange, closes the connection and then reconnects. I didn't trust wininet either and wrote a standalone test to mimic the consecutive connects. It showed that wininet is right in reusing the connection, though I didn't perform any reads or writes. You say that reuse depends on reaching end of stream, so this might actually be a variation of the other bug, which is that in chunked mode we can't know whether we reached end of stream.
Re: [2/2] rpcrt4: Clear the connection pool when closing an http connection.
On Wed, 2013-09-04 at 15:28 +0200, Jacek Caban wrote: > On 09/04/13 15:01, Hans Leidekker wrote: > > --- > > dlls/rpcrt4/rpc_transport.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/dlls/rpcrt4/rpc_transport.c b/dlls/rpcrt4/rpc_transport.c > > index d0857fa..1eb0151 100644 > > --- a/dlls/rpcrt4/rpc_transport.c > > +++ b/dlls/rpcrt4/rpc_transport.c > > @@ -3234,6 +3234,8 @@ static int rpcrt4_ncacn_http_close(RpcConnection > > *Connection) > >HeapFree(GetProcessHeap(), 0, httpc->servername); > >httpc->servername = NULL; > > > > + /* don't allow this connection to be reused */ > > + InternetSetOptionW(NULL, INTERNET_OPTION_SETTINGS_CHANGED, NULL, 0); > > There must be a better way to do that than removing all existing wininet > cached connections to all hosts. Removing INTERNET_FLAG_KEEP_CONNECTION > from HttpOpenRequest call would be a good start... We can't do that, it breaks NTLM and Negotiate.
Re: wininet: Don't start the next chunk if the read is satisfied.
On Wed, 2013-09-04 at 13:11 +0200, Jacek Caban wrote:
> On 09/04/13 09:22, Hans Leidekker wrote:
> > ---
> > dlls/wininet/http.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/dlls/wininet/http.c b/dlls/wininet/http.c
> > index 1832b4f..521e4aa 100644
> > --- a/dlls/wininet/http.c
> > +++ b/dlls/wininet/http.c
> > @@ -2795,7 +2795,7 @@ static DWORD chunked_read(data_stream_t *stream,
> > http_request_t *req, BYTE *buf,
> > chunked_stream_t *chunked_stream = (chunked_stream_t*)stream;
> > DWORD read_bytes = 0, ret_read = 0, res = ERROR_SUCCESS;
> >
> > -if(chunked_stream->chunk_size == ~0u) {
> > +if(!chunked_stream->chunk_size || chunked_stream->chunk_size == ~0u) {
> > res = start_next_chunk(chunked_stream, req);
> > if(res != ERROR_SUCCESS)
> > return res;
> > @@ -2835,7 +2835,7 @@ static DWORD chunked_read(data_stream_t *stream,
> > http_request_t *req, BYTE *buf,
> > chunked_stream->chunk_size -= read_bytes;
> > size -= read_bytes;
> > ret_read += read_bytes;
> > -if(!chunked_stream->chunk_size) {
> > +if(size && !chunked_stream->chunk_size) {
> > assert(read_mode != READMODE_NOBLOCK);
> > res = start_next_chunk(chunked_stream, req);
> > if(res != ERROR_SUCCESS)
>
> This patch breaks two assumptions:
> - chunk_size outside chunked_read means the end of the stream
That's strange, considering that there could always be another chunk. We can't
know if we're at the end of the stream in chunked mode, so I wonder if we should
always try the next chunk (and block) if we're being asked to read more than the
current chunk gives us.
The bug I'm seeing is when the current chunk is exactly the size of the read
request (or has that many bytes left). In that case the current code still tries
to start the next chunk and potentially blocks.
> - READMODE_SYNC should read as much data as possible
But no more than asked for? I don't see how my patch breaks this assumption.
Re: [PATCH] iphlpapi: Add AllocateAndGetTcpExTableFromStack().
On Wed, 2013-08-28 at 14:49 +0200, Ralf Habacker wrote: > >> diff --git a/dlls/iphlpapi/ipstats.h b/dlls/iphlpapi/ipstats.h > >> index bf5bb92..efdb1cc 100644 > >> --- a/dlls/iphlpapi/ipstats.h > >> +++ b/dlls/iphlpapi/ipstats.h > >> @@ -33,6 +33,7 @@ > >> DWORD getInterfaceStatsByName(const char *name, PMIB_IFROW entry) > >> DECLSPEC_HIDDEN; > >> > >> DWORD WINAPI AllocateAndGetUdpTableFromStack(PMIB_UDPTABLE *ppUdpTable, > >> BOOL bOrder, HANDLE heap, DWORD flags) DECLSPEC_HIDDEN; > >> +DWORD WINAPI AllocateAndGetTcpExTableFromStack(PVOID *ppTcpTable, BOOL > >> bOrder, HANDLE heap, DWORD flags, DWORD family) DECLSPEC_HIDDEN; > > Again, you don't need this part. > This is because it is used nowhere else yet ? Yes, it's not used and likely won't be because build_tcp_table can be used instead.
Re: [PATCH] iphlpapi: Add AllocateAndGetTcpExTableFromStack().
On Wed, 2013-08-28 at 12:21 +0200, Ralf Habacker wrote: > diff --git a/dlls/iphlpapi/iphlpapi.spec b/dlls/iphlpapi/iphlpapi.spec > index 36ba13f..1eed5ae 100644 > --- a/dlls/iphlpapi/iphlpapi.spec > +++ b/dlls/iphlpapi/iphlpapi.spec > @@ -6,6 +6,7 @@ > @ stdcall AllocateAndGetIpForwardTableFromStack( ptr long long long ) > @ stdcall AllocateAndGetIpNetTableFromStack( ptr long long long ) > @ stdcall AllocateAndGetTcpTableFromStack( ptr long long long ) > +@ stdcall AllocateAndGetTcpExTableFromStack( ptr long long long long ) AllocateAndGetTcpExTableFromStack sorts before AllocateAndGetTcpTableFromStack. > diff --git a/dlls/iphlpapi/ipstats.h b/dlls/iphlpapi/ipstats.h > index bf5bb92..efdb1cc 100644 > --- a/dlls/iphlpapi/ipstats.h > +++ b/dlls/iphlpapi/ipstats.h > @@ -33,6 +33,7 @@ > DWORD getInterfaceStatsByName(const char *name, PMIB_IFROW entry) > DECLSPEC_HIDDEN; > > DWORD WINAPI AllocateAndGetUdpTableFromStack(PMIB_UDPTABLE *ppUdpTable, BOOL > bOrder, HANDLE heap, DWORD flags) DECLSPEC_HIDDEN; > +DWORD WINAPI AllocateAndGetTcpExTableFromStack(PVOID *ppTcpTable, BOOL > bOrder, HANDLE heap, DWORD flags, DWORD family) DECLSPEC_HIDDEN; Again, you don't need this part.
Re: [PATCH] iphlpapi: Add AllocateAndGetTcpExTableFromStack().
On Wed, 2013-08-28 at 11:41 +0200, Ralf Habacker wrote:
> diff --git a/dlls/iphlpapi/iphlpapi.spec b/dlls/iphlpapi/iphlpapi.spec
> index 36ba13f..cad2ca9 100644
> --- a/dlls/iphlpapi/iphlpapi.spec
> +++ b/dlls/iphlpapi/iphlpapi.spec
> @@ -6,6 +6,7 @@
> @ stdcall AllocateAndGetIpForwardTableFromStack( ptr long long long )
> @ stdcall AllocateAndGetIpNetTableFromStack( ptr long long long )
> @ stdcall AllocateAndGetTcpTableFromStack( ptr long long long )
> +@ stdcall AllocateAndGetTcpExTableFromStack( ptr long long long )
These should be sorted.
> diff --git a/dlls/iphlpapi/ipstats.c b/dlls/iphlpapi/ipstats.c
> index 809dbc6..bcb3021 100644
> --- a/dlls/iphlpapi/ipstats.c
> +++ b/dlls/iphlpapi/ipstats.c
> @@ -2327,6 +2327,32 @@ DWORD WINAPI AllocateAndGetTcpTableFromStack(
> PMIB_TCPTABLE *ppTcpTable, BOOL bO
> return build_tcp_table( TCP_TABLE_BASIC_ALL, (void **)ppTcpTable,
> bOrder, heap, flags, NULL );
> }
>
> +/**
> + *AllocateAndGetTcpExTableFromStack (IPHLPAPI.@)
> + *
> + * Get the TCP connection table.
> + * Like GetTcpTable(), but allocate the returned table from heap.
> + *
> + * PARAMS
> + * ppTcpTable [Out] pointer into which the MIB_TCPTABLE_EX is
> + * allocated and returned.
> + * bOrder [In] whether to sort the table
> + * heap [In] heap from which the table is allocated
> + * flags [In] flags to HeapAlloc
> + *
> + * RETURNS
> + * ERROR_INVALID_PARAMETER if ppTcpTable is NULL, whatever GetTcpTable()
> + * returns otherwise.
> + */
> +DWORD WINAPI AllocateAndGetTcpExTableFromStack( PMIB_TCPTABLE_EX
> *ppTcpTable, BOOL bOrder,
> +HANDLE heap, DWORD flags )
> +{
> +TRACE("table %p, bOrder %d, heap %p, flags 0x%08x\n", ppTcpTable,
> bOrder, heap, flags);
> +
> +if (!ppTcpTable) return ERROR_INVALID_PARAMETER;
> +return build_tcp_table( TCP_TABLE_OWNER_PID_ALL, (void **)ppTcpTable,
> bOrder, heap, flags, NULL );
> +}
> +
> static DWORD get_udp_table_sizes( UDP_TABLE_CLASS class, DWORD row_count,
> DWORD *row_size )
> {
> DWORD table_size;
> diff --git a/dlls/iphlpapi/ipstats.h b/dlls/iphlpapi/ipstats.h
> index bf5bb92..823fadb 100644
> --- a/dlls/iphlpapi/ipstats.h
> +++ b/dlls/iphlpapi/ipstats.h
> @@ -34,6 +34,7 @@ DWORD getInterfaceStatsByName(const char *name, PMIB_IFROW
> entry) DECLSPEC_HIDDE
>
> DWORD WINAPI AllocateAndGetUdpTableFromStack(PMIB_UDPTABLE *ppUdpTable, BOOL
> bOrder, HANDLE heap, DWORD flags) DECLSPEC_HIDDEN;
> DWORD WINAPI AllocateAndGetTcpTableFromStack(PMIB_TCPTABLE *ppTcpTable, BOOL
> bOrder, HANDLE heap, DWORD flags) DECLSPEC_HIDDEN;
> +DWORD WINAPI AllocateAndGetTcpExTableFromStack(PMIB_TCPTABLE_EX *ppTcpTable,
> BOOL bOrder, HANDLE heap, DWORD flags) DECLSPEC_HIDDEN;
> DWORD WINAPI AllocateAndGetIpNetTableFromStack(PMIB_IPNETTABLE
> *ppIpNetTable, BOOL bOrder, HANDLE heap, DWORD flags) DECLSPEC_HIDDEN;
> DWORD WINAPI AllocateAndGetIpForwardTableFromStack(PMIB_IPFORWARDTABLE
> *ppIpForwardTable, BOOL bOrder, HANDLE heap, DWORD flags) DECLSPEC_HIDDEN;
You don't need this, AllocateAndGetTcpExTableFromStack can be made static. The
first
parameter should be a void ** like in recent versions of iphlpapi.h.
> diff --git a/include/tcpmib.h b/include/tcpmib.h
> index 477500c..bc63c3f 100644
> --- a/include/tcpmib.h
> +++ b/include/tcpmib.h
> @@ -150,6 +150,9 @@ typedef struct _MIB_TCPTABLE2
> MIB_TCPROW2 table[1];
> } MIB_TCPTABLE2, *PMIB_TCPTABLE2;
>
> +typedef MIB_TCPROW_OWNER_PID _MIB_TCPROW_EX;
> +typedef MIB_TCPTABLE_OWNER_PID MIB_TCPTABLE_EX;
> +typedef PMIB_TCPTABLE_OWNER_PID PMIB_TCPTABLE_EX;
Recent headers don't define these so we shouldn't either.
Re: Is it a little bug in wbemprox?
On Sat, 2013-08-10 at 15:55 +0800, Jing Li wrote: > Hi, > When I ran a simple win32 program just via WMI with query of "SELECT * > from Win32_Directory", wine1.6 crashed. And when I checked the related code > I found that the function of "static UINT seed_dirs( struct dirstack > *dirstack, const struct expr *cond, WCHAR root, UINT *count )", > in dlls/wbemprox/builtin.c:1086, accessing cond's member without checking > if it's NULL! Unfortunately a NULL is passed to it. That's why my program > crashed. > It's a bug, right? Please open a bug report and attach a +wbemprox trace.
Re: [wbemprox] Patches for adding currentclockspeed in record_processor
On Tue, 2013-07-02 at 20:34 +0900, Rosen Diankov wrote:
> you are right again, i'll be more careful. now, i've addressed every
> problem except creating the patches with git. any help would be
> appreciated.
See http://wiki.winehq.org/GitWine
> --- wine-1.6-rc4-old/dlls/wbemprox/builtin.c2013-06-29 04:53:55.0
> +0900
> +++ wine-1.6-rc4/dlls/wbemprox/builtin.c2013-07-02 19:30:39.157916619
> +0900
> @@ -168,6 +168,8 @@
> {'M','a','n','u','f','a','c','t','u','r','e','r',0};
> static const WCHAR prop_maxclockspeedW[] =
> {'M','a','x','C','l','o','c','k','S','p','e','e','d',0};
> +static const WCHAR prop_currentclockspeedW[] =
> +{'C','u','r','r','e','n','t','C','l','o','c','k','S','p','e','e','d',0};
> static const WCHAR prop_memberW[] =
> {'M','e','m','b','e','r',0};
> static const WCHAR prop_methodW[] =
> @@ -381,6 +383,7 @@
> { prop_familyW, CIM_UINT16, VT_I4 },
> { prop_manufacturerW, CIM_STRING|COL_FLAG_DYNAMIC },
> { prop_maxclockspeedW,CIM_UINT32, VT_I4 },
> +{ prop_currentclockspeedW,CIM_UINT32, VT_I4 },
> { prop_nameW, CIM_STRING|COL_FLAG_DYNAMIC },
> { prop_numcoresW, CIM_UINT32, VT_I4 },
> { prop_numlogicalprocessorsW, CIM_UINT32, VT_I4 },
> @@ -633,6 +636,7 @@
> UINT16 family;
> const WCHAR *manufacturer;
> UINT32 maxclockspeed;
> +UINT32 currentclockspeed;
> const WCHAR *name;
> UINT32 num_cores;
> UINT32 num_logical_processors;
> @@ -1690,20 +1694,30 @@
> regs_to_str( regs, 16, name + 32 );
> }
> }
Please keep properties sorted alphabetically.
> -static UINT get_processor_maxclockspeed( void )
> +static void get_processor_clockspeeds( UINT* maxclockspeed, UINT*
> currentclockspeed )
> {
> PROCESSOR_POWER_INFORMATION *info;
> -UINT ret = 1000, size = get_processor_count() *
> sizeof(PROCESSOR_POWER_INFORMATION);
> +UINT size = get_processor_count() * sizeof(PROCESSOR_POWER_INFORMATION);
> NTSTATUS status;
> -
> +UINT valueswritten=0;
> if ((info = heap_alloc( size )))
> {
> status = NtPowerInformation( ProcessorInformation, NULL, 0, info,
> size );
> -if (!status) ret = info[0].MaxMhz;
> +if (!status)
> +{
> +*maxclockspeed = info[0].MaxMhz;
> +*currentclockspeed = info[0].CurrentMhz;
> +valueswritten = 1;
> +}
> heap_free( info );
> }
> -return ret;
> +if( valueswritten == 0 )
> +{
> +*maxclockspeed = 1000;
> +*currentclockspeed = 1000;
> +}
> }
You don't need an extra variable, just return early.
Re: [wbemprox] Patches for adding currentclockspeed in record_processor
On Mon, 2013-07-01 at 18:08 +0900, Rosen Diankov wrote: > you are right, i'm attaching a new patch. You didn't address all comments.
Re: [wbemprox] Patches for adding currentclockspeed in record_processor
On Mon, 2013-07-01 at 08:56 +0900, Rosen Diankov wrote:
> diff -ru wine-1.6-rc3-old/dlls/wbemprox/builtin.c
> wine-1.6-rc3/dlls/wbemprox/builtin.c
> --- wine-1.6-rc3-old/dlls/wbemprox/builtin.c2013-06-22 03:24:01.0
> +0900
> +++ wine-1.6-rc3/dlls/wbemprox/builtin.c2013-06-27 23:04:20.170154454
> +0900
Please use git to generate patches.
> @@ -168,6 +168,8 @@
> {'M','a','n','u','f','a','c','t','u','r','e','r',0};
> static const WCHAR prop_maxclockspeedW[] =
> {'M','a','x','C','l','o','c','k','S','p','e','e','d',0};
> +static const WCHAR prop_currentclockspeedW[] =
> +{'C','u','r','r','e','n','t','C','l','o','c','k','S','p','e','e','d',0};
Please keep properties sorted here and below.
> static const WCHAR prop_memberW[] =
> {'M','e','m','b','e','r',0};
> static const WCHAR prop_methodW[] =
> @@ -381,6 +383,7 @@
> { prop_familyW, CIM_UINT16, VT_I4 },
> { prop_manufacturerW, CIM_STRING|COL_FLAG_DYNAMIC },
> { prop_maxclockspeedW,CIM_UINT32, VT_I4 },
> +{ prop_currentclockspeedW,CIM_UINT32, VT_I4 },
> { prop_nameW, CIM_STRING|COL_FLAG_DYNAMIC },
> { prop_numcoresW, CIM_UINT32, VT_I4 },
> { prop_numlogicalprocessorsW, CIM_UINT32, VT_I4 },
> @@ -633,6 +636,7 @@
> UINT16 family;
> const WCHAR *manufacturer;
> UINT32 maxclockspeed;
> +UINT32 currentclockspeed;
> const WCHAR *name;
> UINT32 num_cores;
> UINT32 num_logical_processors;
> @@ -1690,7 +1694,33 @@
> regs_to_str( regs, 16, name + 32 );
> }
> }
> -static UINT get_processor_maxclockspeed( void )
> +static void get_processor_clockspeeds( UINT* maxclockspeed, UINT*
> currentclockspeed )
> +{
> +PROCESSOR_POWER_INFORMATION *info;
> +UINT size = get_processor_count() * sizeof(PROCESSOR_POWER_INFORMATION);
> +NTSTATUS status;
> +
> +if ((info = heap_alloc( size )))
> +{
> +status = NtPowerInformation( ProcessorInformation, NULL, 0, info,
> size );
> +if (!status)
> +{
> +*maxclockspeed = info[0].MaxMhz;
> +*currentclockspeed = info[0].CurrentMhz;
> +}
> +heap_free( info );
> +}
> +if( !!maxclockspeed )
> +{
> +*maxclockspeed = 1000;
> +}
> +if( !!currentclockspeed )
> +{
> +*currentclockspeed = 1000;
> +}
> +}
You probably didn't mean to query the clock speeds and then overwrite them with
static values.
NULL checks are not needed in helpers like this.
> +/*static UINT get_processor_maxclockspeed( void )
> {
> PROCESSOR_POWER_INFORMATION *info;
> UINT ret = 1000, size = get_processor_count() *
> sizeof(PROCESSOR_POWER_INFORMATION);
> @@ -1703,7 +1733,8 @@
> heap_free( info );
> }
> return ret;
> -}
> +}*/
Don't comment out code. Remove it.
> static const WCHAR *get_osarchitecture(void)
> {
> SYSTEM_INFO info;
> @@ -1717,7 +1748,7 @@
> static const WCHAR fmtW[] = {'C','P','U','%','u',0};
> WCHAR device_id[14], processor_id[17], manufacturer[13], name[49] = {0};
> struct record_processor *rec;
> -UINT i, offset = 0, maxclockspeed, num_cores, num_logical_processors,
> count = get_processor_count();
> +UINT i, offset = 0, maxclockspeed = 0, currentclockspeed = 0, num_cores,
> num_logical_processors, count = get_processor_count();
> enum fill_status status = FILL_STATUS_UNFILTERED;
>
> if (!resize_table( table, count, sizeof(*rec) )) return
> FILL_STATUS_FAILED;
> @@ -1726,7 +1757,7 @@
> get_processor_manufacturer( manufacturer );
> get_processor_name( name );
>
> -maxclockspeed = get_processor_maxclockspeed();
> +get_processor_clockspeeds(&maxclockspeed, ¤tclockspeed);
> num_logical_processors = get_logical_processor_count( &num_cores ) /
> count;
> num_cores /= count;
You're not using currentclockspeed.
Re: [PATCH]winhttp:session.c fix a bug(return value is null)
On Mon, 2013-06-24 at 19:57 +0900, 中川祥 wrote:
> diff --git a/dlls/winhttp/session.c b/dlls/winhttp/session.c
> index 660eae5..dfe23d4 100644
> --- a/dlls/winhttp/session.c
> +++ b/dlls/winhttp/session.c
> @@ -1316,6 +1316,8 @@ BOOL WINAPI WinHttpDetectAutoProxyConfigUrl( DWORD
> flags, LPWSTR *url )
> {
> static int fixme_shown;
> if (!fixme_shown++) FIXME("discovery via DHCP not supported\n");
> +flags = WINHTTP_AUTO_DETECT_TYPE_DNS_A;
> +ret=TRUE;
> }
> if (flags & WINHTTP_AUTO_DETECT_TYPE_DNS_A)
> {
We shouldn't perform DNS detection if the app didn't ask for it. Do you have an
app
that fails because we return an error here?
Re: wbemprox: Implement Win32_OperatingSystem.LocalDateTime.
On Sun, 2013-06-23 at 18:55 +0200, Sylvain Petreolle wrote:
> +static WCHAR *get_localdatetime(void)
> +{
> +static const WCHAR fmtW[] =
> +
> {'%','0','4','u','%','0','2','u','%','0','2','u','%','0','2','u','%','0','2','u','%','0','2','u',
> + '.','%','0','6','u','+','0','0','0',0};
> +SYSTEM_TIMEOFDAY_INFORMATION ti;
> +TIME_FIELDS tf;
> +WCHAR *ret;
> +
> +if (!(ret = heap_alloc( 26 * sizeof(WCHAR) ))) return NULL;
> +
> +NtQuerySystemInformation( SystemTimeOfDayInformation, &ti, sizeof(ti),
> NULL );
> +RtlTimeToTimeFields( &ti.liKeBootTime, &tf );
> +sprintfW( ret, fmtW, tf.Year, tf.Month, tf.Day, tf.Hour, tf.Minute,
> tf.Second, tf.Milliseconds * 1000 );
> +return ret;
> +}
This will not return local time.
Re: wbemprox: Implement some properties of Win32_BIOS. (resend 4)
On Sat, 2013-06-22 at 12:17 +0900, Kim Jung Eon (김중언) wrote:
> static const struct column col_bios[] =
> {
> +{ prop_biosversionW, CIM_STRING|CIM_FLAG_ARRAY},
> { prop_descriptionW, CIM_STRING },
> { prop_manufacturerW, CIM_STRING },
> { prop_releasedateW, CIM_DATETIME },
> @@ -508,6 +511,7 @@ struct record_baseboard
> };
> struct record_bios
> {
> +const struct array *biosversion;
> const WCHAR *description;
> const WCHAR *manufacturer;
> const WCHAR *releasedate;
> @@ -695,9 +699,13 @@ static const struct record_baseboard data_baseboard[] =
> {
> { baseboard_manufacturerW, baseboard_serialnumberW, baseboard_tagW }
> };
> +static const struct array data_bios_biosversion =
> +{
> +1, (void*)bios_versionW
> +};
This can't work. BIOSVersion is of type CIM_STRING|CIM_FLAG_ARRAY, which
translates
to a pointer to an array of string pointers.
Re: [PATCH] winhttp: initialize schemes (Coverity) (resubmit)
On Tue, 2013-06-04 at 10:16 +0200, Marcus Meissner wrote:
> 1020943 Uninitialized scalar variable
>
> After suggestion by Hans, do it in the helper ... especially
> as the other external callers do not initialize schemes either.
I also said that you shouldn't touch the parameter on error :) The other
callers pass a pointer to a user variable and the attached test confirms
that native doesn't touch it.
diff --git a/dlls/winhttp/tests/winhttp.c b/dlls/winhttp/tests/winhttp.c
index 03de012..76d84fd 100644
--- a/dlls/winhttp/tests/winhttp.c
+++ b/dlls/winhttp/tests/winhttp.c
@@ -1859,7 +1859,7 @@ static void test_basic_request(int port, const WCHAR *verb, const WCHAR *path)
{
HINTERNET ses, con, req;
char buffer[0x100];
-DWORD count, status, size;
+DWORD count, status, size, supported, first, target;
BOOL ret;
ses = WinHttpOpen(test_useragent, 0, NULL, NULL, 0);
@@ -1882,6 +1882,13 @@ static void test_basic_request(int port, const WCHAR *verb, const WCHAR *path)
ok(ret, "failed to query status code %u\n", GetLastError());
ok(status == 200, "request failed unexpectedly %u\n", status);
+supported = first = target = 0x;
+ret = WinHttpQueryAuthSchemes(req, &supported, &first, &target);
+ok(!ret, "unexpected success\n");
+ok(supported == 0x, "got %x\n", supported);
+ok(first == 0x, "got %x\n", first);
+ok(target == 0x, "got %x\n", target);
+
count = 0;
memset(buffer, 0, sizeof(buffer));
ret = WinHttpReadData(req, buffer, sizeof buffer, &count);
@@ -1900,7 +1907,7 @@ static void test_basic_authentication(int port)
static const WCHAR userW[] = {'u','s','e','r',0};
static const WCHAR passW[] = {'p','w','d',0};
HINTERNET ses, con, req;
-DWORD status, size, error;
+DWORD status, size, error, supported, first, target;
BOOL ret;
ses = WinHttpOpen(test_useragent, 0, NULL, NULL, 0);
@@ -1912,6 +1919,13 @@ static void test_basic_authentication(int port)
req = WinHttpOpenRequest(con, NULL, authW, NULL, NULL, NULL, 0);
ok(req != NULL, "failed to open a request %u\n", GetLastError());
+supported = first = target = 0x;
+ret = WinHttpQueryAuthSchemes(req, &supported, &first, &target);
+ok(!ret, "unexpected success\n");
+ok(supported == 0x, "got %x\n", supported);
+ok(first == 0x, "got %x\n", first);
+ok(target == 0x, "got %x\n", target);
+
ret = WinHttpSendRequest(req, NULL, 0, NULL, 0, 0, 0);
ok(ret, "failed to send request %u\n", GetLastError());
Re: [Lcms-user] mscms: Use lcms2, when available [try 2]
On Tue, 2013-06-04 at 01:36 +0200, Marti Maria wrote: > > (specifically cmsReadRawTag and cmsWriteRawTag don't do what we want), > > Ops, so I did something wrong. These functions were in the API to > implement mscms > and JDK APIs. Could you please elaborate what the issue is? They don't allow reading or writing to an offset in the tag data. We could of course work around that by buffering all of the data, but in this case I think it's cleaner to stick with our own implementation, ported to the lcms2 header. Thank you for your efforts to support Wine better, lcms2 is a real improvement.
Re: mscms: Use lcms2, when available [try 2]
On Tue, 2013-06-04 at 00:48 +0200, Detlef Riekenberg wrote: > Hans, you found a way to change Wine to use getter/setter functions > while still linking to lcms v1 and than switch to lcms2? No, I'm not interested in doing that.
Re: [PATCH] mscms: Use lcms2, when available [try 2]
On Sun, 2013-06-02 at 22:49 +0200, Detlef Riekenberg wrote: > try2: > import the unchanged icc34.h from the lcms (v1) dev. package. > (Do we need to add a Wine LGPL licence header for icc34.h?) > > The plan is to remove this include, when changing Wine > to use the new getter/setter functions of lcms2 > (After the release of Wine-1.6) My plan was to switch to lcms2 soon *after* the release of Wine 1.6. I studied the API some more and my conclusion is that while it doesn't have all we need (specifically cmsReadRawTag and cmsWriteRawTag don't do what we want), we can avoid importing the icc34.h header. I'm not in favor of adding temporary workarounds. If distributions want to drop support for lcms1 in 1.6 I'd suggest that they backport the patches.
Re: [1/2] explorer: Initial implementation of desktop launchers.
On Fri, 2013-05-31 at 03:30 +0800, Qian Hong wrote:
> On Tue, May 28, 2013 at 6:42 PM, Hans Leidekker wrote:
> > +hr = SHGetKnownFolderPath( &FOLDERID_Desktop, 0, NULL, &desktop_folder
> > );
> > +if (FAILED( hr ))
> > +{
> > +WINE_ERR("Could not get user desktop folder\n");
> > +return;
> > +}
> > +hr = SHGetKnownFolderPath( &FOLDERID_PublicDesktop, 0, NULL,
> > &desktop_folder_public );
> > +if (FAILED( hr ))
> > +{
> > +WINE_ERR("Could not get public desktop folder\n");
> > +CoTaskMemFree( desktop_folder );
> > +return;
> > +}
>
> Does KF_FLAG_CREATE look better here? Other wise every time a new
Yes, that makes sense.
Re: [PATCH] winhttp: initialize schemes (Coverity) (resubmit)
On Tue, 2013-05-28 at 22:46 +0200, Marcus Meissner wrote: > 1020943 Uninitialized scalar variable > > schemes is |= in the called function, but not > initialized before. > > Was submitted 2 weeks ago already, I think it is good. It's good but query_auth_schemes is called from more places, so it would be better to fix it in that helper, without touching the parameter on error.
Re: [PATCH 3/3] iphlpapi: Implement CancelIPChangeNotify.
On Fri, 2013-05-10 at 20:07 -0600, Erich E. Hoover wrote:
> +/***
> + * IPHLPAPI_createmonitorhandle (INTERNAL)
> + *
> + * Routine to create the interface monitoring handle for NotifyAddrChange
> requests.
> + */
> +static NTSTATUS IPHLPAPI_createmonitorhandle(HANDLE *h)
> +{
> +NTSTATUS status = ERROR_NOT_SUPPORTED;
> +
> +*h = INVALID_HANDLE_VALUE;
> +#if defined(NETLINK_ROUTE)
I don't see why you're still using these defines. It's unlikely that this code
will work if NETLINK_ROUTE is defined but RTMGRP_IPV4_IFADDR isn't, for example.
If there are versions of the netlink headers that don't have all of them it
may be possible to work around that (e.g. by defining them ourselves), if not
we can expand the configure check.
Re: [PATCH 3/3] iphlpapi: Implement CancelIPChangeNotify.
On Fri, 2013-05-10 at 08:00 -0600, Erich E. Hoover wrote: > On Fri, May 10, 2013 at 3:48 AM, Hans Leidekker wrote: > > > > On Wed, 2013-05-08 at 15:34 -0600, Erich E. Hoover wrote: > > > ... > > > +#ifdef NLMSG_OK > > > +# define WINE_LINKMON_FAMILY PF_NETLINK > > > +# define WINE_LINKMON_TYPE SOCK_RAW > > > +# define WINE_LINKMON_PROTO NETLINK_ROUTE > > > +# define WINE_LINKMON_ADDRCHANGE(b) (NLMSG_OK((struct nlmsghdr*)b, len) > > > \ > > > + && (((struct nlmsghdr*)b)->nlmsg_type == RTM_NEWADDR \ > > > + || ((struct nlmsghdr*)b)->nlmsg_type == RTM_DELADDR)) > > > +#endif > > > > I don't see the need for these defines. WINE_LINKMON_ADDRCHANGE could be > > turned > > into a function. > > > > For the reason you noted below, on Mac it would make sense to set > these defines to be PF_SYSTEM, SOCK_RAW, and SYSPROTO_EVENT > (respectively). I wasn't sure whether the address change would be > better as a define or a function so I made it a define to fit with the > others, but I'd be happy to change it if you think it'd be better as a > function. It also seems that on Mac one uses an ioctl() request > instead of bind() to finish setting up the socket, but I figured that > having that as another define (or function) would be a little bit much > for this patch. I think it would be nicer to push the platform specifics to helper functions (e.g. setting up the socket or checking for an address change) and use the usual header defines (HAVE_LINUX_NETLINK_H and/or HAVE_SYS_KERN_EVENT_H) for anything that needs to be compiled conditionally.
Re: [PATCH 3/3] iphlpapi: Implement CancelIPChangeNotify.
On Wed, 2013-05-08 at 15:34 -0600, Erich E. Hoover wrote:
> +#ifdef HAVE_LINUX_NETLINK_H
> +# include
> +#endif
> +#ifdef HAVE_LINUX_RTNETLINK_H
> +# include
> +#endif
> +
> +#ifdef NLMSG_OK
> +# define WINE_LINKMON_FAMILY PF_NETLINK
> +# define WINE_LINKMON_TYPE SOCK_RAW
> +# define WINE_LINKMON_PROTO NETLINK_ROUTE
> +# define WINE_LINKMON_ADDRCHANGE(b) (NLMSG_OK((struct nlmsghdr*)b, len) \
> + && (((struct nlmsghdr*)b)->nlmsg_type == RTM_NEWADDR \
> + || ((struct nlmsghdr*)b)->nlmsg_type == RTM_DELADDR))
> +#endif
I don't see the need for these defines. WINE_LINKMON_ADDRCHANGE could be turned
into a function.
> @@ -2043,12 +2100,79 @@ DWORD WINAPI IpRenewAddress(PIP_ADAPTER_INDEX_MAP
> AdapterInfo)
> * FIXME
> * Stub, returns ERROR_NOT_SUPPORTED.
> */
> -DWORD WINAPI NotifyAddrChange(PHANDLE Handle, LPOVERLAPPED overlapped)
> +DWORD WINAPI NotifyAddrChange(PHANDLE handle, LPOVERLAPPED overlapped)
> {
> - FIXME("(Handle %p, overlapped %p): stub\n", Handle, overlapped);
> - if (Handle) *Handle = INVALID_HANDLE_VALUE;
> - if (overlapped) ((IO_STATUS_BLOCK *) overlapped)->u.Status =
> STATUS_PENDING;
> - return ERROR_IO_PENDING;
> +#ifdef WINE_LINKMON_FAMILY
> +IO_STATUS_BLOCK *iosb = (IO_STATUS_BLOCK *) overlapped;
> +struct sockaddr_nl addr;
> +NTSTATUS status;
> +int fd = -1;
> +HANDLE h;
> +
> +TRACE("(handle %p, overlapped %p): stub\n", handle, overlapped);
> +
> +h = INVALID_HANDLE_VALUE;
> +SERVER_START_REQ( create_socket )
Using a generic socket object might work I guess. The cost will be a new socket
per caller whereas in theory we could serve all callers with a single socket,
though that would require a dedicated server request. That might be needed
anyway,
if this handle somehow turns out to be special.
I know MacOS has a similar socket based mechanism that could probably be handled
in the same way.
Re: [PATCH] mscms: Use lcms2, when available
On Fri, 2013-05-10 at 02:05 +0200, Detlef Riekenberg wrote: > > For most apps it will be a matter of adding a configure check for lcms2 > > and then recompiling. This is not the case for Wine as you have pointed out. > - cmsCreateLabProfile was renamed to cmsCreateLab2Profile > cmsvirt.c (1.19: 570) / (2.4: 458 and 497) > > - cmsSetLogErrorHandler can be used as replacement for cmsSetErrorHandler > > > At this point there's probably still a substantial number of users on > > distributions that don't include lcms2, and trying to support both doesn't > > make the source any prettier. > > The changes to support both versions are resonable small. There's more. lcms1 carried the icc34.h header that defines the ICC format. It's included via lcms.h and we depend on it for functionality that the lcms1 API doesn't provide. I have some hope that the lcms2 API will be sufficient for this functionality, otherwise we may have to carry this header ourselves.
Re: [PATCH] mscms: Use lcms2, when available
On Thu, 2013-05-09 at 18:56 +0200, wine@web.de wrote: > Hi Hans. > > >> lcms2 was released on 8. May 2010 > >> lcms v1 is on the way to be phased out in the newest > >> distribution versions. > > >Are there any distributions that have released without v1? > > I don't know, but Ubuntu has a bug for replacing lcms v1. > > There is no reason, that Wine is the last app, which depends on lcms v1. For most apps it will be a matter of adding a configure check for lcms2 and then recompiling. This is not the case for Wine as you have pointed out. At this point there's probably still a substantial number of users on distributions that don't include lcms2, and trying to support both doesn't make the source any prettier. So it doesn't seem unreasonable to wait a little longer.
Re: [PATCH] mscms: Use lcms2, when available
On Tue, 2013-05-07 at 22:45 +0200, Detlef Riekenberg wrote: > lcms2 was released on 8. May 2010 and should be present in important > distributions since a while. > > lcms v1 is on the way to be phased out in the newest distribution > versions. Are there any distributions that have released without v1?
Re: [PATCH 3/3] iphlpapi: Implement CancelIPChangeNotify.
On Tue, 2013-05-07 at 10:04 -0600, Erich E. Hoover wrote: > On Tue, May 7, 2013 at 3:14 AM, Hans Leidekker wrote: > > ... > > TerminateThread is not the right tool here. If the thread is blocked on the > > recv call while being terminated it will leak the netlink socket. > > > > You'd need a way to synchronize the threads, but an overhead of one thread > > per > > caller is rather costly in the first place. This suggests that the server > > should > > poll the socket. > > So far the iphlpapi hasn't been doing anything directly with the > server... How about launching a single thread for NotifyAddrChange > (when needed) and keeping a list of OVERLAPPED structures to signal It's a system-wide event so the server seems to be a pretty good candidate to handle it, and it wouldn't need any extra threads. > when the list changes? It's easy enough to use MSG_DONTWAIT to make > recv non-blocking so that it's easy to cancel the thread. You'd be consuming cpu instead of properly waiting. I guess you could make this work if you poll an array with the netlink fd and another unix fd to signal the cancel event, but again, I think this is best handled by the server.
Re: [PATCH 3/3] iphlpapi: Implement CancelIPChangeNotify.
On Mon, 2013-05-06 at 19:23 -0600, Erich E. Hoover wrote:
> --- a/dlls/iphlpapi/iphlpapi_main.c
> +++ b/dlls/iphlpapi/iphlpapi_main.c
> @@ -210,8 +210,14 @@ DWORD WINAPI
> AllocateAndGetIpAddrTableFromStack(PMIB_IPADDRTABLE *ppIpAddrTable,
> */
> BOOL WINAPI CancelIPChangeNotify(LPOVERLAPPED overlapped)
> {
> - FIXME("(overlapped %p): stub\n", overlapped);
> - return FALSE;
> +HANDLE handle;
> +
> +TRACE("(overlapped %p)\n", overlapped);
> +
> +if (!overlapped)
> +return FALSE;
> +handle = (HANDLE) overlapped->u.Pointer;
> +return TerminateThread(handle, STATUS_CANCELLED);
> }
TerminateThread is not the right tool here. If the thread is blocked on the
recv call while being terminated it will leak the netlink socket.
You'd need a way to synchronize the threads, but an overhead of one thread per
caller is rather costly in the first place. This suggests that the server should
poll the socket.
Re: [PATCH 2/2] msi: Use next cabinet from the media table instead of failing when there is a mismatch with continuous cabinet.
On Wed, 2013-05-01 at 12:40 +0200, Christian Costa wrote:
> diff --git a/dlls/msi/media.c b/dlls/msi/media.c
> index 612624d..4145c76 100644
> --- a/dlls/msi/media.c
> +++ b/dlls/msi/media.c
> @@ -358,8 +358,18 @@ static INT_PTR cabinet_next_cabinet(FDINOTIFICATIONTYPE
> fdint,
>
> if (strcmpiW( mi->cabinet, cab ))
> {
> -ERR("Continuous cabinet does not match the next cabinet in the Media
> table\n");
> -goto done;
> +LPSTR next_cab;
> +
> +WARN("Continuous cabinet %s does not match the next cabinet %s in
> the media table => use latter one\n", debugstr_w(cab),
> debugstr_w(mi->cabinet));
> +
> +/* Use cabinet name from the media table */
> +next_cab = strdupWtoA(mi->cabinet);
> +/* Modify path to cabinet file with full filename (psz3 points to a
> 256 bytes buffer that can be modified contrary to psz1 and psz2) */
> +strcat(pfdin->psz3, "\\");
> +strcat(pfdin->psz3, next_cab);
> +/* Path psz3 and cabinet psz1 are concatenated by FDI so just reset
> psz1 */
> +*pfdin->psz1 = 0;
> +msi_free(next_cab);
The cabinet name alone can be up 256 characters so you should at least check
that you're
not overflowing the buffer.
Re: wbemprox: Implement some properties of Win32_ComputerSystem and Win32_DiskPartition.
On Mon, 2013-04-08 at 01:56 +0900, Kim Jung Eon (김중언) wrote:
> @@ -241,18 +243,21 @@ static const struct column col_compsys[] =
> { prop_modelW,CIM_STRING },
> { prop_numlogicalprocessorsW, CIM_UINT32, VT_I4 },
> { prop_numprocessorsW,CIM_UINT32, VT_I4 },
> -{ prop_totalphysicalmemoryW, CIM_UINT64 }
> +{ prop_totalphysicalmemoryW, CIM_UINT64 },
> +{ prop_nameW, CIM_STRING|COL_FLAG_DYNAMIC }
Please keep properties sorted.
Re: webservices: Add new dll
On Thu, 2013-02-21 at 18:47 +1100, Alistair Leslie-Hughes wrote: > +#include "wine/debug.h" > + > +WINE_DEFAULT_DEBUG_CHANNEL(wincodecs); That channel is already taken :)
Re: [1/4] windowscodecs: Implement IWICColorContext::InitializeFromFilename.
On Wed, 2013-02-06 at 18:10 +0100, Ludger Sprenker wrote: > On Wed, 06 Feb 2013 13:51:32 +0100, Hans Leidekker wrote: > > > + handle = CreateFileW(filename, GENERIC_READ, 0, NULL, > > > OPEN_EXISTING, 0, NULL); > > > + if (handle == INVALID_HANDLE_VALUE) return > > > HRESULT_FROM_WIN32(GetLastError()); > > I think, you are hiding a fixme for the default color profiles shipped with > windows. > > (like "c:\windows\system32\spool\drivers\color\sRGB Color Space Profile.icm") I don't think this is the right place for such a fixme, it could just as well be loading a profile that comes bundled with the app. Also note that any attempt to use the color context will cause a fixme to be printed.
Re: [4/4] windowscodecs: Implement IWICComponentFactory::CreateDecoderFromFileHandle.
On Wed, 2013-02-06 at 10:25 -0600, Vincent Povirk wrote: > > +hr = map_file(file, &map, &mem, &size); > > +if (FAILED(hr)) return hr; > > + > ... > > + > > +hr = IWICStreamImpl_InitializeFromMemory(stream, mem, size.u.LowPart); > > +if (FAILED(hr)) goto error; > > This makes me sad. > > Is this really simpler than implementing Write and Seek for a file handle? I'd say it's a comparable amount of code. I don't have a strong preference for either option.
Re: fusion: Allow null to be the value of the public key
On Thu, 2013-01-31 at 19:35 +1100, Alistair Leslie-Hughes wrote:
> diff --git a/dlls/fusion/asmname.c b/dlls/fusion/asmname.c
> index 162a76b..2c4b379 100644
> --- a/dlls/fusion/asmname.c
> +++ b/dlls/fusion/asmname.c
> @@ -561,6 +561,10 @@ static HRESULT parse_pubkey(IAssemblyNameImpl *name,
> LPCWSTR pubkey)
> {
> int i;
> BYTE val;
> +static WCHAR nullstr[] = {'n','u','l','l',0};
This should be const.
> +if(lstrcmpiW(pubkey, nullstr) == 0)
> +return S_OK;
Please add a test to show that it's case insensitive.
> if (lstrlenW(pubkey) < CHARS_PER_PUBKEY)
> return FUSION_E_INVALID_NAME;
> diff --git a/dlls/fusion/tests/asmcache.c b/dlls/fusion/tests/asmcache.c
> index e97cfbe..79f8709 100644
> --- a/dlls/fusion/tests/asmcache.c
> +++ b/dlls/fusion/tests/asmcache.c
> @@ -1016,6 +1016,10 @@ static void test_QueryAssemblyInfo(void)
> 'p','u','b','l','i','c','K','e','y','T','o','k','e','n','=',
> '2','d','0','3','6','1','7','b','1','c','3','1','e','2','f','5',',',
> 'c','u','l','t','u','r','e','=','n','e','u','t','r','a','l',0};
> +static const WCHAR nullpublickey[] = {
> +
> 'm','s','c','o','r','l','i','n',',','v','e','r','s','i','o','n','=','0',
> +
> 'p','u','b','l','i','c','K','e','y','T','o','k','e','n','=','n','u','l',
> +'c','u','l','t','u','r','e','=','n','e','u','t','r','a','l',0};
>
> size = MAX_PATH;
> hr = pGetCachePath(ASM_CACHE_GAC, asmpath, &size);
> @@ -1386,6 +1390,17 @@ static void test_QueryAssemblyInfo(void)
> "Assembly path was changed\n");
> ok(info.cchBuf == MAX_PATH, "Expected MAX_PATH, got %d\n", info.cchBuf);
>
> +/* display name is
> "mscorlib.dll,version=0.0.0.0,publicKeyToken=null,culture=neutral" */
This comment doesn't agree with the string initialized above.
> +INIT_ASM_INFO();
> +lstrcpyW(name, nullpublickey);
> +hr = IAssemblyCache_QueryAssemblyInfo(cache, 0, name, &info);
> +ok(info.cbAssemblyInfo == sizeof(ASSEMBLY_INFO),
> + "Expected sizeof(ASSEMBLY_INFO), got %d\n", info.cbAssemblyInfo);
You should check the return value of QueryAssemblyInfo but
CreateAssemblyNameObject
is actually a better way to test this.
Re: windowscodecs: Implement GetThumbnail in the ICO frame decoder.
On Wed, 2013-01-30 at 18:52 +0800, Dmitry Timoshkov wrote: > > The test added by 6395af1ae7b0cc5f2f1f82796502e2a605bc5a6b says otherwise, > > GetThumbnail > > is supported for ICO frames. > > It's still better to return WINCODEC_ERR_CODECNOTHUMBNAIL instead of > E_INVALIDARG. Real implementation depends on the codec set, and although > obviously MS implementation returns a real interface, that doesn't mean > that an implementation that returns WINCODEC_ERR_CODECNOTHUMBNAIL is wrong. Well, the app that prompted me to write that test wasn't fooled by returning WINCODEC_ERR_CODECNOTHUMBNAIL (which is what was returned before my patch). And since you're changing the FIXME to a TRACE you are effectively hiding the bug.
Re: windowscodecs: Implement GetThumbnail in the ICO frame decoder.
On Wed, 2013-01-30 at 18:02 +0800, Dmitry Timoshkov wrote:
> diff --git a/dlls/windowscodecs/icoformat.c b/dlls/windowscodecs/icoformat.c
> index 7aec245..da5a733 100644
> --- a/dlls/windowscodecs/icoformat.c
> +++ b/dlls/windowscodecs/icoformat.c
> @@ -200,8 +200,12 @@ static HRESULT WINAPI
> IcoFrameDecode_GetColorContexts(IWICBitmapFrameDecode *ifa
> static HRESULT WINAPI IcoFrameDecode_GetThumbnail(IWICBitmapFrameDecode
> *iface,
> IWICBitmapSource **ppIThumbnail)
> {
> -FIXME("(%p,%p)\n", iface, ppIThumbnail);
> -return E_NOTIMPL;
> +TRACE("(%p,%p)\n", iface, ppIThumbnail);
> +
> +if (!ppIThumbnail) return E_INVALIDARG;
> +
> +*ppIThumbnail = NULL;
> +return WINCODEC_ERR_CODECNOTHUMBNAIL;
> }
The test added by 6395af1ae7b0cc5f2f1f82796502e2a605bc5a6b says otherwise,
GetThumbnail
is supported for ICO frames.
Re: [PATCH 6/8] winhttp: Added schannel-based netconn_get_cipher_strength implementation
On Tue, 2013-01-22 at 16:49 +0100, Jacek Caban wrote: > @@ -1460,6 +1460,13 @@ int netconn_get_cipher_strength( netconn_t *conn ) > pSSL_CIPHER_get_bits( cipher, &bits ); > return bits; > #else > -return 0; > +SecPkgContext_ConnectionInfo conn_info; > +SECURITY_STATUS res; > + > +if (!conn->secure) return 0; > +res = QueryContextAttributesW(&conn->ssl_ctx, > SECPKG_ATTR_REMOTE_CERT_CONTEXT, (void*)&conn_info); That should be SECPKG_ATTR_CONNECTION_INFO.
Re: wininet: Map socket errors to ERROR_INTERNET_CANNOT_CONNECT in create_netconn_socket.
On Wed, 2012-12-19 at 12:25 +0100, Marvin wrote: > Hi, > > While running your changed tests on Windows, I think I found new failures. > Being a bot and all I'm not very good at pattern recognition, so I might be > wrong, but could you please double-check? > Full results can be found at > http://testbot.winehq.org/JobDetails.pl?Key=23542 > > Your paranoid android. > > > === W7PRO (32 bit http) === > http.c:3337: Test failed: HttpSendRequest failed: 12002 This is a timeout that occurs before the added test is run.
Re: wininet: Don't perform revocation checks when verifying a certificate.
On Tue, 2012-12-11 at 12:59 -0800, Juan Lang wrote: > Getting the client to trust the server cert can be as easy as ignoring > untrusted > root errors, if you don't think this impacts the revocation results. > > Returning revocation is straightforward enough, assuming you have a server > under > your control. So self-sign the CRL too. I guess that might work if ignoring untrusted root errors extends to verification of the CRL.
Re: wininet: Don't perform revocation checks when verifying a certificate.
On Tue, 2012-12-11 at 11:52 -0800, Juan Lang wrote: > On Tue, Dec 11, 2012 at 6:10 AM, Hans Leidekker wrote: > On Tue, 2012-12-11 at 14:52 +0100, Jacek Caban wrote: > > On 12/11/12 09:45, Hans Leidekker wrote: > > > https://testbot.winehq.org/JobDetails.pl?Key=23300 is a test > which shows that > > > revocation checks fail for the certificate on outlook.com when > passed straight > > > to CertVerifyRevocation. The reason is that a CRL link specified > in the > > > certificate does not resolve. > > > > > > https://testbot.winehq.org/JobDetails.pl?Key=23301 is a test > which makes > > > a secure connection to outlook.com from wininet and shows that > this succeeds. > > > > > > My conclusion is that native wininet doesn't perform revocation > checks. > > > > Your tests prove that we should relax our verification on > > CERT_TRUST_IS_OFFLINE_REVOCATION or something similar. To prove that > > revocation checks are not made, a test with truly revoked cert > would be > > needed. > > > True, though to perform the revocation check the CRL has to be > retrieved and my > tests with wireshark didn't show any signs of that. > > > Would adding to the tests as part of this patch be a bad thing? I thought about that but I am hesitant to use a random site that's not under our control. The alternative is to setup our own site with a certificate that only fails the revocation check, which I think means that we need to have it signed by a trusted root and then revoked. I'm not sure we have the means to do that currently. Do you have any suggestions?
Re: wininet: Don't perform revocation checks when verifying a certificate.
On Tue, 2012-12-11 at 14:52 +0100, Jacek Caban wrote: > On 12/11/12 09:45, Hans Leidekker wrote: > > https://testbot.winehq.org/JobDetails.pl?Key=23300 is a test which shows > > that > > revocation checks fail for the certificate on outlook.com when passed > > straight > > to CertVerifyRevocation. The reason is that a CRL link specified in the > > certificate does not resolve. > > > > https://testbot.winehq.org/JobDetails.pl?Key=23301 is a test which makes > > a secure connection to outlook.com from wininet and shows that this > > succeeds. > > > > My conclusion is that native wininet doesn't perform revocation checks. > > Your tests prove that we should relax our verification on > CERT_TRUST_IS_OFFLINE_REVOCATION or something similar. To prove that > revocation checks are not made, a test with truly revoked cert would be > needed. True, though to perform the revocation check the CRL has to be retrieved and my tests with wireshark didn't show any signs of that.
Re: msi: Fix logical expressions that always evaluate to FALSE
On Sat, 2012-12-08 at 21:59 +, Andrew Talbot wrote:
> diff --git a/dlls/msi/action.c b/dlls/msi/action.c
> index 076d1b3..677a43f 100644
> --- a/dlls/msi/action.c
> +++ b/dlls/msi/action.c
> @@ -6763,7 +6763,7 @@ static UINT ACTION_RemoveODBC( MSIPACKAGE *package )
> #define ENV_MOD_PREFIX 0x8000
> #define ENV_MOD_MASK0xC000
>
> -#define check_flag_combo(x, y) ((x) & ~(y)) == (y)
> +#define check_flag_combo(x, y) ((x) & (y)) == (y)
>
> static UINT env_parse_flags( LPCWSTR *name, LPCWSTR *value, DWORD *flags )
> {
It makes the tests fail here:
err:msi:env_parse_flags Invalid flags: 0007
err:msi:ITERATE_Actions Execution halted, action L"RemoveEnvironmentStrings"
returned 1627
err:msi:env_parse_flags Invalid flags: 0007
action.c:5899: Test failed: Expected ERROR_SUCCESS, got 1627
action.c:5905: Test failed: Expected ERROR_FILE_NOT_FOUND, got 0
action.c:5908: Test failed: Expected ERROR_FILE_NOT_FOUND, got 0
action.c:5920: Test failed: Expected ERROR_FILE_NOT_FOUND, got 0
I'd suggest to simply remove these checks.
Re: [1/2] schannel/tests: Fix test on win8
On Thu, 2012-12-06 at 12:56 +0100, André Hentschel wrote: > diff --git a/dlls/schannel/tests/main.c b/dlls/schannel/tests/main.c > index b576848..9c28e8b 100644 > --- a/dlls/schannel/tests/main.c > +++ b/dlls/schannel/tests/main.c > @@ -179,8 +179,8 @@ static void testGetInfo(void) > /* First package: Unified */ > status = pTables->GetInfo(&PackageInfo); > ok(status == STATUS_SUCCESS, "status: 0x%x\n", status); > -ok(PackageInfo.fCapabilities == 0x107b3, "fCapabilities: 0x%x\n", > - PackageInfo.fCapabilities); > +ok((PackageInfo.fCapabilities & ~SECPKG_FLAG_APPCONTAINER_PASSTHROUGH) > == 0x107b3, > + "fCapabilities: 0x%x\n", PackageInfo.fCapabilities); > ok(PackageInfo.wVersion == 1, "wVersion: %d\n", PackageInfo.wVersion); > ok(PackageInfo.wRPCID == 14, "wRPCID: %d\n", PackageInfo.wRPCID); > ok(PackageInfo.cbMaxToken == 0x4000 || A nicer fix would be to use a disjunction of or-ed symbolic constants, just like the negotiate test.
Re: msi: Fix a couple of leaks (coverity)
On Fri, 2012-11-16 at 11:50 +0100, Frédéric Delanoy wrote:
> diff --git a/dlls/msi/files.c b/dlls/msi/files.c
> index 11913ef..98d7513 100644
> --- a/dlls/msi/files.c
> +++ b/dlls/msi/files.c
> @@ -336,6 +336,7 @@ UINT ACTION_InstallFiles(MSIPACKAGE *package)
> if (rc != ERROR_SUCCESS)
> {
> ERR("Unable to load media info for %s (%u)\n",
> debugstr_w(file->File), rc);
> +msi_free(mi);
> return ERROR_FUNCTION_FAILED;
This is a loop, so you should instead call msi_free_media_info to also
free any strings allocated in a previous call to msi_load_media_info.
Re: advapi32: fix a too small buffer in CredUnmarshalCredentialW [try2]
On Thu, 2012-11-15 at 09:41 +0100, Stefan Leichter wrote: > i have to say sorry, this answer is rude. > > But i don't like to do unnecessary iteration on source code especially when > the "complain" has been in the previous version too. Source code usually does > not get better from iteration to iteration in this case. No offense taken. You second attempt certainly was an improvement, I just spotted more problems that I should have seen and mentioned in the first review.
Re: advapi32: fix a too small buffer in CredUnmarshalCredentialW [try2]
On Wed, 2012-11-14 at 16:28 +0100, Stefan Leichter wrote:
> @@ -2053,6 +2053,8 @@ static BOOL cred_decode( const WCHAR *cred, unsigned
> int len, char *buf )
> char c0, c1, c2, c3;
> const WCHAR *p = cred;
>
> +TRACE("%s\n", debugstr_wn(cred,len));
This string is already traced in CredUnmarshalCredentialW.
> @@ -2134,6 +2136,7 @@ BOOL WINAPI CredUnmarshalCredentialW( LPCWSTR cred,
> PCRED_MARSHAL_TYPE type, PVO
> case UsernameTargetCredential:
> {
> USERNAME_TARGET_CREDENTIAL_INFO *target;
> +ULONGLONG size = 0;
>
> if (len < 9 || !cred_decode( cred + 3, 6, (char *)&size ) || !size
> || size % sizeof(WCHAR))
> {
You should also perform a sanity check on 'size' to avoid overflow in
calculations that follow.
Re: fix a too small buffer in CredUnmarshalCredentialW
On Wed, 2012-11-14 at 09:48 +0100, Stefan Leichter wrote: > +char buffer[6]; > +unsigned int buflen, *size = (unsigned int*) buffer; > > -if (len < 9 || !cred_decode( cred + 3, 6, (char *)&size ) || !size > || size % sizeof(WCHAR)) > +if (len < 9 || !cred_decode( cred + 3, 6, buffer ) || !*size || > *size % sizeof(WCHAR)) You're still truncating the decoded size. You should instead make 'size' a ULONGLONG and initialize it to 0. Is this little-endian ARM?
Re: wine.inf: Set the FontSmoothingGamma value.
On Mon, 2012-11-12 at 21:16 +0100, Alexandre Julliard wrote: > > Fixes a crash in the .NET 4 runtime which queries this value via > > SystemParametersInfo(SPI_GETFONTSMOOTHINGCONTRAST). > > It shouldn't need the registry key to behave sanely. Does b5d96da32f > help? Yes.
Re: msi: Fix some leaks (coverity)
On Thu, 2012-11-08 at 01:32 +0100, Frédéric Delanoy wrote:
> diff --git a/dlls/msi/action.c b/dlls/msi/action.c
> index f9ab550..8a6c952 100644
> --- a/dlls/msi/action.c
> +++ b/dlls/msi/action.c
> @@ -5809,7 +5809,16 @@ static UINT ITERATE_InstallService(MSIRECORD *rec,
> LPVOID param)
> {
> int len = strlenW(file->TargetPath) + strlenW(args) + 2;
> if (!(image_path = msi_alloc(len * sizeof(WCHAR
> +{
> +msi_free(name);
> +msi_free(disp);
> +msi_free(load_order);
> +msi_free(serv_name);
> +msi_free(pass);
> +msi_free(depends);
> +msi_free(args);
> return ERROR_OUTOFMEMORY;
> +}
This function already has a 'done' label where memory is freed. Why not make the
return value a variable and jump to the label instead of repeating these
statements?
> strcpyW(image_path, file->TargetPath);
> strcatW(image_path, szSpace);
> @@ -6070,6 +6079,7 @@ static BOOL stop_service_dependents(SC_HANDLE scm,
> SC_HANDLE service)
> goto error;
> }
>
> +msi_free(dependencies);
> return TRUE;
Same here.
>
> error:
> diff --git a/dlls/msi/files.c b/dlls/msi/files.c
> index 11913ef..98d7513 100644
> --- a/dlls/msi/files.c
> +++ b/dlls/msi/files.c
> @@ -336,6 +336,7 @@ UINT ACTION_InstallFiles(MSIPACKAGE *package)
> if (rc != ERROR_SUCCESS)
> {
> ERR("Unable to load media info for %s (%u)\n",
> debugstr_w(file->File), rc);
> +msi_free(mi);
> return ERROR_FUNCTION_FAILED;
> }
> if (!file->Component->Enabled) continue;
> @@ -513,6 +514,7 @@ UINT ACTION_PatchFiles( MSIPACKAGE *package )
> if (rc != ERROR_SUCCESS)
> {
> ERR("Unable to load media info for %s (%u)\n",
> debugstr_w(file->File), rc);
> +msi_free(mi);
> return ERROR_FUNCTION_FAILED;
> }
> comp->Action = msi_get_component_action( package, comp );
And here.
Re: winhttp: Fix the error returned from WinHttpGetProxyForUrl when autodetection fails.
On Thu, 2012-11-01 at 14:52 +0100, Marvin wrote: > While running your changed tests on Windows, I think I found new failures. > Being a bot and all I'm not very good at pattern recognition, so I might be > wrong, but could you please double-check? > Full results can be found at > http://testbot.winehq.org/JobDetails.pl?Key=22654 > > Your paranoid android. > > > === WXPPROSP3 (32 bit winhttp) === > winhttp.c:2405: Test failed: got 80072ee2 > winhttp.c:2411: Test failed: got 800a This is a timeout unrelated to the tests I added. Here's a job running new tests only: https://testbot.winehq.org/JobDetails.pl?Key=22629
Re: [PATCH] kernel32: Implement CompareStringOrdinal. (try 2)
On Tue, 2012-10-30 at 09:50 +0100, Christian Costa wrote:
> Just by curiosity. What the difference with strncmpW?
strncmp tests the first string for null termination. You should really
determine that by yourself though :-)
> >> Do you have a concrete example that does not work with this implementation
> > How about CompareStringOrdinal( "a\0a", 3, "a\0b", 3 )?
> >
> >
> There is a similar test in the patch:
>
> WCHAR test1[] = { 't','e','s','t',0 };
> WCHAR test3[] = { 't','e','s','t','3',0 };
>
> ret = pCompareStringOrdinal(test1, 5, test3, 5, TRUE);
> ok(ret == CSTR_LESS_THAN, "Got %u, expected %u\n", ret, CSTR_LESS_THAN);
It's not the same test. The nulls are not at the same index and you're not
asking CompareStringOrdinal to read past a null.
Re: [PATCH] kernel32: Implement CompareStringOrdinal. (try 2)
On Tue, 2012-10-30 at 08:42 +0100, Christian Costa wrote: > > This is almost the same as your first try. You're testing a trivial > > case only - ASCII range. > > I feel like it should behave more like memcmp for case insensitive > > comparison, > > that's what documentation mildly suggests. > > > > > Well, It's not the same implementation. It's ordinal now. I have test > for that. > And how can memcmp be used for insensitive comparison? Have a look at memicmpW. > Do you have a concrete example that does not work with this implementation How about CompareStringOrdinal( "a\0a", 3, "a\0b", 3 )?
Re: [PATCH] wineboot: detect correct processor name and vendorID, use Intel Pentium 4 as fallback
On Sat, 2012-10-20 at 18:18 +0200, Patrick Rudolph wrote: > >> The code was copied from ntdll and modified in order to retrieve > >> processorname and vendorID. There's no similiár functionality in > >> wine. > > > > There is such functionality in WBEM (formerly known as WMI). I think > > you're looking for Win32_Processor.Name and > > Win32_Processor.Manufacturer. > It looks like wbem Win32_Processor.Name / Win32_Processor.Manufacturer > are only available on i386 compatible systems. What about arm ? Is it Support for ARM can be added, but I wouldn't worry about that until we find an app that needs it.
Re: [PATCH] wineboot: detect correct processor name and vendorID, use Intel Pentium 4 as fallback
On Sat, 2012-10-20 at 16:04 +0200, Patrick Rudolph wrote: > Am 2012-10-20 14:54, schrieb Eric Pouech: > > Le 20/10/2012 12:42, Patrick Rudolph a écrit : > >> This patch need lots of testing, since I can only test it on linux > >> using an Intel CPU. > >> Please tests on other OS and systems, too. please use the ntdll or > >> kernel32 relevant API so that you don't have to copy the same code > > A+ > The code was copied from ntdll and modified in order to retrieve > processorname and vendorID. There's no similiár functionality in wine. There is such functionality in WBEM (formerly known as WMI). I think you're looking for Win32_Processor.Name and Win32_Processor.Manufacturer.
Re: wbemprox: support for getting methods from WBEM objects
On Tue, 2012-10-09 at 20:30 +0200, Daniel Jelinski wrote: > diff --git a/dlls/wbemprox/table.c b/dlls/wbemprox/table.c > index c414aa9..71c62c8 100644 > --- a/dlls/wbemprox/table.c > +++ b/dlls/wbemprox/table.c > @@ -63,6 +63,7 @@ UINT get_type_size( CIMTYPE type ) > return sizeof(INT64); > case CIM_DATETIME: > case CIM_STRING: > +case CIM_OBJECT: > return sizeof(WCHAR *); This should be fixed differently. I already have a patch in my tree.
Re: [PATCH] msi: Use next cabinet from the media table instead of failing when there is a mismatch with continuous cabinet.
On Thu, 2012-09-27 at 23:07 +0200, Christian Costa wrote: > >> if (!(cabinet_file = get_cabinet_filename(mi))) > > The requirement that the continuous cabinet name matches the next cabinet > > in the media table > > might simply be too strict. Perhaps we need to try them one by one and let > > FDI decide based > > on the setID and iCabinet fields. Can you please add some tests? > > > Does not seem trivial to test this but I will try. It shouldn't be too hard, we already have some tests for continuous cabinets.
Re: [PATCH] msi: Use next cabinet from the media table instead of failing when there is a mismatch with continuous cabinet.
On Thu, 2012-09-27 at 09:59 +0200, Christian Costa wrote:
> diff --git a/dlls/msi/media.c b/dlls/msi/media.c
> index 612624d..8a39fcd 100644
> --- a/dlls/msi/media.c
> +++ b/dlls/msi/media.c
> @@ -358,8 +358,18 @@ static INT_PTR cabinet_next_cabinet(FDINOTIFICATIONTYPE
> fdint,
>
> if (strcmpiW( mi->cabinet, cab ))
> {
> -ERR("Continuous cabinet does not match the next cabinet in the Media
> table\n");
> -goto done;
> +LPSTR next_cab;
> +
> +WARN("Continuous cabinet %s does not match the next cabinet %s in
> the media table\n", debugstr_w(cab), debugstr_w(mi->cabinet));
> +
> +/* Use cabinet name from the media table */
> +next_cab = strdupWtoA(mi->cabinet);
> +/* Modify path to cabinet file with full filename (psz3 points to a
> 256 bytes buffer that can be modified contrary to psz1 and psz2) */
> +strcat(pfdin->psz3, "\\");
> +strcat(pfdin->psz3, next_cab);
> +/* Path psz3 and cabinet psz1 are concatenated by FDI so just reset
> psz1 */
> +*pfdin->psz1 = 0;
> +msi_free(next_cab);
> }
>
> if (!(cabinet_file = get_cabinet_filename(mi)))
The requirement that the continuous cabinet name matches the next cabinet in
the media table
might simply be too strict. Perhaps we need to try them one by one and let FDI
decide based
on the setID and iCabinet fields. Can you please add some tests?
Re: include/winhttp.h: Make strings in WINHTTP_PROXY_INFO non-const
On Thu, 2012-07-19 at 07:27 +, Heiko Hund wrote: > The strings shouldn't be const according to the docs at [1] and > declaring them as LPWSTR removes some false warnings when > compiling code GlobalFree()ing them with mingw. Please also remove unnecessary casts from Wine.
Re: msi: fix incorrect decoding of msi stream-names
On Wed, 2012-07-11 at 15:03 +, Oleg Jakobsen wrote: > @@ -203,6 +203,7 @@ BOOL decode_streamname(LPCWSTR in, LPWSTR out) > *out++ = mime2utf(ch&0x3f); > count++; > ch = mime2utf((ch>>6)&0x3f); > + continue; > } > } > *out++ = ch; Why is it incorrect? It seems to me that the upper bits should be written as the next character but now you are discarding them.
Re: wbemprox: Add a Win32_LogicalDisk class stub.
On Thu, 2012-06-28 at 22:22 +0300, John Yani wrote: > On 28 June 2012 22:17, Hans Leidekker wrote: > > On Thu, 2012-06-28 at 21:53 +0300, John Yani wrote: > > That's better. Does NFS actually query the Caption and Description > > properties? > > > > > Well, no. I just thought the output should be similar to the one on > winetestbot: https://testbot.winehq.org/JobDetails.pl?Key=19642&log_201=1#k201 No, that's just a test I wrote to investigate this stuff.
Re: wbemprox: Add a Win32_LogicalDisk class stub.
On Thu, 2012-06-28 at 21:53 +0300, John Yani wrote:
> +static void fill_logicaldisk( struct table *table )
> +{
> +static const WCHAR caption[] =
> +{'C',':',0};
> +static const WCHAR description[] =
> +{'L','o','c','a','l',' ','F','i','x','e','d',' ','D','i','s','k',0};
> +static const WCHAR device_id[] =
> +{'C',':',0};
> +struct record_logicaldisk *rec;
> +FIXME("returns a fake logical disks table\n");
> +if (!(table->data = heap_alloc( sizeof(*rec) )))
> +{
> +return;
> +}
> +rec = (struct record_logicaldisk *)(table->data);
> +rec->caption= caption;
> +rec->description= description;
> +rec->device_id = device_id;
> +table->num_rows = 1;
> +TRACE("created %u row(s)\n", table->num_rows);
That's better. Does NFS actually query the Caption and Description properties?
Re: wbemprox: Add a Win32_LogicalDisk class stub.
On Wed, 2012-06-27 at 14:41 +0300, John Yani wrote:
> +static void fill_logicaldisk( struct table *table )
> +{
> +static const WCHAR device_id[] = {'0',0};
> +struct record_logicaldisk *rec;
> +FIXME("returns a fake logical disks table\n");
> +if (!(table->data = heap_alloc( sizeof(*rec) )))
> +{
> +return;
> +}
> +rec = (struct record_logicaldisk *)(table->data);
> +rec->device_id= heap_strdupW( device_id );
> +table->num_rows = 1;
> +TRACE("created %u rows\n", table->num_rows);
> +}
There's no need to create a copy of device_id. Just assign it to
rec->device_id and remove COL_FLAG_DYNAMIC from the column definition.
Re: wbemprox: Add a Win32_LogicalDisk class stub.
On Wed, 2012-06-27 at 11:34 +0300, John Yani wrote:
> +static void fill_logicaldisk( struct table *table )
> +{
> +static const WCHAR fmtW[] = {'%','u',0};
> +WCHAR device_id[11];
> +struct record_logicaldisk *rec;
> +UINT num_rows = 1;
> +FIXME("returns a fake logical disks table\n");
> +if (!(table->data = heap_alloc( sizeof(*rec) * num_rows )))
> +{
> +return;
> +}
> +rec = (struct record_networkadapter *)(table->data);
This should be (struct record_logicaldisk *)(table->data);
> +sprintfW( device_id, fmtW, 0 );
> +rec->device_id= heap_strdupW( device_id );
> +
> +TRACE("created %u rows\n", num_rows);
> +table->num_rows = num_rows;
> +}
You don't need the num_rows variable since it's a fixed value. You
also don't need to build the device id, it can just be a static string.
Re: [PATCH 1/7] ntdll: Add support for junction point creation.
On Thu, 2012-06-21 at 09:57 -0600, Erich E. Hoover wrote: > On Thu, Jun 21, 2012 at 4:28 AM, Hans Leidekker wrote: > > ... > > > > This is not an atomic operation since you need two Unix calls. So you would > > need locking or rollback to deal with possible races. > > It is my understanding that directories only support advisory locks, > are you aware of a way to lock a directory? No, I don't see a way to fix this other than adding a new primitive to the kernel.
Re: [PATCH 1/7] ntdll: Add support for junction point creation.
On Wed, 2012-06-20 at 17:26 -0600, Erich E. Hoover wrote:
> +TRACE("Linking %s to %s\n", unix_src.Buffer, unix_dest.Buffer);
> +if (rmdir( unix_src.Buffer ) < 0)
> +{
> +status = FILE_GetNtStatus();
> +goto cleanup;
> +}
> +if (symlink( unix_dest.Buffer, unix_src.Buffer ) < 0)
> +{
> +status = FILE_GetNtStatus();
> +goto cleanup;
> +}
> +status = STATUS_SUCCESS;
This is not an atomic operation since you need two Unix calls. So you would
need locking or rollback to deal with possible races.
Should permissions and ownership be preserved on the directory when a
junction is removed?
Re: Incompatibility in Kernel32
On Thu, 2012-06-21 at 16:03 +0900, Dmitry Timoshkov wrote:
> > Shall I file a bug report?
>
> Try to add a test case to dlls/kenel32/tests/file.c,test_file_sharing().
>
> > HANDLE h2 = CreateFileA(filename, GENERIC_READ, FILE_SHARE_DELETE |
> > FILE_SHARE_READ, NULL, OPEN_EXISTING, 0, 0);
> > assert (h2 != 0);
> >
> > if (!DeleteFile(filename)) {
> > r = GetLastError();
> > fprintf(stderr, "This only happens in Wine. I got error: %d\n", r);
> > assert(0);
> > }
>
> Is there any reason that you call DeleteFile() on a still being opened file?
That's the whole point of this test. The observed behavior is that you can pass
a
filename to MsiRecordSetStream, successfully delete the file, and keep on using
the
stream.
We currently implement MsiRecordSetStream by creating an in-memory copy of the
stream. As Robert noticed, this will fail if the stream is too large.
The idea is to move to an implementation on top of a file handle, but it should
still allow DeleteFile to succeed while the file is in use.
RE: msi:RECORD_StreamFromFile bug, help needed
On Wed, 2012-06-20 at 12:13 +0200, robert.van.h...@serioustoys.com wrote: > IStream is not exposed to the user, but an internal interface would > probably end up looking a lot like IStream because we need read/write/seek > operations, reference counting and the ability to create clones. So we > might as well use IStream. > > Hmmm... Then I want exactly the same as SHCreateStreamOnFileEx, but the > only difference is that the file should be opened with FILE_SHARE_DELETE. > (I think). You will also need IStream_Clone, which shlwapi doesn't implement. > What is that policy on this? It seems overkill to copy istream.c from > shlwapi, just because I want to change 1 flag. On the other hand, I don't > want to expose more functions in the shlwapi header, as that would make > it different from the Windows implementation... You can look at istream.c for inspiration, but please avoid copying it verbatim with its 2-space indentation and CamelCaseNotation. Try following the coding style of surrounding code in msi. You can probably also simplify the code a bit. E.g. you can leave out parameter validation and code/structure fields that will not be used by msi.
RE: msi:RECORD_StreamFromFile bug, help needed
On Tue, 2012-06-19 at 21:45 +0200, robert.van.h...@serioustoys.com wrote: > > We don't have to call SHCreateStreamOnFileEx, we can add a suitable > > implementation to msi. > > Ah, I see. > > I guess this stream cannot escape from MSI. The only way it can be accessed is > through MsiRecordReadStream and MsiRecordSetStream, but it can never be > accessed > directly? > > In that case, we don't have to use an IStream... > I could alter msipriv.h such that MSIFIELD just holds a HANDLE in case the > field > is a stream. > The handle can then be created through CreateFile, and I guess then I could > alter > the permissions such that it matches the MS implementation... > > Do you think that that is the way to go? > > Or is there a deep reason why we should use IStream, and create our own > IStream > implementation? IStream is not exposed to the user, but an internal interface would probably end up looking a lot like IStream because we need read/write/seek operations, reference counting and the ability to create clones. So we might as well use IStream.
RE: msi:RECORD_StreamFromFile bug, help needed
On Tue, 2012-06-19 at 20:55 +0200, robert.van.h...@serioustoys.com wrote: > > Yes, the concern is that the file could be changed or removed. We should > > test what native does here but it probably means that we need to create a > > stream > > on the file with a sharing mode that denies writing. > > So... I tried this out. I do not understand what I found though! > > On Windows, after MSI_RecordSetStreamFromFileW, you CAN call DeleteFile on > that file. > But you cannot open the file for writing; that will give ERROR_ACCESS_DENIED. > > I found in the specs of DeleteFile that this behaviour is understandable, > because > " > The DeleteFile function marks a file for deletion on close. Therefore, the > file > deletion does not occur until the last handle to the file is closed. > Subsequent > calls to CreateFile to open the file fail with ERROR_ACCESS_DENIED. > " > > On the other hand, it says: > " > The DeleteFile function fails if an application attempts to delete a file that > is open for normal I/O or as a memory-mapped file. > " > > So... I figured that somehow in the MS implementation of > RecordSetStreamFromFile > they called SHCreateStreamOnFileEx with parameters that allowed it to be > scheduled > for deletion through DeleteFile, and didn't allow the file to be opened again > for > writing. > > So far so good. > > So I set out to find this combination of parameters and I cannot find it. I > saw > that for CreateFile, you can pass FILE_SHARE_DELETE, and I guess that that > allows > for DeleteFile to be called. However, you cannot pass flags to > SHCreateStreamOnFileEx > that will cause this flag to be set... We don't have to call SHCreateStreamOnFileEx, we can add a suitable implementation to msi.
RE: msi:RECORD_StreamFromFile bug, help needed
On Tue, 2012-06-19 at 12:36 +0200, robert.van.h...@serioustoys.com wrote: > I altered my local source tree, such that it opens the file exclusively as a > stream. That seems to work. > > Having that fixed, at least provisionally, I am now facing another issue: my > MSIs > are most of the time very small, only about 5 megs. This is wrong. In some > runs, > it goes fine though, and they reach 300 megs, which is the right size. > I find no error messages in the +relay trace... The process seems to be > completely > unpredictable. > > I can see in my Windows temp dir that the temporary files created there do > seem to > add up to about 300 megs, but then when the final MSI is baked, something > seems > to go wrong... (?) > > Would you have any idea of how this could be? > Maybe some race-condition? > > It seems to be unrelated to the other bug (the one described before: > GlobalAllocing > a big block to copy file contents into), because before this fix this current > behaviour also happened in the instances where the GlobalAlloc didn't fail. Can you please open a bug report for this and attach a +msi,+msidb trace?
Re: msi:RECORD_StreamFromFile bug, help needed
On Mon, 2012-06-18 at 14:14 +0200, robert.van.h...@serioustoys.com wrote: > But then I was thinking: this seems like such an obvious thing to do, that it > is > almost suspicious. Was there any reason for copying the file contents to main > memory and then create a memory stream? > > For instance, was the intention to have a non-mutable copy in memory, in case > the backing file would be altered later on? Yes, the concern is that the file could be changed or removed. We should test what native does here but it probably means that we need to create a stream on the file with a sharing mode that denies writing. > I guess if that's the case, I could just create a copy of the original file in > a temp dir, and mark it as STGM_DELETEONRELEASE. It would take a long time to copy such a large file.
Re: [1/5] msi: Add support for 64-bit registry components.
On Tue, 2012-06-05 at 20:35 +0200, Alexandre Julliard wrote: > Hans Leidekker writes: > > > --- > > dlls/msi/action.c| 30 +++--- > > dlls/msi/msi_main.c |1 + > > dlls/msi/msipriv.h |1 + > > dlls/msi/package.c |2 - > > dlls/msi/tests/install.c | 140 > > ++ > > 5 files changed, 162 insertions(+), 12 deletions(-) > > This breaks the Gecko installer on 64-bit for me. That's a bug in the installer, its registry component should be marked as a 64-bit component.
Re: msi: add support for MSIRUNMODE_REBOOTNOW in MsiGetMode
On Fri, 2012-03-30 at 17:27 -0700, Austin English wrote: > +case MSIRUNMODE_REBOOTNOW: > +r = package->need_reboot; > +break; > + MSIRUNMODE_REBOOTNOW and MSIRUNMODE_REBOOTATEND are not the same thing.
Re: MsiGetFileHash
On Thu, 2012-03-29 at 18:29 +0200, Robert van Herk wrote:
> + mapping = CreateFileMappingW( handle, NULL, PAGE_READONLY, 0, 0, NULL
> );
> + if (mapping)
> + {
> + p = MapViewOfFile( mapping, FILE_MAP_READ, 0, 0, length );
> + if (p)
> + {
> + MD5_CTX ctx;
> +
> + MD5Init( &ctx );
> + MD5Update( &ctx, p, length );
> + MD5Final( &ctx );
> + UnmapViewOfFile( p );
> +
> + memcpy( pHash->dwData, ctx.digest, sizeof pHash->dwData );
> + r = ERROR_SUCCESS;
> + }
> + CloseHandle( mapping );
> + }
> +} else {
> + /* Empty file -> set hash to 0 */
> + memset( pHash->dwData, 0, sizeof pHash->dwData );
> + r = ERROR_SUCCESS;
> }
> +
> CloseHandle( handle );
Please use 4 space indentation and put brackets on a new line like in the
rest of that function. Same for the test.
Please also provide a more descriptive subject line and prefix it with "msi: "
Re: [PATCH] implement MSIMODIFY_MERGE function in TABLE_modify
On Mon, 2012-03-26 at 11:14 +0200, Andoni Morales wrote:
> -case MSIMODIFY_REPLACE:
> case MSIMODIFY_MERGE:
> + /* check for a duplicated key */
> + r = msi_table_find_row( tv, rec, &row, column );
> + if (r == ERROR_SUCCESS) {
> +/* check that both are identical */
> +r = compare_record (tv, row, rec);
> +if (r != ERROR_SUCCESS)
> + break;
> + } else {
> +r = table_validate_new( tv, rec, NULL );
> +if (r != ERROR_SUCCESS)
> + break;
> +r = TABLE_insert_row( view, rec, -1, FALSE );
> + break;
> + }
> +
> +case MSIMODIFY_REPLACE:
Please use bracketing and indentation style of the surrounding code.
Some tests would be nice.
Re: advapi32: Assign a default value (clang)
On Fri, 2012-03-09 at 19:54 +1100, Alistair Leslie-Hughes wrote:
> @@ -755,7 +755,7 @@ SC_HANDLE WINAPI OpenSCManagerA( LPCSTR lpMachineName,
> LPCSTR lpDatabaseName,
> SC_HANDLE WINAPI OpenSCManagerW( LPCWSTR lpMachineName, LPCWSTR
> lpDatabaseName,
> DWORD dwDesiredAccess )
> {
> -SC_HANDLE handle;
> +SC_HANDLE handle = INVALID_HANDLE_VALUE;
These functions return NULL on failure.
Re: [1/2] ws2_32/tests: Test some completion port behaviour.
Hi Ričardas, > +key = 0xdeadbeef; > +num_bytes = 0xdeadbeef; > +olp = (WSAOVERLAPPED *)0xdeadbeef; > +bret = GetQueuedCompletionStatus( io_port, &num_bytes, &key, &olp, 100 ); > +todo_wine ok( bret == FALSE && GetLastError() == ERROR_NETNAME_DELETED, > "failed to get completion status %u\n", GetLastError() ); For tests like this you should set last error to some known value before calling the function. And it would be better to split them up; one test for the return value and one for the error. > +key = 0xdeadbeef; > +num_bytes = 0xdeadbeef; > +olp = (WSAOVERLAPPED *)0xdeadbeef; > +bret = GetQueuedCompletionStatus( io_port, &num_bytes, &key, &olp, 100 ); > +todo_wine ok(bret == FALSE && GetLastError() == ERROR_OPERATION_ABORTED, > "failed to get completion status %u\n", GetLastError() ); > +todo_wine ok(key == 125, "Key is %lu\n", key); > +todo_wine ok(num_bytes == 0, "Number of bytes transfered is %u\n", > num_bytes); > +todo_wine ok(olp == &ov, "Overlaped structure is at %p\n", olp); > +todo_wine ok(olp && (olp->Internal == (ULONG)STATUS_CANCELLED), > "Internal status is %lx\n", olp ? olp->Internal : 0); You may want to call GetQueuedCompletionStatus again here to show that this was the last completion.
Re: [PATCH 5/7] msi/custom.c: get rid of MSIRUNNINGACTION
> -static UINT HANDLE_CustomType50(MSIPACKAGE *package, LPCWSTR source,
> +static msi_custom_action_info *HANDLE_CustomType50(MSIPACKAGE *package,
> LPCWSTR source,
> LPCWSTR target, const INT type, LPCWSTR
> action)
> {
> WCHAR *exe, *arg;
> msi_custom_action_info *info;
>
> -if (!(exe = msi_dup_property( package->db, source ))) return
> ERROR_SUCCESS;
> +if (!(exe = msi_dup_property( package->db, source ))) return NULL;
Are you sure this should be an error? Native might ignore a missing property.
> -static UINT HANDLE_CustomType53_54(MSIPACKAGE *package, LPCWSTR source,
> +static msi_custom_action_info *HANDLE_CustomType53_54(MSIPACKAGE *package,
> LPCWSTR source,
> LPCWSTR target, const INT type, LPCWSTR
> action)
> {
> msi_custom_action_info *info;
> WCHAR *prop;
> -UINT r;
>
> TRACE("%s %s\n", debugstr_w(source), debugstr_w(target));
>
> prop = msi_dup_property( package->db, source );
> -if (!prop) return ERROR_SUCCESS;
> +if (!prop) return NULL;
Same here.
Re: [PATCH 1/7] msi/custom.c: unify thread handling of different custom action types (resend)
On Tue, 2011-10-11 at 13:20 +0200, Bernhard Loos wrote:
+static HRESULT get_action_info( msi_custom_action_info *info, INT *type,
+BSTR *source, BSTR *target, BSTR *name,
+IWineMsiRemotePackage **package )
{
-IClassFactory *cf = NULL;
-IWineMsiRemoteCustomAction *rca = NULL;
-HRESULT r;
-
-r = DllGetClassObject( &CLSID_WineMsiRemoteCustomAction,
- &IID_IClassFactory, (LPVOID *)&cf );
-if (FAILED(r))
-{
-ERR("failed to get IClassFactory interface\n");
-return ERROR_FUNCTION_FAILED;
-}
-
-r = IClassFactory_CreateInstance( cf, NULL,
&IID_IWineMsiRemoteCustomAction, (LPVOID *)&rca );
-if (FAILED(r))
-{
-ERR("failed to get IWineMsiRemoteCustomAction interface\n");
-return ERROR_FUNCTION_FAILED;
-}
-
-r = IWineMsiRemoteCustomAction_GetActionInfo( rca, guid, type, handle,
dll, funcname, package );
-IWineMsiRemoteCustomAction_Release( rca );
-if (FAILED(r))
-{
-ERR("GetActionInfo failed\n");
-return ERROR_FUNCTION_FAILED;
+IWineMsiRemotePackage *rp;
+HRESULT res;
+*source = *target = *name = NULL;
+
+res = create_msi_remote_package( NULL, (void **) &rp );
+if (FAILED( res ))
+return res;
+res = IWineMsiRemotePackage_SetMsiHandle( rp, alloc_msihandle(
&info->package->hdr ) );
You should check for failure from alloc_msihandle.
+if (FAILED( res ))
+goto error;
+
+*source = SysAllocString( info->source );
+*target = SysAllocString( info->target );
+*name= SysAllocString( info->action );
+if (!*source || !*target || !*name) {
+res = E_OUTOFMEMORY;
+goto error;
}
-
+
+*package = rp;
+*type= info->type;
+
return ERROR_SUCCESS;
This should become S_OK. You are mixing up return types in more places.
Re: Update on testbot
On Sun, 2011-09-18 at 16:05 +0200, Maarten Lankhorst wrote: > I moved the windows 7 x64 vm with sp1 applied to the base pool now. This means > that all submitted patches will be run on that vm as well. The original win7 > vm > is still available under its own name. Since all tests pass on that according > to winehq tests I'm keeping it in the base pool. I hope to see all tests on > the new vm fixed as well. They mostly appear to be related to the ie9 update, > but msctf:inputprocessor and msi:install fail as well. The msi:install test is fixed now as well as a failure in wininet:http on ie9. The msi:install failure was caused by pending renames on that machine, which is good for testing variety but you may want to check that out nonetheless.
Re: msi: Print the string and not the pointer in a TRACE.
On Tue, 2011-09-20 at 10:51 +0200, Michael Stefaniuc wrote:
> Hans Leidekker wrote:
> > On Mon, 2011-09-19 at 21:24 +0200, Michael Stefaniuc wrote:
> >> @@ -489,7 +489,7 @@ UINT WINAPI MsiGetSourcePathA( MSIHANDLE hInstall,
> >> LPCSTR szFolder,
> >> awstring str;
> >> UINT r;
> >>
> >> -TRACE("%s %p %p\n", debugstr_a(szFolder), debugstr_a(szPathBuf),
> >> pcchPathBuf);
> >> +TRACE("%s %s %p\n", debugstr_a(szFolder), debugstr_a(szPathBuf),
> >> pcchPathBuf);
> >
> > szPathBuf is an output buffer.
> Ok, then the format should stay as %p and the debugstr_a() should go.
Yes, note that Francois has already sent a patch.
Re: msi: Print the string and not the pointer in a TRACE.
On Mon, 2011-09-19 at 21:24 +0200, Michael Stefaniuc wrote:
> @@ -489,7 +489,7 @@ UINT WINAPI MsiGetSourcePathA( MSIHANDLE hInstall, LPCSTR
> szFolder,
> awstring str;
> UINT r;
>
> -TRACE("%s %p %p\n", debugstr_a(szFolder), debugstr_a(szPathBuf),
> pcchPathBuf);
> +TRACE("%s %s %p\n", debugstr_a(szFolder), debugstr_a(szPathBuf),
> pcchPathBuf);
szPathBuf is an output buffer.
Re: [PATCH] msiexec: implement a sane server stub (i.e. not while(TRUE); )
On Fri, 2011-08-26 at 09:32 +0200, Bernhard Loos wrote: > On Fri, Aug 26, 2011 at 9:11 AM, Hans Leidekker wrote: > > On Fri, 2011-08-26 at 04:58 +0200, Bernhard Loos wrote: > > > > - * Copyright 2007 Google (James Hawkins) > > + * Copyright 2011 Bernhard Loos > > > > Please keep the original copyright information. > > > > Well, there is basically nothing left of the original file. Even if there's nothing left I think it serves to acknowledge the person who started the implementation. On occasion it's also useful to know when the implementation was started. Though of course all of this information can also be dug up from git history.
Re: [PATCH] msiexec: implement a sane server stub (i.e. not while(TRUE); )
On Fri, 2011-08-26 at 04:58 +0200, Bernhard Loos wrote: - * Copyright 2007 Google (James Hawkins) + * Copyright 2011 Bernhard Loos Please keep the original copyright information.
Re: (resend)usp10/test: test ScriptXtoX on an RTL set with differring cChars and cGlyphs
On Thu, 2011-08-25 at 07:38 -0500, Aric Stewart wrote: > Yes, it depends on if it is a RTL or LTR string. That is correct. What I meant is that it does not always match the right hand operand in the test condition.
Re: (resend)usp10/test: test ScriptXtoX on an RTL set with differring cChars and cGlyphs
On Thu, 2011-08-25 at 07:15 -0500, Aric Stewart wrote: > "should return piX=0 not 0"? seems like 0 == 0 to me. They are both > int values... That test doesn't look right: winetest_ok(piX == offsets[iCP+1], "ScriptCPtoX trailing: iCP=%d should return piX=%d not %d\n", iCP, offsets[iCP+direction], piX); direction can be either -1 or 1.
Re: [2/3] msi/tests: Skip a test if the process is limited.
On Wed, 2011-08-24 at 15:11 +0200, Francois Gouget wrote: > From my understanding, in this case using a simple skip() means that > anyone running WineTest on Windows in a non-administrator account will > get a test failure. Since we don't seem to have a policy mandating that Quite the opposite, running it as a regular user causes a test failure now, my patch avoids that.
Re: [2/3] msi/tests: Skip a test if the process is limited.
On Wed, 2011-08-24 at 03:26 -0500, Austin English wrote: > Shouldn't that be a win_skip()? Wine always runs as administrator, and > if the process is running as a limited user, something may be broken? > Unless you're preparing for when wine supports non-admin mode.. :) No, win_skip is for behavior observed on Windows that Wine should not emulate, which is not the case here. And testing if a process is correctly limited should be done elsewhere.
Re: wldap32/tests: Skip tests if ldap_init failed (try 3)
On Tue, 2011-08-09 at 19:31 +0200, André Hentschel wrote:
> diff --git a/dlls/wldap32/tests/parse.c b/dlls/wldap32/tests/parse.c
> index 4cf8564..ed37d88 100644
> --- a/dlls/wldap32/tests/parse.c
> +++ b/dlls/wldap32/tests/parse.c
> @@ -128,7 +128,11 @@ START_TEST (parse)
> LDAP *ld;
>
> ld = ldap_initA((char *)"ldap.itd.umich.edu", 389 );
> -ok( ld != NULL, "ldap_init failed\n" );
> +if (!ld)
> +{
> +skip("ldap_init failed, skipping tests\n");
> +return;
> +}
>
> test_ldap_parse_sort_control( ld );
> test_ldap_search_extW( ld );
If it fails on Wine it's probably because LDAP development files
were not present at build time.
I'm not sure if we want to hide this failure though, applications
may expect ldap_init to succeed.
Re: wldap32/tests: Skip tests if ldap_init failed
On Tue, 2011-08-09 at 19:17 +0200, André Hentschel wrote:
> diff --git a/dlls/wldap32/tests/parse.c b/dlls/wldap32/tests/parse.c
> index 4cf8564..faad474 100644
> --- a/dlls/wldap32/tests/parse.c
> +++ b/dlls/wldap32/tests/parse.c
> @@ -128,7 +128,11 @@ START_TEST (parse)
> LDAP *ld;
>
> ld = ldap_initA((char *)"ldap.itd.umich.edu", 389 );
> -ok( ld != NULL, "ldap_init failed\n" );
> +if (!ld)
> +{
> +win_skip("ldap_init failed, skipping tests\n");
> +return;
> +}
>
> test_ldap_parse_sort_control( ld );
> test_ldap_search_extW( ld );
Do you see it fail on Windows?
Re: advapi32: Add stub TraceMessage, TraceMessageVa
On Wed, 2011-08-03 at 13:01 +1000, Alistair Leslie-Hughes wrote: -@ stub TraceMessage -@ stub TraceMessageVa +@ varargs TraceMessage(long long ptr long) +@ stdcall TraceMessageVa(long long ptr long ptr) These functions take a TRACEHANDLE as the first argument which is an int64.
Re: [2/3] winhttp: Implement IWinHttpRequest::get_ResponseBody. (try 2)
On Sat, 2011-07-23 at 16:01 +0200, Jacek Caban wrote: > > +VariantInit( &body ); > > +V_VT( &body ) = VT_ERROR; > > +hr = IWinHttpRequest_get_ResponseBody( req, &body ); > > +ok( hr == S_OK, "got %08x\n", hr ); > > +ok( V_VT( &body ) == (VT_ARRAY|VT_UI1), "got %08x\n", V_VT( &body ) ); > > + > > +ok( hr == S_OK, "got %08x\n", hr ); > > This duplicates the above test. Did you mean to test the result of > VariantClear here (otherwise there is a leak)? Yes, looks like that line got lost somehow. Thanks for the notice.
Re: [2/3] winhttp: Implement IWinHttpRequest::get_ResponseBody. (try 2)
On Sat, 2011-07-23 at 09:57 +0200, Marvin wrote: > === W2K8SE (32 bit winhttp) === > winhttp.c:926: Test failed: failed to receive response 12152 > winhttp.c:933: Test failed: expected ERROR_INSUFFICIENT_BUFFER, got 12019 > winhttp.c:937: Test failed: failed unexpectedly 12019 > winhttp.c:938: Test failed: unexpected size 0 > winhttp.c:943: Test failed: failed unexpectedly 12019 12152 == ERROR_WINHTTP_INVALID_SERVER_RESPONSE This is outside the tests added by this patch.
Re: [2/3] winhttp: Implement IWinHttpRequest::get_ResponseBody.
On Fri, 2011-07-22 at 16:16 +0200, Jacek Caban wrote: > The caller is responsible for freeing the result, so it will destroy > your internal data. I think you should create an array here and not use > it to store the data internally. How do you know that the caller is responsible for freeing this property? It seems wasteful to duplicate this potentially large buffer.
Re: TaskDialog implementation
On Fri, 2011-06-10 at 23:29 -0500, Vincent Povirk wrote: > Well, I think you may be right that changing it to E_NOTIMPL at this > point would break something and might be a bad idea. Sadly, Hans > didn't say what the stub was originally for. Hopefully, we'll soon > have a working implementation and it won't matter. ;) I added it for the Internet Explorer 9 installer but it has been broken already when someone changed it to return IDYES instead of IDOK in pnButton. I don't know which app that was for.
Re: advapi: test and fix GetUserName() in case where buffer is too small
On Tue, 2011-05-31 at 06:32 -0500, Andrew Nguyen wrote: > -DWORD sizeW = *lpSize * 2; > +DWORD sizeW = 0; > + > +GetUserNameW( NULL, &sizeW ); Can we avoid the extra call? > I didn't intend to scoop Dan's work, but I have a version of his fix, > which is attached, with deeper tests which I wrote before Hans expressed > his concerns. Should I just send in the tests with todo_wine and let Dan > fix those, or should I send a revised version and allow a "winner" to be > picked? I vote for getting some tests in first.
Re: advapi: test and fix GetUserName() in case where buffer is too small
On Mon, 2011-05-30 at 21:13 -0700, Dan Kegel wrote:
> @@ -91,7 +91,7 @@ GetUserNameW( LPWSTR lpszName, LPDWORD lpSize )
>
> if (len > *lpSize)
> {
> -SetLastError(ERROR_MORE_DATA);
> +SetLastError(ERROR_INSUFFICIENT_BUFFER);
GetUserNameA also needs to be fixed. It sets ERROR_MORE_DATA when
WideCharToMultiByte fails, which can simply be removed because that
function will set ERROR_INSUFFICIENT_BUFFER itself if the buffer is too
small.
And there are at least two cases, in dlls/crypt32/protectdata.c and
dlls/msi/package.c, where we depend on the wrong error.
