wine-devel

wine-devel@winehq.org

7 participants
84405 discussions

RE: mssip.h trouble
by Patrik Stridvall 10 Sep '02

10 Sep '02

> Patrik Stridvall wrote: > > > - No GNU C compiler not even 3.1 support #pragma pack(8) nor > > does it support #pragma pack(16). It doesn't even give a warning. > > Well, GCC does support #pragma pack(8) and #pragma pack(16). > They just don't do anything on i386, because they don't need to. > > What #pragma pack(N) means is: if any structure member would > require a default alignment > N according to the normal rules, > reduce that alignment to N. > > As on i386 no structure member ever requires a default alignment > of > 8, #pragma pack(8) never needs to do anything. > > You appear to think that #pragma pack(N) should sometimes > *increase* some alignment requirement -- but it never does. That sound reasonable. However Microsoft doesn't seem to be reasonable. :-) > Under what circumstances do you think it should? Actually I don't think anything. I have tested what different compilers does. :-) As I explain in my latest mail (that perhaps you didn't read since it was only 5 minutes between the mails :-). On MSVC it does matter in the following example whether n is 4 or 8. Or 16 for that matter eventhough that gives the same result as 8. #ifdef _MSC_VER typedef __int64 longlong_t; #else typedef long long longlong_t; #endif #pragma pack(n) typedef struct { char x; longlong_t y; /* offset is 4 if n = 4, but is 8 if n = 8 or n = 16*/ } longlong_n; #pragma pack()

2 1

RE: mssip.h trouble
by Patrik Stridvall 10 Sep '02

10 Sep '02

> Patrik Stridvall <ps(a)leissner.se> writes: > > > I'm totally at loss as to what to do. Not that it currently > matters since > > the data structures isn't currently used but someday > somebody will... > > Well, we need to add explicit padding in the structures to respect the > alignment that MSVC uses. That should only be required at the end of > the structures I think, so it's not too bad. Unfortunately its a little more complicated than that, it effects the internal offset of fields as well in some case. In the normal case below it doesn't matter whether n is 4 or 8. #pragma pack(n) typedef struct { char x; long y; /* offset is 4 both for n = 4 and n = 8 */ } long_n; #pragma pack() However is the case below is does matter whether n is 4 or 8: #ifdef _MSC_VER typedef __int64 longlong_t; #else typedef long long longlong_t; #endif #pragma pack(n) typedef struct { char x; longlong_t y; /* offset is 4 if n = 4, but is 8 if n = 8 */ } longlong_n; #pragma pack() In short, if the size of a field is larger than 4 it matters internally whether alignment is 4 or 8. Now in the case of mssip.h, I can't see any fields that have sizes larger than 4 so we could easily handle that special case as you suggested. However in general we can not...

2 1

RE: mssip.h trouble
by Patrik Stridvall 10 Sep '02

10 Sep '02

> On Tue, 10 Sep 2002, Patrik Stridvall wrote: > > > > While compiling the latest tree, I get this: > > > > > > [dimi@dimi wine]$ make -s > > > In file included from ../../include/mssip.h:49, > > > from main.c:22: > > > ../../include/pshpack8.h:47:6: warning: #warning "8 as > > > alignment is not supported" > [...] > > It seems that gcc or at least a previous versions of GCC > > didn't support it since somebody (Alexandre?) added a > > warning in pshpack8.h. > > It may have been me. I think at the time it was only meant for gcc, > which was the only things you could compile Wine with anyway. > The specific version of gcc is 2.95, and but I'm really not sure the > situation has changed. It hasn't. GCC 3.1 has the same problem. > > Futhermore Micrsoft C/C++ (or at least later version of it) > > contrary to the claim of pshpack8.h support it so it > > wrong at the point at least AFAIK. > > Yes, Visual C++ supports alignment 8 which is the problem (so that > defined(_MSC_VER) should have been moved elsewhere, just to avoid the > warning in fact). > > How we can handle such alignment issues in Wine whn compiled with gcc > I'm not sure. Neither am I. I have tested the alignment with various compiler and got the follow result: - No GNU C compiler not even 3.1 support #pragma pack(8) nor does it support #pragma pack(16). It doesn't even give a warning. - MSVC 6.0 support #pragma pack(8) but not #pragma pack(16) - The Solaris compiler (old version) doesn't even support #pragma pack(4), but then 4 is the default so it doesn't really matter that much (except for a warning). It support #pragma pack(1) and #pragma pack(2) though. So the conclusion is that only MSVC support #pragma pack(8), nothing else. I'm totally at loss as to what to do. Not that it currently matters since the data structures isn't currently used but someday somebody will... PS. I can't understand why GNU C doesn't support it, it can't be that difficult to implement can it? Nether the less that fact that it doesn't warn about it is even worse. That certainly can't be difficult to implement.

5 4

RE: mssip.h trouble
by Patrik Stridvall 10 Sep '02

10 Sep '02

> While compiling the latest tree, I get this: > > [dimi@dimi wine]$ make -s > In file included from ../../include/mssip.h:49, > from main.c:22: > ../../include/pshpack8.h:47:6: warning: #warning "8 as > alignment is not supported" > In file included from ../../include/mssip.h:80, > from main.c:22: > ../../include/pshpack8.h:47:6: warning: #warning "8 as > alignment is not supported" > In file included from ../../include/mssip.h:88, > from main.c:22: > ./../include/pshpack8.h:47:6: warning: #warning "8 as > alignment is not supported" > In file included from ../../include/mssip.h:97, > from main.c:22: > ../../include/pshpack8.h:47:6: warning: #warning "8 as > alignment is not supported" > In file included from ../../include/mssip.h:109, > from main.c:22: > ../../include/pshpack8.h:47:6: warning: #warning "8 as > alignment is not supported" > In file included from ../../include/mssip.h:123, > from main.c:22: > ../../include/pshpack8.h:47:6: warning: #warning "8 as > alignment is not supported" > In file included from ../../include/mssip.h:140, > from main.c:22: > ../../include/pshpack8.h:47:6: warning: #warning "8 as > alignment is not supported" Ah, yes. I forgot to investigate that issue further. Sorry. First of all I want so say that the warning in itself is harmless. However anything using data structures from the header might not work correctly, but then currently only stub functions use it so it doesn't really matter. The problem is that mssip.h needs 8 as alignement for its data structures. It seems that gcc or at least a previous versions of GCC didn't support it since somebody (Alexandre?) added a warning in pshpack8.h. Futhermore Micrsoft C/C++ (or at least later version of it) contrary to the claim of pshpack8.h support it so it wrong at the point at least AFAIK. The big question is how should the following lines look like: # if defined(__GNUC__) || defined(__SUNPRO_C) || defined(__SUNPRO_CC) || defined(_MSC_VER) # pragma pack(8) # warning "8 as alignment is not supported" # elif !defined(RC_INVOKED) # error "Adjusting the alignment is not supported with this compiler" # endif Or perhaps phrased in another way: What compiler and specifically what versions or the compiler supports and handles #pragma pack(8) correctly? Note that just because it compiles without warning (or error) doesn't mean that it supports it correctly.

2 1

patches policy
by Dimitrie O. Paun 10 Sep '02

10 Sep '02

Hi all, There is something concerning submitting patches that bothers me to no end: inlining vs. attaching them. I don't know about others, but for me 99/1 rule applies: I at least skim over 99% of the patches inlines in the message, whereas I bother to read at most 1% of the ones attached/tar.gzed/ziped. Maybe I'm an extreme case, but there's got to be more to it than my personal quirks. If a patch is sent to wine-patches, it's sent there for peer review. If you don't want the review, send it to Alexandre directly, even though I suggest this is avoided as much as possible. However, it you do send it to wine-patches, please, *please* inline it! What about a nice patch submission policy: -- unified diff only (required) -- have a decent subject (recommended) -- a long description (optional, if the change warrants it) -- a meaningful ChangeLog entry (required) -- new files, if any, included in patch, diffed against /dev/null (required) -- patch inlined at the end of the message (required) -- one changeset per message Most of these things are already followed by most people, with the exception of the inlining bit. Alexandre, what about you reject patches that aren't in this format with a pointer to these rules? All this is a matter of habit, and I think we'll all benefit if these rules are followed. -- Dimi.

9 12

mssip.h trouble
by Dimitrie O. Paun 10 Sep '02

10 Sep '02

While compiling the latest tree, I get this: [dimi@dimi wine]$ make -s In file included from ../../include/mssip.h:49, from main.c:22: ../../include/pshpack8.h:47:6: warning: #warning "8 as alignment is not supported" In file included from ../../include/mssip.h:80, from main.c:22: ../../include/pshpack8.h:47:6: warning: #warning "8 as alignment is not supported" In file included from ../../include/mssip.h:88, from main.c:22: ../../include/pshpack8.h:47:6: warning: #warning "8 as alignment is not supported" In file included from ../../include/mssip.h:97, from main.c:22: ../../include/pshpack8.h:47:6: warning: #warning "8 as alignment is not supported" In file included from ../../include/mssip.h:109, from main.c:22: ../../include/pshpack8.h:47:6: warning: #warning "8 as alignment is not supported" In file included from ../../include/mssip.h:123, from main.c:22: ../../include/pshpack8.h:47:6: warning: #warning "8 as alignment is not supported" In file included from ../../include/mssip.h:140, from main.c:22: ../../include/pshpack8.h:47:6: warning: #warning "8 as alignment is not supported" nm: devenum.dll.tmp.o: no symbols nm: dplay.dll.tmp.o: no symbols -- Dimi.

1 0

Re: winedbg: enhancing symbol lookup
by Alexandre Julliard 10 Sep '02

10 Sep '02

Eric Pouech <eric.pouech(a)wanadoo.fr> writes: > --- debugger/break.c 30 Jul 2002 00:06:34 -0000 1.35 > +++ debugger/break.c 2 Sep 2002 19:39:59 -0000 > @@ -421,10 +421,15 @@ > DBG_VALUE value; > int i; > > - if (DEBUG_GetSymbolValue(name, lineno, &value, TRUE)) > + switch (DEBUG_GetSymbolValue(name, lineno, &value, TRUE)) > { > + case TRUE: > DEBUG_AddBreakpoint(&value, NULL, TRUE); > return; > + case FALSE: > + break; > + case ABORT: /* user aborted symbol lookup */ > + return; > } Please don't invent a third boolean state. A boolean is either TRUE or FALSE; if you need more than that you should define a new enum type with meaningful names. Reusing TRUE and FALSE will cause major confusion. -- Alexandre Julliard julliard(a)winehq.com

1 0

Re: winemaker: wineapploader generation
by Alexandre Julliard 10 Sep '02

10 Sep '02

Francois Gouget <fgouget(a)codeweavers.com> writes: > * tools/winemaker > Generate wineapploader using autoconf rather than from the Makefile Actually it's better to generate this kind of file from the Makefile; this way you can change things without having to re-run configure (by passing the values directly to make). For that reason the autoconf guidelines state that configure should only generate makefiles, not other files. -- Alexandre Julliard julliard(a)winehq.com

1 0

RE: patches policy
by Patrik Stridvall 09 Sep '02

09 Sep '02

> There is something concerning submitting patches that bothers me > to no end: inlining vs. attaching them. Yes!!! Finally. Another flamewar. :-) I will mainly comment how I submit my patches. I don't personally care that much what others do. However if Alexandre finds some way more convenient I can certain change... > I don't know about others, but for me 99/1 rule applies: I at least > skim over 99% of the patches inlines in the message, whereas > I bother to read at most 1% of the ones attached/tar.gzed/ziped. > Maybe I'm an extreme case, but there's got to be more to it than > my personal quirks. > > If a patch is sent to wine-patches, it's sent there for peer review. > If you don't want the review, send it to Alexandre directly, even > though I suggest this is avoided as much as possible. However, > it you do send it to wine-patches, please, *please* inline it! > > What about a nice patch submission policy: > -- unified diff only (required) Agreed (if the file already exists). > -- have a decent subject (recommended) Agreed. > -- a long description (optional, if the change warrants it) Agreed. > -- a meaningful ChangeLog entry (required) Agreed. > -- new files, if any, included in patch, diffed against > /dev/null (required) I currently use .tar files since it handles the case that Uwe was talking about that is new directories. But then, I use a script to generate my patches, so I can make the normal case (no new directory) as a diff against /dev/null. > -- patch inlined at the end of the message (required) If it is inlined at all it should certainly be at the end, agreed. > -- one changeset per message What about dependent changesets? Or changeset that are otherwise logically dependent (even if not syntactically or semantically)? I guess they could be perhaps is multiple messages like: Subject: Fix various bugs (part 1 of 3) Subject: Fix various bugs (part 2 of 3) Subject: Fix various bugs (part 3 of 3) I currently mail all my patches from Windows (Outlook) just out of habit I guess since it is where I send my other mail so that would require manual work. However I guess I could mail them directly from my Linux box using another script...

3 2

re: RFC: 'test' bugzilla keyword
by Dan Kegel 09 Sep '02

09 Sep '02

Francois Gouget wrote: > if we make it easy to track > conformance test issues in Bugzilla, we may get new contributers (sure I > realize it's not a magic bullet either). Thus I propose to add a 'test' > keyword to make it easy to find such issues in bugzilla for people > specifically interested in conformance tests. Excellent idea. The keyword 'test' might be a bit confusing -- some bug tracking systems use that word as a state. How about something short that has very few hits on Google? Like, um, "apifail" or "unitfail"? - Dan

4 6

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

2004

2003

2002

2001

wine-devel