Re: Bad CPU autoscaling is making games unplayable for a lot of users

2013-09-16 Thread David Laight
On Fri, Sep 13, 2013 at 02:14:24PM -0700, Scott Ritchie wrote:
> Over the past couple years I've been able to try out Wine games on many
> different environments -- laptops, desktops, even cloud servers.
> 
> On many occasions, I've discovered that a game appears to run
> functionally but slowly, however upon further investigation I find that
> forcing the CPU to run at 100% "performance" mode can make the game
> playable.

I've found similar problems running high cpu use programs on
both linux and windows.
It seems that the OS doesn't increase the cpu frequency until all the
cpu are busy - so for a 'mostly single-threaded' application this
doesn't happen and the cpus stays at 1.6GHz instead of increasing
to 3.4GHz.

Running 'while :; do :; done' once for eacg cpu makes the program
run faster!

David

-- 
David Laight: da...@l8s.co.uk




Re: [wine-devel] Re: Bug 24018 which appears to be a showstopper for running Cygwin on Wine

2013-07-01 Thread David Laight
On Mon, Jul 01, 2013 at 06:13:26PM +0200, Peter Rosin wrote:
> 
> I would like to point out that it seems that the current bug does not
> appear to be *in* setup.exe, but rather occurs when setup.exe runs
> a bash post-install script, where the bash.exe that interprets the
> script depends on cygwin1.dll. If my analysis is correct, the bug
> would occur also when duplicating the actions of setup.exe manually.

If you read into what cygwin does when trying to emulate fork() it
is surprising it works at all!

Putting some effort into getting the shell (be it bash or an ash derivative)
and gmake to use posix_spawn() instead of fork() and adding posix_spawn()
to cygwin (for which patches are avaiable) would speed up cygwin no end
and probably avoid  some of these bugs (and the random fork failed errors).

One day I'll get annoyed at some issues with cygwin and start fixing them!
(Like getting ^C to work for windows console programs in mintty windows.)

    David

-- 
David Laight: da...@l8s.co.uk




Re: d3dx9: Implement D3DXSHMultilpy5

2013-04-15 Thread David Laight
On Mon, Apr 15, 2013 at 10:24:28AM +0200, Rico Sch?ller wrote:
> You are right. I'm not really sure why the code should be slower though. 
> The #defines shouldn't have an impact on the performance, but it might 
> be because it is translated to:
> 
> ta = 0.28209479f * a[0] + -0.12615662f * a[6] + -0.21850968f * a[8];
> tb = 0.28209479f * b[0] + -0.12615662f * b[6] + -0.21850968f * b[8];
> out[1] = 0.0f + ta * b[1] + tb * a[1];
> t = a[1] * b[1];
> out[0] = out[0] + 0.28209479f * t;
> out[6] = 0.0f + -0.12615662f * t;
> out[8] = 0.0f + -0.21850968f * t;
> 
> instead of:
> ta = 0.28209479f * a[0] - 0.12615662f * a[6] - 0.21850968f * a[8];
> tb = 0.28209479f * b[0] - 0.12615662f * b[6] - 0.21850968f * b[8];
> out[1] = ta * b[1] + tb * a[1];
> t = a[1] * b[1];
> out[0] += 0.28209479f * t;
> out[6] = -0.12615662f * t;
> out[8] = -0.21850968f * t;

If everything is 'float' (no doubles anywhere) then I can't see
why the above would compile to different code at any sane
optimisation level - best to look at the object code.

Unless the compiler knows that out[] can't overlap a[] or b[] the
generated code is likely to be better if 't' is evaluated before
the write to out[1].

David

-- 
David Laight: da...@l8s.co.uk




Re: [PATCH v3 0/7] Add O_DENY* support for VFS and CIFS/NFS

2013-03-01 Thread David Laight
On Thu, Feb 28, 2013 at 01:53:25PM -0800, Andy Lutomirski wrote:
> > O_DENYMAND - to switch on/off three flags above.
> 
> O_DENYMAND doesn't deny anything.  Would a name like O_RESPECT_DENY be
> better?

Possibly rename to O_CHECK_DENY ?

    David

-- 
David Laight: da...@l8s.co.uk




Re: d3dx9: Avoid expensive computations

2013-02-26 Thread David Laight
On Mon, Feb 25, 2013 at 11:08:02AM +0100, Henri Verbeet wrote:
> On 25 February 2013 10:24, Rico Sch?ller  wrote:
> > I did some small tests for speed with the following results. You may also
> > avoid such a lot of variable assignments like *pout = out and you may use 4
> > vecs instead. This should save ~48 assignments and it should also improve
> > the speed a bit more (~10%). Though, native is still 40% faster than that.
> >
> I'd somewhat expect native to use SSE versions of this kind of thing
> when the CPU supports those instructions. You also generally want to
> pay attention to the order in which you access memory, although
> perhaps it doesn't matter so much here because an entire matrix should
> be able to fit in a single cacheline, provided it's properly aligned.

Also make sure that are memory that is written to can't be aliased
by the memory that is read.
If aliasing is possible the compiler has sequence the code to
ensure the writes and reads happen in the correct order.

That function probably has too many live values for the normal
registers - so some values will get flushed to stack.
SSE might be better.

David

-- 
David Laight: da...@l8s.co.uk




Re: Adding DECLSPEC_ALIGN(4) to structs for atomic DWORD access

2013-01-21 Thread David Laight
On Mon, Jan 21, 2013 at 06:52:23PM +0100, joerg-cyril.hoe...@t-systems.com 
wrote:
> Hi,
> 
> there are some places in Wine where I believe atomic access to 32bit entities
> is expected by the programmer, yet I see little or no measures to ensure this.
> 
> DWORD-Alignment is essential when several threads can access
> the same DWORD variable outside of a mutex (or within the implementation of 
> the mutex).
> 
> DECLSPEC_ALIGN(4) is the declaration that could be used to enforce
> it within a struct, assuming the struct itself is somehow aligned.

32bit items will be 32bit aligned unless the structure is 'packed'
(or similar).

David

-- 
David Laight: da...@l8s.co.uk




Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs

2012-12-12 Thread David Laight
On Sat, Dec 08, 2012 at 12:43:14AM +0400, Pavel Shilovsky wrote:
> 
> The problem is the possibility of denial-of-service attacks here. We 
> can try to prevent them by:

FWIW I already see a DoS 'attack'.
I have some filestore shared using NFS (to Linux and Solaris) and 
using samba (to Windows).

I use it for release builds of a product to ensure the versions
built for the different operating systems match, and because some
files have to be built on an 'alien' system (eg gcc targetted at
embedded card).

I can't run the windows build at the same time as the others
because the windows C compiler manages to obtain exclusive access
to the source files - stopping the other systems from reading them.

David

-- 
David Laight: da...@l8s.co.uk




Re: [PATCH 1/4] kernel32: Support UTF-7 in MultiByteToWideChar.

2012-12-05 Thread David Laight
On Tue, Dec 04, 2012 at 08:30:55PM -0700, Alex Henrie wrote:
> 2012/12/4 Fr?d?ric Delanoy :
> > The above MSDN comment indicates pre-Vista versions are buggy, so it's
> > probably not a good idea to match that behaviour.
> 
> I think encoding and decoding in UTF-7 arbitrary binary data was
> considered a "feature" in Windows XP. As MSDN said, "Code written in
> earlier versions of Windows that rely on this behavior to encode
> random non-text binary data might run into problems." So I'm sure
> there's at least one application that depends on the data not being
> Unicode-normalized. Whoever adds normalization will have to make sure
> it's turned off in Windows XP (or older) mode.

Actually UTF-8 is a PITA - a program has to know whether every
individual C string (or file) is UTF-8 or 8bit ascii (well 8859-x).
Assuming UTF-8 doesn't work unless in can process all arbitrary
byte sequences (and write them back) - which the standard doesn't
allow for.

In the US it probably isn't often an issue, but in europe there are
mane files that have occaisional characters with the top bit set.
In the UK we only see 0xA3 (pound sterling) - but it can crop up
anywhere - and causes my mail program (which, for some reason I
don't understand) assumes UTF-8 do drop core responsing to mails!

David

-- 
David Laight: da...@l8s.co.uk




Re: [PATCH] includes: always add __force_align_arg_pointer__ for i386

2012-11-12 Thread David Laight
On Mon, Nov 12, 2012 at 09:25:57AM +0800, Dmitry Timoshkov wrote:
> Maarten Lankhorst  wrote:
> 
> > Fixes flstudio installer on my system, which breaks otherwise in 
> > strcasestr_sse42_nonascii
> > due to stack being misaligned.
> 
> That's a compiler bug: http://bugs.winehq.org/show_bug.cgi?id=22316

Having recently gone around that gcc 'feature loop' for NetBSD (NetBSD
6 (just released) has gcc build to only assume/maintain 4 byte stack
alignment), but not having looked at strcasestr_sse42_nonascii() itself!

There is thread on some linux mailing listA about issues with the
assumption of 16 byte stack alignment, and someone trying to run code
where the compielr had been patched to generate a prologue that generated
an error message if stack frames were misaligned - it happened a lot
even on with code only compiled by gcc (probably due to asm stubs and
old code/code compiled with different options).

We discovered that -mstackalign eventualy got fixed to align stacks of
functions that save 16byte aligned items on stack (used %bp to access
parameters and %sp to locals - horrid if alloca() also used).

If you tell gcc the stack is only 4 byte aligned, gcc will generate
some internal consistency errors unless some other patches are made.
Maintaining 8 byte alignment should be ok.

If you only set -mstackalign then the default alignment is assumed
valid for items requesting 8-byte alignment. This is done becase it
is useful to align double and long long - but the cpu won't fault
on any misaligned accesses to 8-byte items.

I'm not sure anyone has actually done any measurements that account
for the increased cache usage of the 16 byte aligning the stack against
the gain for not having to align it for the rare functions that
actually need 16 byte alignment.

Intereseting, maintaining 8 byte alignment - so that double can be
chaply aligned - may be more useful!

NetBSD found issues with 'normal' code that, when compiled with
suitable options, would try to save SSE2 registers on stack (loop
got vectorised by the compiler).

>From a wine point of view, any code that calls into libc (or similar)
and might have come from microsoft code probably has to align the stack
(set the incoming stack alignment - possibly as an attribute).
Functions that don't call out of wine can just set -mstackrealign in case
they happen to use SSE2 instrctions.

It is all a pig's breakfast.

David

-- 
David Laight: da...@l8s.co.uk




Re: WANTED: compiler barrier for use within Wine

2012-11-01 Thread David Laight
On Thu, Nov 01, 2012 at 09:25:53AM +0100, joerg-cyril.hoe...@t-systems.com 
wrote:
> 
> Background: I want an atomic read of whatever value a single aligned
> int (or DOWRD or whatever) currently appears visible to one core.

A simple read of a volatile data item will do that.
Look at what signal handlers are allowed to change.

New compilers link clang support the gcc asm semantics - they need to
in order to compile a lot of their target code.

The purpose of 'asm volatile ("":::""memory);' is to constrain
the values that the compiler can have cached in registers.

I use it for 3 purposes:
1) To ensure object code accesses memory in the correct order (provided
   the cpu can't reorder the memory accesses itself (which many modern
   cpu will do - at least for cached accesses).
2) To reduce the number 'live' values in a function to stop values
   being spilled to stack.
3) To force the generated code to read values early (sometimes before
   an 'if' when the value is only used in one clause) in order to
   remove cpu stalls waiting for memory and to fill otherwise
   unfillable delay slots.
   This one is very cpu specific - but I am counting every instruction
   in that function.
   (This is a MIPS-like embedded cpu.)

David

-- 
David Laight: da...@l8s.co.uk




Re: [PATCH 5/8] ntdll: Add detection for PF_SSE_DAZ_MODE_AVAILABLE

2012-10-21 Thread David Laight
On Sun, Oct 21, 2012 at 04:16:04PM -0600, James Eder wrote:
> Hmmm... the GCC docs say:
> 
> http://gcc.gnu.org/onlinedocs/gcc-4.7.2/gcc/Type-Attributes.html#Type-Attributes
> 
> "Note that the alignment of any given struct or union type is required
> by the ISO C standard to be at least a perfect multiple of the lowest
> common multiple of the alignments of all of the members of the struct
> or union in question. This means that you can effectively adjust the
> alignment of a struct or union type by attaching an aligned attribute
> to any one of the members of such a type, [...]"
> 
> So, with conforming compilers, the having M128A aligned makes
> XMM_SAVE_AREA32 aligned. I tried it out some and I wasn't able to find
> a case which GCC (4.7.2) didn't align XMM_SAVE_AREA32 at 16 with M128A
> declared the way it is.

With the default gcc configuration and parameters used on Linux, 128 bit
(16 byte) alignment of on-stack items assumes that the stack is 16 byte
aligned.

While the code generated by gcc for function calls maintains this
alignement, it isn't part of the standard ABI and other code may not
do so.
If you read up on this you'll find someone tried detecting misaligned
stacks and found they happened all the time.
Pertinent to wine is that code compiled with MSC will not maintain that
alignment, any assembler wrappers are unlikely to as well.

There are some options to gcc that affect the generated code, one will
force the stack to be realigned if any locals need 16 byte alignement.
Changing some of the other options will cause internal compiler faults!

The code in gcc is made more complicated by the fact that it is useful
to align 8 byte items, but never (AFAICT) necessary to do so, and gcc
tries to align on-stack double and long-long items but not realign the
stack in order to do so.

We've just gone through a series of hoops in NetBSD trying to fix
gcc to generate code that doesn't assume 16 byte aligned stack and
doesn't fault internally.

David

-- 
David Laight: da...@l8s.co.uk




Re: [PATCH 5/8] ntdll: Add detection for PF_SSE_DAZ_MODE_AVAILABLE

2012-10-21 Thread David Laight
On Sun, Oct 21, 2012 at 10:56:51AM -0600, James Eder wrote:
> static inline void save_fpux( CONTEXT *context )
> {
> #ifdef __GNUC__
> /* we have to enforce alignment by hand */
> char buffer[sizeof(XMM_SAVE_AREA32) + 16];
> XMM_SAVE_AREA32 *state = (XMM_SAVE_AREA32 *)(((ULONG_PTR)buffer +
> 15) & ~15);
> 
> __asm__ __volatile__( "fxsave %0" : "=m" (*state) );
> context->ContextFlags |= CONTEXT_EXTENDED_REGISTERS;
> memcpy( context->ExtendedRegisters, state, sizeof(*state) );
> #endif
> }

That is an on-stack buffer, all bets are off!

Traditionally the SYSV ABI (used by everyone except microsoft) only
required the stack to br 4 byte aligned.
At some point the gcc bods unilaterally decided to maintain 16 byte
alignement (so the SSE2 registers can be saved on stack).

The Linux kernel aligns the stack when main (strictly _start) is called,
but anything not compiled by gcc (or compiled with other options)
can lose that alignment.

If you request 32 byte alignment for the save area, (recent enough) gcc
will align the stack before allocating the locals.

David

-- 
David Laight: da...@l8s.co.uk




Re: [PATCH 5/8] ntdll: Add detection for PF_SSE_DAZ_MODE_AVAILABLE

2012-10-21 Thread David Laight
On Sun, Oct 21, 2012 at 06:06:56PM +0200, Joris Huizer wrote:
> I would think the construct is necessary when allocating memory (the 
> allocation functions don't allow to require a certain alignment as far 
> as I know)
> That might be where you saw this being done?

A conformant malloc() should return 16 byte aligned memory.
But I bet some i386 ones don't.
Especially since gcc is quite capable of suddently using the
SSE2 (etc) instructions that require such alignment.

    David

-- 
David Laight: da...@l8s.co.uk




Re: [PATCH 1/1] include/basetsd.h: fix bad casting

2012-10-13 Thread David Laight
On Sat, Oct 13, 2012 at 10:49:49AM +0900, Dmitry Timoshkov wrote:
> Max TenEyck Woodbury  wrote:
> 
> > >> -#define IntToPtr(i) ((void *)(INT_PTR)((INT)i))
> > >> -#define UIntToPtr(ui)   ((void *)(UINT_PTR)((UINT)ui))
> > >> -#define LongToPtr(l)((void *)(LONG_PTR)((LONG)l))
> > >> -#define ULongToPtr(ul)  ((void *)(ULONG_PTR)((ULONG)ul))
> > >> +#define IntToPtr(i) ((void *)(INT_PTR)(i))
> > >> +#define UIntToPtr(ui)   ((void *)(UINT_PTR)(ui))
> > >> +#define LongToPtr(l)((void *)(LONG_PTR)(l))
> > >> +#define ULongToPtr(ul)  ((void *)(ULONG_PTR)(ul))
> > > 
> > > You forgot to explain what's bad with it.
> > > 
> > 
> > The original applied the extra conversion to ONLY THE FIRST
> > component of the expression, not to the WHOLE expression.  That
> > can (although it's not likely to) cause hard to diagnose problems.
> 
> What 'the first component' are you talking about? This code is win32 only.

This example is a bit bogus but shows what he means:
with:
struct foo { int a[2] } bar;
expand:
IntToPtr(bar + 4)

I can't actually remember what all the perverted type names above
actually are! I had a feeling that INT_PTR had something do do with
an integer that is big enough to hold the value of a pointer rather
than being a pointer to an INT, but that means that having both
INT_PTR and LONG_PTR is silly.

Having an extra typedef for 'foo *' is really silly, amongst other 
things you can't convert it to 'const foo *' (only to 'foo * const').

David

-- 
David Laight: da...@l8s.co.uk




Re: [PATCH 10/25] mciseq: Limit concurrency when starting to play.

2012-10-10 Thread David Laight
On Wed, Oct 10, 2012 at 10:42:53AM +0200, joerg-cyril.hoe...@t-systems.com 
wrote:
> Hi,
> 
> David Laight wrote:
> 
> >Better to code as:
> >status = wmm->dwStatus;
> >if (...)
> Incidentally, that's what I did tonight in patch 20/25 try 2.
> 
> >but even then I think the compiler is allowed to perform extra reads.
> >It might, for example, do so if the condition was complicayted and
> >there was a lot of pressure on registers - so instead of spilling 'status'
> >to stack, it would re-read it.
> 
> Interesting. Do you think/believe or are you sure?

Probably in the realms of 'is allowed to' (under the general 'is if'
rule), but in practise won't.

> >With gcc you can force it to only to one read by adding:
> >asm volatile("":::"memory");
> >The only portable way is with volatile.
> "only portable way" from the C perspective. However, given a specific target
> environment, we could use its API functions.
> Either
> MemoryBarrier(); /* which MSDN documents but Wine does not provide in 
> include/*.h
> Or
> InterlockedExchange(&status, wmm->dwStatus);
> 
> So if AJ is still not satisfied with try 2, I'll change all reads of 
> wmm->dwStatus
> within the player into InterlockedExchange.
> Yet I think that would be a superfluous extra memory barrier within the 
> player.

That use of InterlockedExchange() is OTT.
As well as being a slow, locked bus cycle is will force a stack slot
for 'status', and the value be read back from there after anything
that might modify memory (eg a function call).

I can't remember the minumum requirement to stop the compiler doing
a reload. A call to an external function is more than enough.

There are also a lot of different tyoes of 'memory barrier' dunno which
sort MSDN's MemoryBarrier() generates.
On x86/amd64 you almost never need to request a barrier.

David

-- 
David Laight: da...@l8s.co.uk




Re: [PATCH 10/25] mciseq: Limit concurrency when starting to play.

2012-10-09 Thread David Laight
On Tue, Oct 09, 2012 at 04:05:09PM +0200, Alexandre Julliard wrote:
...
> >> Also there's no such thing as
> >>"conceptually volatile", unless you add explicit memory barriers.
> > Forget about "conceptually volatile".  I meant to express the idea
> > that some variable may be updated from another thread.  This has
> > nothing to do with the C keyword of that name.
> >
> > Once again I read some articles about memory barriers and fail to see
> > what's wrong with the final code (after patch 21/25):
> >
> > if (wmm->dwStatus == MCI_MODE_STOP || wmm->dwStatus == 
> > MCI_MODE_NOT_READY)
> > break;
> > else if (wmm->dwStatus != MCI_MODE_PLAY || recheck)
> > continue;
> 
> That depends on what state transitions are possible and what the
> behavior should be in various states. I haven't analyzed that in detail.

If the dwStatus field ISN'T volatile then the compile might generate
code for the above that reads it once, twice, or three times.

If we assume that reads of it are atomic (ie it is not larger than a
machine word, and is aligned) then there is no need to acquire any mutex
across the read - unless you are also checking another variable.

The above code almost certainly requires that only a single read be done.
This is likely to be the case if the code is optimised, and unlikely if not.
So in the above the repeated reads probably technically require a lock.

Better to code as:
status = wmm->dwStatus;
if (...)
but even then I think the compiler is allowed to perform extra reads.
It might, for example, do so if the condition was complicayted and 
there was a lot of pressure on registers - so instead of spilling 'status'
to stack, it would re-read it.

With gcc you can force it to only to one read by adding:
asm volatile("":::"memory");
before the first if.
The "memory" constraint of the asm statement tells gcc that it might
change any memory location - thus acting as a barrier.

The only portable way is with volatile.

David

-- 
David Laight: da...@l8s.co.uk




Re: Help getting amd64 assembly patch into wine?

2012-10-05 Thread David Laight
On Fri, Oct 05, 2012 at 04:27:24PM +0200, joerg-cyril.hoe...@t-systems.com 
wrote:
> So bad, now what is actually needed?
> 
> Think assembly.
> 
> Using your test example:
> +p_vcomp_fork(0, 5, _test_worker5, 1, 2, 3, 4, 5);
> 
> _vcomp_fork finds data:
> 1. on the stack
> 2. in registers
> 3. in FP registers
> The C stack layout is (topmost first):
>   0, then 5, &_test_worker5, 1, 2, etc.

That sort of stack layout is likely to be valid for i386, but not for amd64.
64-bit (amd64) windows uses a different calling convention from (almost)
every one else.
But I don't think you can convert a partially consumed va_list back into
an argument list (eg to delete an initail argument) without 'cheating'.

On linux (and probably windows) the first int/ptr args are passed in
integer registers and the first few FP args are passed in FP registers.
When the registers run out, values are stacked.

This means that these two calls are equivalent:
printf("int %d, fp %f\n", 2, 3.15159);
printf("int %d, fp %f\n", 3.15159, 2);
the va_arg() processing has to remember which registers have already
been processed in order to know where to find the next argument.

David

-- 
David Laight: da...@l8s.co.uk




Re: wintrust/tests: Fix build with MSVC.

2012-06-21 Thread David Laight
On Thu, Jun 21, 2012 at 12:56:43PM +0200, Thomas Faber wrote:
> On 2012-06-21 12:34, Dmitry Timoshkov wrote:
> 
> > Thomas Faber  wrote:
> > 
> >> FIELD_OFFSET isn't constant "enough".
> > 
> > The problem is not with FIELD_OFFSET, but with applying shift and mask
> > operations to a constant.
> 
> 
> You're right, it's actually a combination of multiple factors. I'm not sure
> what exactly its problem is - the gist is, FIELD_OFFSET cannot reliably be
> assumed constant.
> 
> E.g. the following code only complains about c4:
> 
> struct a {
> char a;
> int b[5];
> int c;
> };
> int c1 = (int)&(((struct a *)0)->a) << 8;
> int c2 = (int)&(((struct a *)0)->b[0]);
> int c3 = (int)&(((struct a *)0)->b[0]) + 1;
> int c4 = (int)&(((struct a *)0)->b[0]) << 8;

This is a bug in the microsoft C compiler.
It won't let you do multiply or divide offsetof(struct, array_member[index])
by anything and still get a compile-time constant.
The C language requires offsetof() be a compile-time constant, so this
ought to be allowed.
I couldn't find anything that looked like 'offsetof' that would generate
a real compile-time constant.

I can't remember whether it is 'constant enough' to let you do:
static char fubar[offsetof(struct a, b[3])];

David

-- 
David Laight: da...@l8s.co.uk




Re: [PATCH] sfnt2fnt: Fix broken .fon files on !x86 architectures

2012-06-15 Thread David Laight
On Fri, Jun 15, 2012 at 07:52:50PM +0200, Hilko Bengen wrote:
> +#define htole16(_x) ( (((_x) & 0xff) << 8) |\
> +  (((_x) & 0xff00) >> 8) )
> +#define htole32(_x) ( (((_x) & 0xff) << 24) |   \
> +  (((_x) & 0xff00) << 8) |  \
> +  (((_x) & 0xff) >> 8) |\
> +  (((_x) & 0xff00) >> 24) )

You shouldn't really define macros that expand their arguments
multiple times.

David

-- 
David Laight: da...@l8s.co.uk




Re: [PATCH 1/5] win16/int21: make found filename uppercase.

2012-05-30 Thread David Laight
On Wed, May 30, 2012 at 01:32:28PM +0200, Alexandre Julliard wrote:
> Oleksij Rempel  writes:
> 
> > @@ -3961,6 +3962,8 @@ static int INT21_FindNext( CONTEXT *context )
> >  INT21_FindPath = dta->fullPath = NULL;
> >  }
> >  dta->count = n;
> > +p = dta->filename;
> > +for ( ; *p; p++) *p = toupper(*p);
> 
> Using toupper is probably not a good idea on OEM codepage strings. Which
> app depends on this?

I'd also guess that 'p' is defined as 'char *'.
The domain of toupper() isn't char, the value must be cast to
'unsigned char' (or the pointer to 'unsigned char *').

David

-- 
David Laight: da...@l8s.co.uk




Re: running 16bit code

2012-05-08 Thread David Laight
On Tue, May 08, 2012 at 12:03:52PM +0200, Damjan Jovanovic wrote:
> On Tue, May 8, 2012 at 11:40 AM, David Laight  wrote:
> > Does wine support running of 16bit windows apps?
> > If so does it rely on the underlying OS having support
> > for 'virtual 8086 emulation'?
> >
> > I'm thinking of removing the VM86 support from NetBSD,
> > and wine is about the only likely user.
> >
> > ? ? ? ?David
> >
> > --
> > David Laight: da...@l8s.co.uk
> >
> >
> 
> 16 bit Windows applications are written and compiled to either use
> real mode or 16 bit protected mode.
> Those that use real mode (mostly MS-DOS and Windows 1 and 2
> applications) need virtual 8086 mode.
> Those that use protected mode (mostly Windows >= 3.0 applications)
> don't need virtual 8086 mode.

Ok, so wine users would be very unlikely to be affected.
Since most of the 16bit apps post-date windows 3.

> You'd break DOSBOX a lot more than Wine.

Possibly, although some emulators are probably better bets for DOS.

> But why do you want to remove
> this from NetBSD? I thought compatibility with other operating systems
> was one of its major features?

Mainly because it isn't used much, and may well contain security holes.
There are two separate kernel options VM86 (usually enabled) and
KVM86 (usually disabled).
Both change some low level code is obscure ways.

David

-- 
David Laight: da...@l8s.co.uk




running 16bit code

2012-05-08 Thread David Laight
Does wine support running of 16bit windows apps?
If so does it rely on the underlying OS having support
for 'virtual 8086 emulation'?

I'm thinking of removing the VM86 support from NetBSD,
and wine is about the only likley user.

David

-- 
David Laight: da...@l8s.co.uk




Re: [PATCH 1/6] msvcrt: Rewrite asctime function

2012-04-10 Thread David Laight
On Tue, Apr 10, 2012 at 03:09:52PM +0200, Alexandre Julliard wrote:
> Piotr Caban  writes:
> 
> > @@ -48,7 +49,7 @@ static const int MonthLengths[2][12] =
> >  
> >  static inline int IsLeapYear(int Year)
> >  {
> > -return Year % 4 == 0 && (Year % 100 != 0 || Year % 400 == 0);
> > +return (Year % 4 == 0 && (Year % 100 != 0 || Year % 400 == 0)) ? 1 : 0;
> 
> Maybe you should do something like:
> 
> return (((Year % 4 == 0 && (Year % 100 != 0 || Year % 400 == 0)) ? 1 : 0) 
> ? 1 : 0) ? 1 : 0;
> 
> to make really sure you get 0 or 1, you never know what might happen ;-)

But see also http://www.hermetic.ch/cal_stud/cal_art.html for some
info about which years were/are actually leap years ...

David

-- 
David Laight: da...@l8s.co.uk




Re: msxml3: Fix varargs handling in libxml2 error callback implementation

2012-02-16 Thread David Laight
On Thu, Feb 16, 2012 at 03:15:59AM +0300, Nikolay Sivov wrote:
> >If I remember correctly, you can even process a va_list only once
> >on some platforms.
> We use it that way in couple of places, so it seems to work and I can't 
> find a proper description or part of a standard that says it's not portable.

It definitely isn't portable.
The fact that it works sometimes is irrelevant.
 
> See winegcc/wrc for
> ---
> char* strmake(const char* fmt, ...)
> ---
> as an example.
> 
>  That probably means vsnprintf() and similar calls were added as part 
> of C99 as well, so their presence implies working va_copy() is available.

vsnprintf() was added to a lot of system much earlier than va_copy().
Last time I looked I couldn't find a va_copy() for the Microsoft C
compiler we were using at the time.
In my case it wasn't a printf, but a function that took a list of
pointers followed by a NULL - so I copied them into an on-stack array.

David

-- 
David Laight: da...@l8s.co.uk




Re: msxml3: Fix varargs handling in libxml2 error callback implementation

2012-02-15 Thread David Laight
On Wed, Feb 15, 2012 at 11:28:37PM +0100, Marcus Meissner wrote:
> On Thu, Feb 16, 2012 at 01:55:44AM +0300, Nikolay Sivov wrote:
> > The problem is that vsnprintf() was called multiple times with same
> > va_list. Ti fix that it was necessary to get rid of some tracing
> > bits like macro-defined callback calls and a single function for all
> > kinds of error types.
> > 
> > As far as I understand this problem it leads to a stack corruption
> > when va_list is used multiple time without va_start/va_end around
> > it, so it's critical to fix.
> 
> If I remember correctly, you can even process a va_list only once
> on some platforms.
> 
> If you need to process it multiple times, you need to create a copy
> with va_copy() first.
> 
> Ciao, Marcus

Correct - on architectures that don't pass all arguments on the stack
a va_list is a complex data item that can only be processed once.
The Microsoft ABI for amd64 reserves stack space for the arguments
passed in registers so that the processing of integer/ptr args is easy.
For all Unix OS amd64 passed the first 6 (IIRC) integer/ptr args
in normal registers, and the first few FP args in FP regs (regardless
of the order of the parameters), the va_list data has to remember
which register args have been processed.
This is all somewhat tricky! and makes support for printf's argument
order selection stuff extremely difficult to write!

David

-- 
David Laight: da...@l8s.co.uk




Re: riched20: explicitly cast WPARAM to character types

2011-10-23 Thread David Laight
On Sun, Oct 23, 2011 at 09:25:26PM +0200, Michael Stefaniuc wrote:
> 
> No clue anyway why MSVC even warns, integer to integer conversions are
> allowed. ...

Generally better to turn off MSVC's over-pedantic warnings about integer
conversions. Using casts for integer convertions can actually hide
other issues (like accidentally casting a pointer type).

    David

-- 
David Laight: da...@l8s.co.uk




Re: [PATCH] server: Use syscall(2) instead of inline assembly on Mac OS, too.

2011-10-09 Thread David Laight
On Sat, Oct 08, 2011 at 07:48:40PM +0200, Alexandre Julliard wrote:
> Charles Davis  writes:
> 
> > @@ -268,9 +256,9 @@ int send_thread_signal( struct thread *thread, int sig )
> >  if (!mach_port_extract_right( process_port, thread->unix_tid,
> >MACH_MSG_TYPE_COPY_SEND, &port, 
> > &type ))
> >  {
> > -if ((ret = pthread_kill_syscall( port, sig )) < 0)
> > +if ((ret = syscall( SYS___pthread_kill, port, sig )) != 0)
> >  {
> > -errno = -ret;
> > +errno = ret;
> 
> syscall is supposed to take care of errno.

I'm also not at all sure of the portability of using syscall() in
application code.

What happens when you try to run the above on anything other than Linux?
(Eg aone of the BSDs)

David

-- 
David Laight: da...@l8s.co.uk




Re: [PATCH] server: if a debugger is attached to a process, child processes shouldn't get debugged (resend)

2011-10-08 Thread David Laight
On Fri, Oct 07, 2011 at 08:32:27PM +0200, Marcus Meissner wrote:
> On Fri, Oct 07, 2011 at 11:44:24AM +0900, Dmitry Timoshkov wrote:
> > Bernhard Loos  wrote:
> > 
> > > +int  debug_childs:1;  /* also debug all child 
> > > processes */
> > 
> > 'debug_children' would be a better name.
> 
> also
>   unsigned intfoo:1;
> please. (int foo:1 works, but is semidefined only ;)

Why the bitfield anyway?
Unless you are allocating a lot of copies of the structure
it is likely to generate more code than the saved memory.

David

-- 
David Laight: da...@l8s.co.uk




Re: d3dx9_36/tests: Fix printing a NULL string

2011-08-31 Thread David Laight
On Wed, Aug 31, 2011 at 05:18:57PM +0200, joerg-cyril.hoe...@t-systems.com 
wrote:
> Bruno Haible was so kind to provide the following test run results:
> 
> glibc  SIGSEGV
> MacOS X 10.5   (null)
> FreeBSD 6.4(null)
> OpenBSD 4.9(null)
> AIX 7.1empty
> HP-UX 11.31empty
> IRIX 6.5   (null)
> OSF/1 5.1  (null)
> Solaris 10 SIGSEGV
> Cygwin (null)
> mingw  (null)
> 
> for printf("%s", NULL)

Some of the systems will have address zero valid and just read from
that address. Some will generate a non zero length string.
IIRC some very old OS (think vax and pdp11) arranged for address
zero to contain zero (possibly as part of the program header) so
that 'if (x && *x)' could be shortened to 'if (*x)'.
Allowing address zero be mapped (eg by mmap()) is a bad idea
as a kernel 'call through NULL ptr' can be used to escalate
privs.

David

-- 
David Laight: da...@l8s.co.uk




Re: Where is the best place to report a fscanf bug found under wine-1.3.27?

2011-08-30 Thread David Laight
On Mon, Aug 29, 2011 at 06:43:41PM -0700, Alan W. Irwin wrote:
>   double x;
>   while(fscanf(stdin, " %le ", &x) == 1)

You are using the wrong format, %le is for 'long double', this will
probably overwrite too much data.

    David

-- 
David Laight: da...@l8s.co.uk




Re: d3dx9_36/tests: Fix printing a NULL string

2011-08-30 Thread David Laight
On Tue, Aug 30, 2011 at 12:44:05PM +0200, joerg-cyril.hoe...@t-systems.com 
wrote:
> Wine logs in Solaris can randomly crash Wine because it crashes on 
> printf("%s", NULL)?
> 
> I'm very surprised. I thought Solaris was one of the first machines - decades 
> ago - where I observed "(null)" for NULL.
> (Or is that really glibc only?)

Probably glibc, solaris was one of the first unix (I saw) to fault
on accesses to address zero and certainly the current solaris libc
will fault.

David

-- 
David Laight: da...@l8s.co.uk




Re: tools/winegcc: support a trailing / in paths to winebuild

2011-08-23 Thread David Laight
On Mon, Aug 22, 2011 at 07:13:42PM +0200, Bernhard Loos wrote:
> On Mon, Aug 22, 2011 at 7:02 PM, Jerome Leclanche  wrote:
> > That's to be fixed in bash-completion, not winegcc...
> 
> Uhm, what's wrong with bash-completation? -B takes a directory as an
> argument so bash adds a slash during autocomplete, that's ok.
> gcc itself can also deal with a trailing / for -B, it's kinda
> suprising, if this breaks in winegcc.

bash has 'broken' many things by doing that.
Traditionally directory paths wouldn't be expected to have a trailing '/'.

David

-- 
David Laight: da...@l8s.co.uk




Re: GSoC-2011: Implement Missing Mesh Functions in Wine?s D3DX9

2011-08-04 Thread David Laight
On Thu, Aug 04, 2011 at 08:30:41AM -0400, max wrote:
> On 08/03/2011 09:28 AM, Henri Verbeet wrote:
> >Well yes, it's implementation defined, not undefined. The point is
> >that there isn't necessarily any relation to endianness. Just use
> >shifts and masks.
...
> union field_order { int an_int; int a_bit:1 } test;
> test.an_int=0;
> test.a_bit=1;
...

There are 4 likely values for test.an_int (1, 0x80, 0x8000, 0x0100),
and all might be generated regardless of the system endianness.

David

-- 
David Laight: da...@l8s.co.uk




Re: Wine and endianness

2011-08-01 Thread David Laight
On Mon, Aug 01, 2011 at 08:31:50AM +0200, GOUJON Alexandre wrote:
> 
> Sorry to disturb your conversation but the subject is worth discussing.
> 
> I'm currently trying to add UDF support on wine based on Steven Wallace 
> work.
> Quoting the specification : "On the media the UDF structures are stored 
> little endian" as windows API.
> But wine is not limited on x86 so what's the rule ?
> Currently, my code is
> 
> int i = 1;
> char *p = (char *)&i;
> BOOL isBigEndian = p[0]==1;
> 
> /* Tag location (Uint32) at offset 12, little-endian */
> *offs  = (bloc[20+0] & 0xFF) << ( isBigEndian ?  0 : 24);
> *offs += (bloc[20+1] & 0xFF) << ( isBigEndian ?  8 : 16);
> *offs += (bloc[20+2] & 0xFF) << ( isBigEndian ? 16 :  8);
> *offs += (bloc[20+3] & 0xFF) << ( isBigEndian ? 24 :  0);
> 
> Is it correct ?

Depends what the data you are playing with is
But it looks a bit dubious to me.

> Any thoughts ?

1) You want endianness to be a compile time constant, not run time.
2) If that code is trying to read a 32bit LE value from a disk sector
   it is wrong.
3) If you care how fast the code runs, don't repeatedly write through
   a pointer.

If you have 'unsigned char bloc[]' and want to read a 32 bit LE value
you can do:
value = bloc[20 + 0];
value |= bloc[20 + 1] << 8;
value |= bloc[20 + 2] << 16;
value |= bloc[20 + 3] << 24;
*offs = value;
And that is correct on all architectures.

To write a LE value use:
buf[0] = value;
buf[1] = value >>= 8;
buf[2] = value >>= 8;
buf[3] = value >>= 8;

(For BE reverse the indexes)

If the item is known to be correctly aligned (or the architecture supports
mis-aligned reads) then you can just do (eg):
value = *(uint32_t *)(bloc + 20);
value = le32toh(value);
but you'll have to chase around the OS headers to find how to spell le32toh().
If you define a C struct that matches the data area (packed if it might
have misaligned data), the compiler will generate the shifts and masks to
read a native word - so you only need the le32toh() macro.
(le32toh() is from NetBSD, can't remember what Linux calls it!)

It is also worth noting that some architectures (eg ppc) have instructions
for reading and writing memory with byteswap, but no instruction for
swapping a register. Newer gccs can be taught how to use these instructions
for specific source sequences.

David

-- 
David Laight: da...@l8s.co.uk




Re: [PATCH 2/2] ntdll: Set restart_scan on first call to NtQueryDirectoryFile.

2011-07-20 Thread David Laight
On Wed, Jul 20, 2011 at 07:29:13PM +0200, Alexandre Julliard wrote:
> Grazvydas Ignotas  writes:
> 
> > Here is another idea:
> >
> > It seems getdents64 is indeed returning "." and "..", but not
> > necessarily first, like Windows does. I think we could swap first 2
> > entries returned, whatever they are, with "." and "..". It would work
> > like this:
> >
> > - read the 2 first entries to advance file pointer, but ignore them,
> > return "." or ".." instead
> > - return other entries normally
> > - after "." or ".." is encountered, save current file offset and seek
> > to first or second entry (respectively) and return that instead. For a
> > corner case where that spot has ".." or ".", return the other one of
> > first 2.
> > - seek back where we were
> >
> > I think this would also fix fake_dot_dot + single_entry case. What do you 
> > think?
> 
> That could probably be made to work, yes.

Remember that you can only seek to the offsets returned by the OS.
Directory offsets aren't necessarily byte offsets.
(They may be 64bit values even for small directories.)

David

-- 
David Laight: da...@l8s.co.uk




Re: gcc 4.6 warning report

2011-06-29 Thread David Laight
On Tue, Jun 28, 2011 at 03:18:47PM -0700, Juan Lang wrote:
> >> This is a false positive. ?h_addr_list is declared as a char **, and
> >> technically it is, but gethostbyname only returns IPv4 addresses, i.e.
> >> ones that can fit in a DWORD.
> >
> > That doesn't sound like a problem that would give that warning.
> 
> Why not?  A cast of a pointer to a DWORD with 64-bit pointers would
> cause truncation, if it were indeed a pointer that were being cast.
> It isn't, even though it's declared that way.

But 'char **h_addr_list;' is a pointer to an array of pointers
to address buffers (defined as char) - not a pointer to an array of addresses.
IIRC the 'something' will be an IPv4 address - which might
be held as a 32 bit quantity rather that an array of char.
But nowhere should you cast the 32bit int (DWORD) to/from a pointer type.

David

-- 
David Laight: da...@l8s.co.uk




Re: gcc 4.6 warning report

2011-06-28 Thread David Laight
On Tue, Jun 28, 2011 at 12:51:02PM -0700, Juan Lang wrote:
> ../../../dlls/netapi32/nbt.c:580:30: warning: cast from pointer to
> integer of different size [-Wpointer-to-int-cast]
> 
> This is a false positive.  h_addr_list is declared as a char **, and
> technically it is, but gethostbyname only returns IPv4 addresses, i.e.
> ones that can fit in a DWORD.

That doesn't sound like a problem that would give that warning.

    David

-- 
David Laight: da...@l8s.co.uk




Re: [PATCH] comctl32: Fixed strncpy (Coverity)

2011-06-28 Thread David Laight
On Tue, Jun 28, 2011 at 09:37:39AM +0200, Michael Stefaniuc wrote:
> Hello Marcus,
> 
> On 06/28/2011 09:15 AM, Marcus Meissner wrote:
> > This is a 2 CHAR buffer, and we copy in 1 CHAR and \0. So just do that
> > via strcpy() and dont confuse the static checker.
...
> > -strncpy(buff, "x", sizeof(buff)/sizeof(CHAR));
> > +strcpy(buff, "x");

What is wrong with:
buff[0] = 'x';
buff[1] = 0;

David

-- 
David Laight: da...@l8s.co.uk




Re: [PATCH 1/4] Check for null pointers before strcmp. (LLVM/Clang)

2011-06-11 Thread David Laight
On Sat, Jun 11, 2011 at 01:15:42AM +0300, Lauri Kentt? wrote:
> 
> >Second, the ok output message also dereferences pdst by printing it
> 
> That's not true. "%s" formats null pointers as "(null)" instead of 
> dereferencing anything.

That isn't a requirement of the C language, there are many implemenations
which will fault.

David

-- 
David Laight: da...@l8s.co.uk




Re: comctl32/tests: Using unsigned constants to remove sign comparison warning.

2011-05-27 Thread David Laight
On Fri, May 27, 2011 at 10:09:04PM +0400, Nikolay Sivov wrote:
> On Fri, May 27, 2011 at 5:16 PM, Marko Nikolic  wrote:
> > ---
> > ?dlls/comctl32/tests/treeview.c | ? ?6 +++---
> > ?1 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/dlls/comctl32/tests/treeview.c b/dlls/comctl32/tests/treeview.c
> > index 5fd26a0..b1eb85e 100644
> > --- a/dlls/comctl32/tests/treeview.c
> > +++ b/dlls/comctl32/tests/treeview.c
> > @@ -634,7 +634,7 @@ static void test_get_set_bkcolor(void)
> >
> > ? ? /* If the value is -1, the control is using the system color for the 
> > background color. */
> > ? ? crColor = (COLORREF)SendMessage( hTree, TVM_GETBKCOLOR, 0, 0 );
> > - ? ?ok(crColor == -1, "Default background color reported as 0x%.8x\n", 
> > crColor);
> > + ? ?ok(crColor == 0x, "Default background color reported as 
> > 0x%.8x\n", crColor);
> >
> 
> ~0 looks better here imo.

Rather ~0u
The 0x should be 0xu

David

-- 
David Laight: da...@l8s.co.uk




Re: [PATCH 2/5] d3dx9_36: Implemented ID3DXConstantTable_SetIntArray and ID3DXConstantTable_SetInt (Try 5)

2011-03-28 Thread David Laight
> 
> Ints and floats are represented differently and so, if we want to put
> an integer array into floating point registers, we need to have casts.
> If you were to simply call SetFloatArray, you would get nonsense
> values in the floating point registers.

Even using casts will fall foul of the C aliasing rules - which
mean that the compiler can assume that a read of type will not be
modified by a write to a different type so can be reorderd (unless 
one of the types is 'char').

Not only that, but there are systems that will fault is an invalid
FP bit pattern is loaded into an FP register.

    David

-- 
David Laight: da...@l8s.co.uk




Re: [GSoC] Merge winecfg and Control Panel

2011-03-23 Thread David Laight
On Wed, Mar 23, 2011 at 01:31:09PM -0700, Scott Ritchie wrote:
> On 03/23/2011 07:15 AM, Dan Kegel wrote:
> > Also... it would be nice if there were a central commandline
> > way of changing wine settings, kind of like 'net' is a central
> > way of manipulating networking.
> > 
> > I've been thinking that winecfg should have a commandline
> > interface, too, and making it accept winetricks settings
> > directly.Something like that would also make a good soc project.
> > 
> > 
> 
> I remember proposing this idea long ago as I would have found it useful
> for front end user interfaces.

It is worth remembering that an elf program's 'main' can be in a
shared object! This mightmake it easier to write a stub binary
that effectively make a .so executable.

Sort of depends on what the guy way tring to do...

David

-- 
David Laight: da...@l8s.co.uk




Re: wineport: Add support for ctz().

2011-03-17 Thread David Laight
On Wed, Mar 16, 2011 at 01:26:31PM -0500, Adam Martinson wrote:
> 
> __builtin_ctz() compiles to:
> mov0x8(%ebp),%eax
> bsf%eax,%eax
> 
> (ffs()-1) compiles to:
> mov$0x,%edx
> bsf0x8(%ebp),%eax
> cmove  %edx,%eax
...
> 
> So yes, there is a reason, ctz() is at least 50% faster.

I'm not where you get 50% from!

I've read both the intel and amd x86 instruction performance manuals
(but can't clain to remember all of it!).
The 'bsf' will be a slow instruction (with constraints on where it
exectutes, and what can execute in parallel).
The 'cmove' has even worse constraints since it can't execute until
the 'flags' from the previous instruction are known.
cmove is only slightly better than a mis-predicted branch!
In this case there will be complete pipeline stall between the 'bsf'
and the 'cmove'.

ffs would probably execute faster with a forwards conditional branch 
(predicted not taken) in the 'return -1' path.

David

-- 
David Laight: da...@l8s.co.uk




Re: wineport: Use a lookup table to make our slow ffs() a bit faster.

2011-03-11 Thread David Laight
On Fri, Mar 11, 2011 at 05:51:27AM -0800, Dan Kegel wrote:
> 
> Large lookup tables can dirty the L1 cache and slow other things down.

And even small tables can also be a slow data cache miss in the
critical path.
Of course, when benchmarking, the table is all in the cache!
In real life it is likely to be absent (unless you are doing
a lot of lookups together).

David

-- 
David Laight: da...@l8s.co.uk




Re: wineport: Add fast builtin & asm versions of ffs() + ctz() where supported.

2011-03-11 Thread David Laight
On Fri, Mar 11, 2011 at 02:28:27PM +0100, Alexandre Julliard wrote:
> Adam Martinson  writes:
> 
> > @@ -236,7 +241,40 @@ extern int getopt_long_only (int ___argc, char *const 
> > *___argv,
> >  #endif  /* HAVE_GETOPT_LONG */
> >  
> >  #ifndef HAVE_FFS
> > -int ffs( int x );
> > +#if defined(__i386__) || defined(__x86_64__)
...
> > +__asm__("bsfl %1, %0; incl %0" : "=r" (ret) : "r" (x));
> > +return ret;
...
> > +#elif defined(__GNUC__) && GCC_VERSION >= 29503
> > +#define HAVE_FFS
> > +#define ffs(x) __builtin_ffs(x)
> > +#else
> > +int ffs( int x );
> > +#endif
> > +#endif
> 
> You'd have to show benchmarks to prove that this complexity is
> necessary. Given that ffs() should already be inlined on all decent
> platforms, I doubt you'd be able to demonstrate a difference (if
> anything, your version would be slower because of the extra increment).

Never mind the fact that it is possible to write code that is (probably)
faster than the bsfl instruction on most x86 processors!
You'd also get a gain from getting the 'if (x == 0)' statically predicted
correctly (probably for not zero) - that is also likely to be large.

David

-- 
David Laight: da...@l8s.co.uk




Re: d3dx9: Implement D3DXFloat16to32Array and D3DXFloat32to16Array.

2011-02-11 Thread David Laight
On Fri, Feb 11, 2011 at 11:16:55PM +0100, Stefan D?singer wrote:
>  Its just that this function goes to great lengths to 
> make sure it doesn't rely on the actual encoding that it is annoying that we 
> have to give it up for detecting the sign of zeroes.

In that case why not?

int
is_neq_z(double x)
{
union {
char i[sizeof (double)];
double d;
} ux, u0;
int i;

if (x != 0.0)
return 0;
u0.d = 0.0;
ux.d = x;
for (i = 0; i < sizeof (double); i++) {
if (ux.i[i] != u0.i[i])
return 1;
}
return 0;
}

Which works provided that there are only 2 encoding patterns for zero.

    David

-- 
David Laight: da...@l8s.co.uk




Re: d3dx9: Implement D3DXFloat16to32Array and D3DXFloat32to16Array.

2011-02-11 Thread David Laight
On Fri, Feb 11, 2011 at 10:20:10PM +0100, Stefan D?singer wrote:
> Am Freitag 11 Februar 2011, 20:48:58 schrieb Misha Koshelev:
> 
> > +if (*((unsigned int *)&in) == 0x) return 0x;
> > +if (*((unsigned int *)&in) == 0x8000) return 0x8000;
> Thinking about it, there's something about this line that is not so nice: It 
> relies on the actual encoding of the float, which may technically be platform 
> specific if there's a CPU that implements non-IEEE- 754 floats.

The 'usual' non-IEEE fp systems are vax (you probably can't put enough
memory in a vax to run wine!) and some arm systems where the two 32bit
words of a double aren't stored in the expected order!

Some embedded systems might not bother with denormalised encodings
for values near zero. I don't know if anything uses an abolute offset
to avoid that gap (the main difference between aLaw anf uLaw audio).

It also requires that the compiler use stricter data aliasing rules
than are required by the C standard.
Using a union is ok (but you must write and read a union variable,
not just cast to a union type.

David

-- 
David Laight: da...@l8s.co.uk




Re: d3dx9: Implement D3DXFloat16to32Array and D3DXFloat32to16Array.

2011-02-09 Thread David Laight
On Tue, Feb 08, 2011 at 01:02:36PM +0800, Dmitry Timoshkov wrote:
> > +static inline float float_16_to_32(const unsigned short *in) {
> 
> ...  Also placing a brace on its own line would follow the style
> of other functions.

The brace goes in column 1 so that [[ and ]] work in vi.
Putting the function name in column 1 lets you search for the
definition using the anchored regexp ^function_name\>

Putting { and } on the same lines as for, while and else avoids problems
like determining whether this is intended to be the bottom or top of a loop:
bar = 1;
}
while (very long conditional expression that is truncated by the ...
{
int foo;

and whether there is an else on the line you can't see below the bottom
of the window when you can see:
if (...)
{
...
}
The 'else' problem is much worse if you are looking at a printout
on fanfold paper,

David

-- 
David Laight: da...@l8s.co.uk




Re: Wine FAQ edits

2011-02-06 Thread David Laight
On Sat, Feb 05, 2011 at 09:49:30PM -0500, Albert Lee wrote:
> 
> Anyone using the original Bourne sh *interactively* is in for a world
> of pain anyway (speaking as someone who often had to before the ksh93
> replacement). I think relying on tilde expansion can can be considered
> good practice for the interactive use case.

I wouldn't rely on ~ expansion - not unless you detect that it
has returned empty string.
Although anyone using a system that doesn't support it (most likely
Solaris's /bin/sh) will probably know how to avoid the problem, not
knowing what is wrong is more problematical.

Adding a few 'sh' conditional substitutions to a script might show
some people just how powerful the language is - especially if they
have onely ever seen windows cmd.com .bat scripts.

I've seen some recent systems fail to expand ~ properly (for some
definition of the word 'properly') most noticably cygwin on windoes 7.

David

-- 
David Laight: da...@l8s.co.uk




Re: Wine FAQ edits

2011-02-05 Thread David Laight
On Sat, Feb 05, 2011 at 10:14:07AM +0200, Gert van den Berg wrote:
> On Sat, Feb 5, 2011 at 10:05, Gert van den Berg  wrote:
> > On Thu, Feb 3, 2011 at 18:17, Albert Lee  wrote:
> >> The instructions were intended to give some context for users who
> >> might not be familiar with the ~ expansion.
> >
> > Using {$HOME} would be more portable.
> 
> And using "${WINEPREFIX:-${HOME}/.wine}/dosdevices/c:/Program Files"
> should work correctly in any Wine setup (on a Bourne-compatible
> shell), including one using a custom WINEPREFIX. 

Better still "${WINEPREFIX:-${HOME:?}/.wine}/dosdevices/c:/Program Files"
which will generate an error if HOME isn't set.

David

-- 
David Laight: da...@l8s.co.uk




Re: Wine FAQ edits

2011-02-03 Thread David Laight
On Thu, Feb 03, 2011 at 11:17:39AM -0500, Albert Lee wrote:
> 
> >
> >> ?It's exec(2)'ed so it doesn't matter what the program is.
> >
> > Basically run in a shell. So you need an extra wrapper (env) why?
> 
> To handle PATH search, since Wine requires an absolute path. This is a
> common trick used for locating interpreters for e.g. Python as well.

It does depend on 'env' being installed in a standard place!
It is also rather a side effect of env's real purpose - sanitising
the environment.

David

-- 
David Laight: da...@l8s.co.uk




Re: 64-bit Notepad2 crashes

2011-01-01 Thread David Laight
On Fri, Dec 31, 2010 at 06:14:52PM -0700, James McKenzie wrote:
> 
> set | grep LIBRARY
> LD_LIBRARY_PATH=/Applications/Wine.app/Contents/Resources/Lib
> 
> is what I get on my Mac (LD_LIBRARY_PATH has to be set due to the 
> UNIXness of Wine.)

Really a properly linked elf binary shouldn't need LD_LIBRARY_PATH
be set in order to find its libraries.
LD_LIBRARY_PATH should only be used to override the default assignments.
(Linux has a complete fubar here, and you have to do something obscure
to get LD_LIBRARY_PATH to override the info in the elf image.)

If you run 'objdump -p program' the RPATH entries show which which
directories are searched.
Additionally $ORIGIN can be used (in RPATH) entries to refer to the
directory that contains the program binary.

The RPATH entries typically need to be different for 32bit and 64bit
binaries on a 64bit system.

The library names are shown in the NEEDED entries (the value comes from
the soname property of the library found when linking the program.
This is usually just foo.so, but can contain directory names.

David

-- 
David Laight: da...@l8s.co.uk




Re: [PATCH 10/14] [Msvcrt*]: implemented _vc(w)printf

2010-11-08 Thread David Laight
On Wed, Nov 03, 2010 at 10:53:55AM +0100, Alexandre Julliard wrote:
> Eric Pouech  writes:
> 
> > +int CDECL _vcprintf(const char* format, __ms_va_list xvalist)
> >  {
> >char buf[2048], *mem = buf;
> >int written, resize = sizeof(buf), retval;
> > -  __ms_va_list valist;
> > +  __ms_va_list  valist = xvalist;
> 
> You can't copy a valist. This needs to be integrated properly with the
> other printf functions.

Well, you can if you can find a va_copy() function .

David

-- 
David Laight: da...@l8s.co.uk




Re: windowscodecs: Fix *_CopyPixels functions to properly handle a NULL rectangle

2010-10-18 Thread David Laight
On Mon, Oct 18, 2010 at 08:51:49PM +0200, Krzysztof Nowicki wrote:
> W dniu 17.10.2010 18:49, Vitaliy Margolen pisze:
> >On 10/17/2010 01:59 AM, Krzysztof Nowicki wrote:
> >>Doing a memcpy to a local rectangle seems a morenatural way to do it
> >
> >Not really. Doing RECT = RECT is the natural way to do it. Don't use
> >memcpy to copy one structure to another structure of the same type.
> >
> >Vitaliy.
> >
> 
> Where I come from copying structures directly is considered bad practice 
> and it's safer to use memcpy. We had problems before with broken 
> compilers that would try to do some black magic in such cases. So the 
> policy was to use memcpy for structures because it's safer. A good 
> compiler will inline such a memcpy anyway.

Actually, for a normal application, the compiler knows what memcpy()
does and can make assumptions based on the actual structure type
involved. This can cause extreme grief if you try to copy from a
misaligned pointer into a local variable (strictly, in valid C, you
can never get a misaligned pointer - and the compiler will use
word accesses).

David

-- 
David Laight: da...@l8s.co.uk




Re: msxml3: IXMLDOMText_get_text return trimmed text when XML_TEXT_NODE

2010-09-05 Thread David Laight
On Thu, Sep 02, 2010 at 12:10:26PM +0200, Alexandre Julliard wrote:
> Alexandre Goujon  writes:
> 
> > +static inline BSTR bstr_trim(const BSTR str)
> > +{
...
> > +}
> 
> That's ugly and inefficient, especially since the common case is to not
> modify it.

Not to mention the bloatage of using 'inline' for something that size.

David

-- 
David Laight: da...@l8s.co.uk




Re: [PATCH] winealsa.drv: Count micelem in mixer chans, don't add spare capture input for half-duplex mics.

2010-09-05 Thread David Laight
On Wed, Sep 01, 2010 at 05:40:35PM +0200, Alexandre Julliard wrote:
> Jeff Cook  writes:
> 
> > @@ -520,7 +520,12 @@ static void ALSA_MixerInit(void)
> >  }
> >  
> >  /* Add master channel, uncounted channels and an extra for capture 
> >  */
> > -mixdev[mixnum].chans += !!mastelem + !!headelem + !!pcmelem + 1;
> > +/* Do not add the extra channel needed for capture on half-duplex 
> > capture cards
> > +   like snd_usb_audio mics */
> > +if (micelem && !mastelem && !captelem)
> > +mixdev[mixnum].chans += !!mastelem + !!headelem + !!pcmelem + 
> > !!micelem;
> > +else
> > +mixdev[mixnum].chans += !!mastelem + !!headelem + !!pcmelem + 
> > !!micelem + 1;
> 
> This statement wasn't exactly clear before, but now it's really
> impossible to follow. Please rewrite this in a way that makes sense to a
> casual reader.

Clearly it should be:
mixdev[mixnum].chans += !!mastelem + !!headelem + !!pcmelem + (micelem 
&& !mastelem && !captelem);
ducks quickly ... :-)

David

-- 
David Laight: da...@l8s.co.uk




Re: msvcp90: Fix 64-bit compilation warnings

2010-09-05 Thread David Laight
On Wed, Sep 01, 2010 at 11:56:40AM +0200, Alexandre Julliard wrote:
> Piotr Caban  writes:
> 
> > @@ -596,7 +596,7 @@ DEFINE_THISCALL_WRAPPER(MSVCP_basic_string_char_erase, 
> > 12)
> >  basic_string_char* __thiscall MSVCP_basic_string_char_erase(
> >  basic_string_char *this, size_t pos, size_t len)
> >  {
> > -TRACE("%p %d %d\n", this, pos, len);
> > +TRACE("%p %d %d\n", this, (int)pos, (int)len);
> 
> You should be casting to long, otherwise you'll potentially truncate the
> value.

In which case the format specifiers need to be %ld as well...

David

-- 
David Laight: da...@l8s.co.uk




Re: Wine64 debugger

2010-08-17 Thread David Laight
On Tue, Aug 17, 2010 at 05:51:55PM +0200, Marcus Meissner wrote:
> 
> The Wine code is correct as-is, but perhaps the Windows win64 code generates
> only handles with 32bit.

That it very likely.
There are windows #defines/inline functions that convert HANDLE <=> long.
Remember there are some functions that return handles as long (not void *)

Also, when the windows kernel generates a HANDLE it is unlikely to know
whether the app is 32bit or 64bit - so will only generate 32bit values.

I also remember reading somewhere that the kernel handle table is
limited to 2^24 entries - even in 64bit mode.

David

-- 
David Laight: da...@l8s.co.uk




Re: Understanding 64bit implications and wine64

2010-02-09 Thread David Laight
On Mon, Feb 08, 2010 at 11:27:07AM +0100, joerg-cyril.hoe...@t-systems.com 
wrote:
> The wine64 challenge seems to let 32bit MS-windows apps work even
> though the UNIX side has to manage pointer addresses >2^32, i.e. above
> the lower 4GB address range.  Is there some address translation
> involved or only mmap'ing?
> What must the Wine developer know to write correct code for wine64?

A 32bit (i386) windows application binary can only run in a 32bit
Unix application [1].  In which case the Unix kernel will handle the
system call emulation and ensure that the only user-space virtual
addresses the application sees are 32bit (it may be able to give
the app almost 4G of user address space - instead of the usual 3G).

Wine may allow both 32bit and 64bit clients to talk to its server
process - and that may need care so that only fixed-size types are used.

David

[1] The instruction sets are different - the single byte opcodes for
inc/dec register are reallocated for other uses. So you cannot just
load 32bit code into a 64bit binary and expect anything sane to happen.

-- 
David Laight: da...@l8s.co.uk




Re: Wine on OpenBSD, Theo de Raadt

2009-11-05 Thread David Laight
On Thu, Nov 05, 2009 at 02:21:47PM +0100, Yann Droneaud wrote:
> And OpenBSD won't allow page 0 mapping for security reasons (and
> performance reasons).
> 
> Since Wine seems to be working on other *BSD and derivative systems,
> is this a problem ?

NetBSD is likely to disable user space mapping of address 0 some time
in the near future - it does have problems with the compatibility code
for other unix system binariess though (I can't remember which ones,
compat-ultrix might be the one).

    David

-- 
David Laight: da...@l8s.co.uk




Re: Please write audio tests (was: Playing ULAW sample correctly?)

2009-11-03 Thread David Laight
On Tue, Nov 03, 2009 at 07:48:29PM +0200, Damjan Jovanovic wrote:
> On Tue, Nov 3, 2009 at 3:33 PM,   wrote:
> >
> > Wine would benefit from a test in msacm32/tests/ sketched as follows:
> > 0. choose some base frequency like 11025Hz;
> > 1. generate a PCM sine wave tone (like winmm/tests/wave.c);
> > 2. convert it to ?-law via the ACM functions;
> > 3. play it via winmm if wine_interactive is set (like in wave.c)
> > 4. convert it back and apply some delta function to tell if it's
> > ? close enough to the original sine,
> > ? i.e. mechanically verify that the round-trip worked.
> 
> You'd need to verify the energy in the discrete Fourier transform of
> the signal at the frequency you're testing for, is say 90% of the
> total signal energy.

Or use a goertzel detector for the transmitted tone(s).
It is also quite simple to generate sine waves without any trig.

David

-- 
David Laight: da...@l8s.co.uk




Re: cppcheck sept 18 redux

2009-09-22 Thread David Laight
On Tue, Sep 22, 2009 at 02:56:05AM -0400, Mike Kaplinskiy wrote:
> On Tue, Sep 22, 2009 at 1:09 AM, Vitaliy Margolen
>  wrote:
> > Ben Klein wrote:
> >> The question remains, how exactly does
> >> FIELD_OFFSET work, and does it end up dereferencing ca[5]?
> > It does pointer arithmetic and does not dereference anything. "ca[5]" is the
> > same as "(ca + 5)" or on lower level "((char*)ca + 5*sizeof(ca[0]))" and
> > does not require any dereferencing.
> 
> It does, since field offset macro takes the easy approach:
> #define FIELD_OFFSET(type, field) ((LONG)(INT_PTR)&(((type *)0)->field))
> 
> which basically dereferences a null pointer to get the offset. This
> would be a bug in cppcheck since we don't actually dereference ca[5].
> Moreover, since cppcheck doesn't catch the similar FIELD_OFFSET uses
> as bugs, it seems that it is mistaking ca[5] for the local ca, as
> opposed to the cs_t->ca.

I suspect that the above is technically illegal C.
Mainly because pointer arithmetic is only defined for pointers to
objects - and no object can have the address 0.

This is why offsetof() is defined as a builtin in more recent GCC.

The case in question tries to evaluate:
((LONG)(INT_PTR)&(((type *)0)->ca[5]))
which does seem to contain a reference to beyond the end of the array!

Modern C allows the last entry of a struct to be ca[] (ie no array bound)
for these situations where a structure will be malloced with dynamic size.

There is, however, a fubar in the standard. offsetof() is defined to
return a compile-time constant - so the result can be used as an array
size etc.  However there are times when you want to do:
offsetof(type, array_member[expression])
and this now reported as illegal by gcc unless 'espression' is a compile
time constant :-(

David

-- 
David Laight: da...@l8s.co.uk




Re: [PATCH] [Msvcrt]: fixing errno handling in strtol and strtoul (#18151)

2009-08-31 Thread David Laight
On Mon, Aug 31, 2009 at 12:03:11PM +0200, Alexandre Julliard wrote:
> Eric Pouech  writes:
> 
> > @@ -250,3 +250,25 @@ int CDECL __STRINGTOLD( MSVCRT__LDOUBLE *value, char 
> > **endptr, const char *str,
> >  #endif
> >  return 0;
> >  }
> > +
> > +/**
> > + * strtol (MSVCRT.@)
> > + */
> > +long int MSVCRT_strtol(const char* nptr, char** end, int base)
> > +{
> > +/* wrapper to forward libc error code to msvcrt's error codes */
> > +long ret = strtol(nptr, end, base);
> > +msvcrt_set_unix_errno();
> > +return ret;
> > +}
> 
> You can't simply use long here, you need to handle the difference in the
> size of long between Win32 and Unix.

You also need to an 'errno = 0' before the strtol() call.
strtol() will only change errno if there is a numeric overflow.

In the overflow cases the return value will be LONG_MIN or LONG_MAX and
errno is set to ERANGE.  No other errno values should appear.

I thought that the only 'size' difference is that 64bit windows has a
32bit long ?
So values outside 32bits need truncating and ERANGE set ??

David

-- 
David Laight: da...@l8s.co.uk




Re: d3d10: Add annotation skipping.

2009-08-15 Thread David Laight
On Sat, Aug 15, 2009 at 07:37:12PM +0200, Henri Verbeet wrote:
> 2009/8/15 Rico Sch?ller :
> > +static inline void parse_annotation(const char **ptr, unsigned int count)
^^

And there is no reason to inline something this size 

David

-- 
David Laight: da...@l8s.co.uk




Re: How to expanding environmental variables?

2009-07-18 Thread David Laight
On Fri, Jul 17, 2009 at 07:27:11PM -0500, John Klehm wrote:
> 
> Oops I guess SHGetFolderPath is more portable (win95 and up)

It is just a pain getting the right include file and library,
especially on win2k with oldish versions of visual C.

David

-- 
David Laight: da...@l8s.co.uk




Re: Socks support in ws2_32

2009-06-06 Thread David Laight
On Sat, Jun 06, 2009 at 12:45:58PM +0200, Damjan Jovanovic wrote:
> 
> From what I see you can either write/patch a native socksification
> library to track dup()s and dup2()s, as well as possibly do something
> about the unix domain sockets that are used to transfer file
> descriptors between the app and wineserver. Or alternatively fstat() a
> socket and use the inode number as its identity instead of the file
> descriptor, since the inode doesn't change between dup()s.

I'm not 100% sure you can guarantee (probably OS dependant) that
fstat() on a socket return anything sensible for an inode number.

David

-- 
David Laight: da...@l8s.co.uk




Re: NetBSD compiler warnings patches - review request

2009-04-24 Thread David Laight
On Thu, Apr 23, 2009 at 11:46:38AM -0500, Austin English wrote:
> Howdy,
> 
> I recently started retesting NetBSD (5.0 this time), and have filed
> bugs for most of the warnings. A few of them I have patches ready for,
> but figured I should probably get reviewed first:
> 
> There's a NetBSD bug here. Their version of getdirentries returns the
> old 32-bit inode struct dirent, while dirent.h has the  new 64-bit
> inode struct dirent inode since NetBSD version 3.99.8. While it
> arguably should be fixed on their end, we already have a wrapper in
> there for an OS X bug...

I asked a few other developers last night, getdirentries() is only
present for binary compatibility with old binaries.  There shouldn't
be a prototype for it in dirent.h (or anywhere else).

Really the code should use opendir() etc (which are the posix functions).
I've not looked at what wine is doing here, but the value returned by
lseek() for directories isn't necessarily a byte offset.

David

-- 
David Laight: da...@l8s.co.uk




Re: 16bit code generation

2009-03-30 Thread David Laight
On Sun, Mar 29, 2009 at 06:23:53PM -0500, King InuYasha wrote:
>
> Unfortunately, at the moment it really is the best we have. AFAIK, there
> isn't any other FOSS compiler that can build 16-bit DOS/Win16 applications.
> Unless someone was actually willing to figure out how to make GCC be able to
> target Win16 (not likely) or write a whole new compiler toolchain to target
> Win16/DOS, there really isn't anything else left to use.

Will 'Bruce's C Compiler' target DOS/Win 16 ?

David

-- 
David Laight: da...@l8s.co.uk




Re: 16bit code generation

2009-03-22 Thread David Laight
On Sun, Mar 22, 2009 at 11:34:03AM +1100, Ben Klein wrote:
> Oops, missed reply-to-all.
> 
> 2009/3/22 Tijl Coosemans :
> > I was reading through binutils documentation and came across this.
> > Maybe it can be used to compile 16 bit tests.
> >
> > 3.2.4. 16-bit mode
> > Binutils (2.9.1.0.25+) now fully support 16-bit mode (registers and
> > addressing) on i386 PCs. Use .code16 and .code32 to switch between
> > assembly modes.
> >
> > Also, a neat trick used by several people (including the oskit authors)
> > is to force GCC to produce code for 16-bit real mode, using an inline
> > assembly statement asm(".code16\n"). GCC will still emit only 32-bit
> > addressing modes, but GAS will insert proper 32-bit prefixes for them.

I believe that does work and has been used by 'bochs' to compile bios code.

> 16bit mode is required for building bootloaders. The question here is,
> is it possible for Wine to run raw 16bit code when running under a
> *nix kernel, which typically (always?) run in protected mode? I
> believe it is not possible without some sort of 16bit emulator/wrapper
> (e.g., current win16 libs are actually compiled as 32bit).

NetBSD (at least) has VM86 support in the kernel, however I don't know
if it still works :-)

David

-- 
David Laight: da...@l8s.co.uk




Re: CreateThread timing behaviour (Bug 15323 affecting Warhammer Online)

2009-03-01 Thread David Laight
On Sun, Mar 01, 2009 at 06:08:53PM +1100, Paul TBBle Hampson wrote:
> I'm looking into Bug 15323, and it seems to come down to a
> particular undocumented behaviour of CreateThread on Windows,
> which is probably also a bug in the usage of it by warpatch.bin,
> the Warhammer Online patcher.
> 
> Problem code outline:
> Main thread:
> flag = false;
> createThread( newThreadFun )
> sleep until flag = false;
> 
> newThreadFun:
> flag = true
> CreateHiddenWindowAndWaitForMessageFromMainThread;
> flag = false;
> return;
> 
> On inspection, the only way this can work is if the main thread's
> "sleep until" test is done before newThreadFun gets started.

Isn't that backwards?
Doesn't the above rely on the new thread pre-empting the main thread?
(The above example seems to be implementing a function call!)

In any case the above example is doomed to failure.
I don't expect that windows defines which thread runs first.
And on a multi-processor system they are likely to both run.

So I'd guess that warpatch.bin or broken.

David

-- 
David Laight: da...@l8s.co.uk




Re: kernel32: fix cpu detection on NetBSD

2009-01-18 Thread David Laight
On Sun, Jan 18, 2009 at 06:40:30PM +0100, Francois Gouget wrote:
> On Sat, 17 Jan 2009, Austin English wrote:
> [...]
> > bash-3.2$ make thread.ok
> > ../../../tools/runtest -q -P wine -M kernel32.dll -T ../../.. -p
> > kernel32_test.exe.so thread.c && touch thread.ok
> > err:process:__wine_kernel_init boot event wait timed out
> > assertion "thread->pt_blockgen == thread->pt_unblockgen" failed: file
> > "/home/builds/ab/netbsd-4-0-1-RELEASE/src/lib/libpthread/pthread_lock.c",
> > line 195, function "pthread_spinlock"
> > assertion "thread->pt_blockgen == thread->pt_unblockgen" failed: file
> > "/home/builds/ab/netbsd-4-0-1-RELEASE/src/lib/libpthread/pthread_lock.c",
> > line 195, function "pthread_spinlock"
> > *** Error code 1
> > 
> > Stop.
> > make: stopped in /home/austin/wine-git/dlls/kernel32/tests
> > 
> > Haven't found a fix for that one yet. They've got a patch in their
> > port to use use wine-kthread by default, but doesn't seem to help.
> 
> Ok. That looks like another issue. Does anything run at all on NetBSD?

A lot of things run on NetBSD :-)

Which version of NetBSD are you testing on?
NetBSD 4 uses an m:n thread library, so there is no guarantee which system
LWP will run which user thread (it will change dynamically).
(In particular when one user thread wakes another, it can be pre-empted
entirely in userspace.)

NetBSD 5 uses a 1:1 thread library due to intractable problems on SMP
systems.

David

-- 
David Laight: da...@l8s.co.uk




Re: running msys under wine

2009-01-10 Thread David Laight
On Sat, Jan 10, 2009 at 10:23:49AM +0100, Ambroz Bizjak wrote:
> I don't think this is true. When the shell forks to create a child, it doesn't
> have to give the child the same standard file descriptors - rather it creates 
> pipes, and it passes stdin to the child by reading its own stdin than writing 
> the data to the child's stdin pipe. This way it can filter the input (like for
> control sequences). So, if it has any excess input data when it creates a
> child, it simply writes that data to the stdin pipe first.
> That is how I think things are and should be done.

Not in any shells I've fixed.
There is no need for the shell itself to pre-process process input/output.
There are no control sequences for the shell.
(^C etc are detected by the kernel line-discipline code and sent as signals
to the session leader.)

David

-- 
David Laight: da...@l8s.co.uk




Re: running msys under wine

2009-01-09 Thread David Laight
On Fri, Jan 09, 2009 at 06:39:27PM +, Luke Kenneth Casson Leighton wrote:
> 
> further up the strace files, i'm looking at the biggest offender and
> it looks like it's reading files one byte at a time.

Certainly a normal unix shell has to read input (from stdin) byte by byte
since it never knows when it might have to fork a process passing stdin
through - and the child needs the correct file offset.
(And the unix kernel doesn't have a 'read until newline' for files.)

If stdin is seekable or mapable there are other options, but that is
difficult to detect and code for.

David

-- 
David Laight: da...@l8s.co.uk




Re: wow, there are more win64 apps than I thought...

2008-12-21 Thread David Laight
On Sun, Dec 21, 2008 at 02:00:14AM +0100, Francois Gouget wrote:
> 
> But Microsoft decided that since Windows application programmers have 
> never encountered the word 'portability' even once in their life, they 
> could not be expected to have understood the difference between 'int' 
> and 'long', and thus would likely be so upset if 'long' where ever to 
> change size that they might commit suicide in droves. So they decided 
> that 'long' would remain 32bits.

Not only Windows application programmers

What type to you think PDWORD_PTR is ?

David

-- 
David Laight: da...@l8s.co.uk




Re: [PATCH 03/10] jscript: Added NaN value implementation.

2008-10-16 Thread David Laight
On Wed, Oct 15, 2008 at 04:50:30PM -0500, Jacek Caban wrote:
> Alexandre Julliard wrote:
> >I'd suggest to take advantage of the variant union instead of copying
> >all these glibc definitions. Something like
> >
> >#ifdef NAN
> >V_R8(&vt) = NAN;
> >#else
> >V_I8(&vt) = nan_magic_pattern;
> >#endif
> >  
> 
> The problem is that nan_magic_pattern would have to be 8 bytes and AFAIK 
> there is no portable way to do it without ugly casts like in attached patch.

And that break C's 'strict aliasing' rules that gcc might apply
when optimising code.

David

-- 
David Laight: [EMAIL PROTECTED]




Re: msvcrt: scanf fix a typo

2008-09-21 Thread David Laight
On Fri, Sep 19, 2008 at 03:51:00PM -0700, Dan Kegel wrote:
> Here's the gcc error:
> > scanf.c:66: warning: unknown conversion type character `P' in format
> 
> I'm not a programmer, but I play one on TV.  And here's what I
> came up with in five minutes of typing and not enough thinking:
> 
> That particular error depends on gcc knowing intimate details of
> sscanf.   Unless we teach gcc about the particular sscanf we're
> implementing, it's likely to give false errors.

For the tests, the compile time error can be avoided by using
a non-constant format string.

Quite possibly adding a global variable whose value is a
always zero to the format string is enough.

David

-- 
David Laight: [EMAIL PROTECTED]




Re: devenum: Fix order of operations bug (Coverity id 709)

2008-09-10 Thread David Laight
On Wed, Sep 10, 2008 at 01:38:03PM -0700, Juan Lang wrote:
> > I'm pretty sure this would come out as:
> > dwMediaTypeSize + (dwMediaTypeSize < (2 ? 1 : dwMediaTypeSize) / 2)
> 
> It doesn't, check the order of operations again.
> 
> > But even if it doesn't, I don't think it'd hurt to be more explicit:
> > dwMediaTypeSize + ((dwMediaTypeSize < 2) ? 1 : (dwMediaTypeSize/2))
> 
> Why use two parentheses when 6 will do, eh?  Sorry, I don't agree that
> this is clearer.

Agreed, if you add too many parenthesis it gets difficult to see
where they line up (which some tool to show you).
I get peeved that gcc always warns about && and || - where the order
is obvious and correctly defined.

Blindly putting parenthesis around conditional expressions leads people 
to write:
dwMediaTypeSize + (dwMediaTypeSize < 2) ? 1 : dwMediaTypeSize / 2
which is unlikely to have the desired effect!

David

-- 
David Laight: [EMAIL PROTECTED]




Re: [1/3] WineD3D: Use C bitfields to compact the ffp description (try 2)

2008-07-30 Thread David Laight
On Wed, Jul 30, 2008 at 08:01:55AM -0500, Stefan D?singer wrote:
> > It's not very clean to store -1 into unsigned variables, the previous
> > code was better IMO.
> ok, I'll resend

You probably need to specify ~0u otherwise the constant might be
deemed to have the wrong type.

    David

-- 
David Laight: [EMAIL PROTECTED]




Re: xfs getting case-insensitive filesystem support...

2008-04-23 Thread David Laight
On Thu, Apr 24, 2008 at 03:05:51PM +0900, Dmitry Timoshkov wrote:
> "Joerg Mayer" <[EMAIL PROTECTED]> wrote:
> 
> > On Wed, Apr 23, 2008 at 08:39:40PM -0700, Dan Kegel wrote:
> >> Hrm.  This is interesting:
> >>   http://lwn.net/Articles/278961/
> >> I wonder if Wine could run faster if it could delegate
> >> the case-insensitvity to the filesystem...
> > 
> > Would be nice indeed, but I don't think it will come: Linus shot down
> > similar patches in the past because to_lower may be easy to implement
> > for ascii, but once you want to implement it for all charsets supported
> > by the files system it gets very messy.
> 
> Sure, it's messy until the filesystem is unicode internally, and uses
> something more reasonable instead of a multibyte UTF-8.

Unix filesystems treat filenames as byte-strings, this is 100% reasonable :-)
Any conversion to glygps isn't a filesyatem operation.

David

-- 
David Laight: [EMAIL PROTECTED]




Re: Fix "warning: cast from pointer to integer of different size"

2008-04-23 Thread David Laight
On Tue, Apr 22, 2008 at 11:50:05PM +0900, Dmitry Timoshkov wrote:
> "Erik de Castro Lopo" <[EMAIL PROTECTED]> wrote:
> 
> > /* get pointer to object containing list element */
> > #define LIST_ENTRY(elem, type, field) \
> > -((type *)((char *)(elem) - (unsigned int)(&((type *)0)->field)))
> > +((type *)((char *)(elem) - (unsigned long)(&((type *)0)->field)))

How about:
((type *)((char *)(elem) - ((char *)(&((type *)0)->field) - (char *)0)))

David

-- 
David Laight: [EMAIL PROTECTED]




Re: [PATCH 3/3] configure: Use 0 instead of NULL because NULL isn't defined.

2007-10-23 Thread David Laight
On Sat, Oct 20, 2007 at 11:12:10AM +0200, Alexandre Julliard wrote:
> Michael Stefaniuc <[EMAIL PROTECTED]> writes:
> 
> > Actually it's not a cast. It's a heuristic that the compiler has to make
> > to detect a NULL pointer. The best heuristic for this is (char *) 0.

(char *)0 is bad, 0 is ok, as is (void *)0
> 
> There's no heuristic involved here, the assignment is clearly to a
> pointer, and 0 is a perfectly valid null pointer, no cast is needed.

As is 1-1, or any other compile time integer expression with a numeric
vaule of zero.

David

-- 
David Laight: [EMAIL PROTECTED]




Re: widl [3/4]: Remove printf format strings that aren't really format strings.

2007-10-23 Thread David Laight
On Thu, Oct 18, 2007 at 11:12:23AM +0200, Michael Stefaniuc wrote:

> > -fprintf(file, varname);
> > +fprintf(file, "%s", varname);

> fputs() would be a faster alternative to that. Though i doubt that
> matters in this case.

FWIW gcc tends to convert fprintf(file, "%s", arg) into fputs(arg, file).
Discovered on a semi-embedded system which had fprintf, but not fputs :-)

David

-- 
David Laight: [EMAIL PROTECTED]




Re: winedump: Cast-qual warnings fix

2007-07-13 Thread David Laight
On Fri, Jul 13, 2007 at 12:29:36PM +0200, Alexandre Julliard wrote:
> Andrew Talbot <[EMAIL PROTECTED]> writes:
> 
> > +  const char *iter, *base_type, *catch_unsigned;
> > +  union
> > +  {
> > +  const char*constant;
> > +  char  *nonconst;
> > +  } type_str;
> 
> That's not better than simply casting const away, it's just hiding the
> problem from the compiler.

And I'm not sure that the compiler is required to treat the two
fields of the union as being the same data item.
Certainly if the fields of the union are 'void *' and 'intptr_t'
you can't assume that a value written to one field of the union 
can be immediately read from the other.

David

-- 
David Laight: [EMAIL PROTECTED]




Re: Should Wine move to LGPL 3?

2007-07-13 Thread David Laight
On Fri, Jul 13, 2007 at 01:18:41PM +0200, Damjan Jovanovic wrote:
> 
> The GPL3 has no track record so far, and it's too political and
> controversial for my liking. Let's wait a while before making the
> decision.

Many groups are exceedingly worried about parts of GPL3.
Not only commercial companies who may not have been obeying the
general spirit of the GPL, but also people writing software that
is BSD licensed are worried about having GPL3 utilities
included in a software release, and having GPL3 source residing in
the local CVS tree.

    David

-- 
David Laight: [EMAIL PROTECTED]




Re: illegal characters in file names

2007-07-03 Thread David Laight
On Tue, Jul 03, 2007 at 11:04:10AM +0100, Chris Howe wrote:
> True. A friend of mine accidentally created a file called * in his home
> directory on the first day of university and was afraid to delete it for
> 3 years.

Actually, much more fun was the filename that contained the escape
sequence that caused a terminal to send back its identification sequence.

Running 'ls' caused keyboard input 

    David

-- 
David Laight: [EMAIL PROTECTED]




Re: illegal characters in file names

2007-07-01 Thread David Laight
On Sun, Jul 01, 2007 at 02:57:58AM +0200, J?r?me Gardou wrote:
> 
> All I know is that wine shouldn't use * and ? , since it is used in wildcards 
> on both Unix and Windows !

But they are not illegal in unix filenames.
Only 2 byte values cannot be used, 0 and '/'.

    David

-- 
David Laight: [EMAIL PROTECTED]




Re: xcopy try 2

2007-02-24 Thread David Laight
On Sat, Feb 24, 2007 at 01:00:06AM +0100, Detlef Riekenberg wrote:
> 
> +C_SRCS = \
> +xcopy.c
> 
> Spaces are wrong before the filename. You must use TAB in makefiles

That is a contiuation line, IIRC the whitespace at the start of the
continued line should be discarded (and can be space/tab anyway).

David

-- 
David Laight: [EMAIL PROTECTED]




Re: u_int32_t

2006-10-14 Thread David Laight
On Sat, Oct 14, 2006 at 10:53:46PM +1000, Robert Lunnon wrote:
> Seems these typedefs have found their way into some of the source files, from 
> my recollection these definitions aren't used. What is the official typedef 
> for these types;
> IE
> u_int32_t
> uint32_t
> or
> UINT32

The C99 type is 'uint32_t' - and should be defined by inttypes.h


David

-- 
David Laight: [EMAIL PROTECTED]




Re: Copy protection

2006-10-04 Thread David Laight
On Wed, Oct 04, 2006 at 09:41:16AM -0500, Tom Spear wrote:
> 
> I agree  that we shouldn't write to the MBR, but I definitely think that we
> should get some legal guidance before we proceed with writing anything to a
> file (in this case), because

If it isn't a silly question, which part of the mbr sector do the games
think they can access?  Especially for write ?

Having written the mbr code that NetBSD uses - which could validly be
in sector zero of the boot disk of a windows systesm - I'm not at all
certain there any bytes that can usefully be used.

David

-- 
David Laight: [EMAIL PROTECTED]




Re: Visual C++ does not seem to have snprinft

2006-09-08 Thread David Laight
On Fri, Sep 08, 2006 at 12:05:29AM +1000, Jeff L wrote:
> 
>#define snprintf _snprintf

Unfortunately the above isn't adequate, the windows _snprintf() isn't
the same beast as the posix (or is it even part of C now?) snprintf()
in particular:

1) _snprintf() returns -1 if the data wouldn't fit (not the number of bytes
   that would have been output).

2) _snprintf() doesn't always NUL terminate the output!  In particular
   _snprintf(buf, 2, "ab" ) will set buf[0] = 'a', buf[1] = 'b', and
   return 2.
   Almost certainly not what the calling code expceted.

David

-- 
David Laight: [EMAIL PROTECTED]




Re: Alexandre Julliard : crypt32/tests: Avoid sizeof in traces.

2006-08-26 Thread David Laight
On Fri, Aug 25, 2006 at 02:18:49PM -0700, Dan Kegel wrote:
> On 8/25/06, Alexandre Julliard <[EMAIL PROTECTED]> wrote:
> >> Just so I know, what is the warning? Does sizeof() return
> >> a 64-bit integer on those platforms?
> >
> >Not on 32-bit platforms, but it's defined as long instead of int so we
> >still get a printf format warning.
> 
> The incredibly ugly solution I've used in the past is
> 
> // printf macros for size_t, in the style of inttypes.h
> #ifdef _LP64
> #define __PRIS_PREFIX "z"
> #else
> #define __PRIS_PREFIX
> #endif
> #define PRIuS __PRIS_PREFIX "u"
> 
> and then
>  printf("size is %" PRIuS "\n", sizeof(foo));
> 
> Much nicer just to avoid using size_t in printf's if you can.

That doesn't help for the case:
char xxx[...]
...
printf("%.*s", sizeof xxx, xxx);
since the argument for '*' has to be of type 'int'.
Here you have to have the (int) cast, directly of maybe via:
#define isizeof (void)sizeof

David

-- 
David Laight: [EMAIL PROTECTED]




Re: Wide-string Functions: Double Casting

2006-08-08 Thread David Laight
On Tue, Aug 08, 2006 at 09:27:23PM +0200, Eric Pouech wrote:
> what I don't like is that in order to plug a hole (casting from const 
> foo* to foo*), we create a bigger hole by allowing to cast from const 
> foo* to bar* (and the compiler will not give any warning)
> if we want to go into this, then I'd rather have _deconst explicitly use 
> the type:
> #define __deconst(v,X)({const X _v = (v); ((X)(int)_v);})
> and in the previous example use __deconst(str, char*);

That isn't valid C, you could do:
((const X)(v) - (v), (X)(int)(v))
but that is likely to give a 'computed value not used' warning instead.

What you really don't want to do is allow casts from 'int' to 'foo *'.
After all casting from 'foo *' to 'bar *' is easy.

David

-- 
David Laight: [EMAIL PROTECTED]




Re: Wide-string Functions: Double Casting

2006-08-07 Thread David Laight
On Mon, Aug 07, 2006 at 09:54:20PM +0100, Andrew Talbot wrote:
> would like to submit a patch that, for example, changes strchrW() to:
> 
> extern inline WCHAR *strrchrW( const WCHAR *str, WCHAR ch )
> {
> WCHAR *ret = NULL;
> do { if (*str == ch) ret = (WCHAR *)(size_t)str; } while (*str++);
> return ret;
> }

why not just have:

extern inline void *__deconst(const void *v)
{
return (char *)0 + ((const char *)v - (const char *)0));
}

Then the code above could be:
extern inline WCHAR *strrchrW( const WCHAR *str, WCHAR ch )
{
do { if (*str == ch) return __deconst(str); } while (*str++);
return 0;
}

    David

-- 
David Laight: [EMAIL PROTECTED]




Re: pch support

2006-01-12 Thread David Laight
On Thu, Jan 12, 2006 at 09:02:42PM +0100, Rolf Kalbermatter wrote:
> Thomas Weidenmueller wrote:
> 
> > We've been using PCH with GCC for a long time in ReactOS, it's been
> > working very well and reliable. Compiling ReactOS is *a lot* 
> > faster with
> > PCH enabled.
> 
> Basically what I get in MSVC 6 when modifing a header somewhere and
> PCH is enabled is that MSVC keeps mocking about errors in the
> headers, my change was supposed to fix. Disabling PCH (and
> sometimes deleting the *.pch file) always fixes those issues.

Unless, of course, your filesystem is samba-mounted from a linux box,
when PCH tends to cause internal compiler errors.

David

-- 
David Laight: [EMAIL PROTECTED]




Re: [NDR] Implement NdrClientCall2 and NdrServerCall2

2005-12-01 Thread David Laight
On Thu, Dec 01, 2005 at 11:09:29AM +0100, Alexandre Julliard wrote:
> Robert Shearman <[EMAIL PROTECTED]> writes:
> 
> > +"shrl $2, %ecx\n\t" /* divide by 4 */
> > +"rep movsl\n\t" /* Copy dword blocks */
> > +"movl %eax, %ecx\n\t"
> > +"andl $3, %ecx\n\t" /* modulus 4 */
> > +"rep movsb\n\t" /* Copy remainder */
> 
> If the argument size is not a multiple of 4 you are in serious
> trouble...

Not only that, but the code above is not very efficient!
The setup time for 'rep movsx' instruction is significant on many
modern cpus, making the second 'rep movsb' particularly slow.
I'm not even sure what the break-even length for the one is!

Sequence like (give or take assembler syntax):
mov %eax,(%esi+%ecx-4)
    mov (%edi+%ecx-4),%eax
shrl $2, %ecx
rep movsl
should be better given large enough %ecx

David

-- 
David Laight: [EMAIL PROTECTED]




Re: Wine on NetBSD?

2005-09-25 Thread David Laight
On Sun, Sep 25, 2005 at 01:04:42AM +0100, Ivan Leo Puoti wrote:
> Bryce Robilliard wrote:
> >Hello,
> >
> >I was enquiring as to whether or not Wine is compatible with NetBSD, or 
> >if any other port of Wine is compatible with NetBSD.  If not are there 
> >any plans to make this so?
> 
> Wine is meant to work on NetBSD, however chances are the build has broken 
> over time due to lack of testing. I suggest you try building it and see 
> what happens.

There has been a working version in pkgsrc, but I suspect it is old.

David

-- 
David Laight: [EMAIL PROTECTED], [EMAIL PROTECTED]




Re: pf_vsnprintf implementatnion of 'I', 'I32' and 'I64' prefixes for type specifiers

2005-08-02 Thread David Laight
On Tue, Aug 02, 2005 at 10:35:59PM +0900, Mike McCormack wrote:
> 
> It's a little more complicated than you think, after you start handling 
> all the width modifiers, various floating point formats, etc.

Not to mention coding in the 'brokenness'!
On windows:
  _snprintf(buff, 3, "ab" ) returns 2, buff = "ab<0>"
  _snprintf(buff, 3, "abcd" ) returns -1, buff = "ab<0>"
but:
  _snprintf(buff, 3, "abc" ) returns 3, buff = "abc"
without NUL terminating the string.

Do you replicate that?

David

-- 
David Laight: [EMAIL PROTECTED]



Re: unicode.h and -Wcast-qual

2005-06-20 Thread David Laight
On Mon, Jun 20, 2005 at 10:26:32PM +0200, Stefan Huehner wrote:
> In samba_4 sourcecode i discovered the following macros, which are
> apparently a hack but silences these warnings:
> 
> #define discard_const(ptr) ((void *)((intptr_t)(ptr)))
> #define discard_const_p(type, ptr) ((type *)discard_const(ptr))

Or:

static inline void *deconst(const void *p)
{
return ((const char *)p - (const char *)0) + (char *)0;
}

David

-- 
David Laight: [EMAIL PROTECTED]



Re: unixfs: cache canonicalized unix paths corresponding to dos devices

2005-06-17 Thread David Laight
On Thu, Jun 16, 2005 at 11:26:45PM +0200, Alexandre Julliard wrote:
> 
> I think the whole canonicalization thing is suspect; you should never
> compare Unix path strings. If you have to compare paths you should use
> stat and compare device/inode.

Except that some FS have difficulty generating unique inode numbers.
(There is no answer to the problem though)

    David

-- 
David Laight: [EMAIL PROTECTED]



Re: Debugger under Solaris

2005-06-11 Thread David Laight
On Sat, Jun 11, 2005 at 09:17:09AM +1000, Robert Lunnon wrote:
> I need some help to implement the debugger under Solaris. In particular I 
> need 
> help with how ptrace interacts with the threading model under Linux which I 
> understand uses processes for threads. Since Solaris has user mode threads, 
> stopping a pid stops all the threads in the debugee which used to deadlock 
> the debugger. This suggests I need to operate on threads within processes 
> rather than the processes themselves. This is possible with a few IPC tricks, 
> but I need to understand the semantics

I think you will find that solaris uses kernel LWPs for threads.
If you give ps the correct incantation it wil show them.

    David

-- 
David Laight: [EMAIL PROTECTED]



  1   2   >