Re: Prevent wildcards from being accepted in filenames (NT mode)
"Shachar Shemesh" <wine-patches(a)shemesh.biz> wrote:
+ /* If wer'e in NT mode - don't allow wildcards in file name */ + if( (strchrW(name, '?') || strchrW(name, '*')) && (GetVersion()&0x80000000)==0 )
(GetVersion() & 0x80000000)==0) construct is very inefficient. It forces compiler generate both a bit mask test and a comparison operation. While many modern processors have very effective bit test instructions, and we have to tell to the compiler that we want only a bit test. Simple !(GetVersion() & 0x80000000) is much better IMO. -- Dmitry.
"Dmitry Timoshkov" <dmitry(a)baikal.ru> writes:
(GetVersion() & 0x80000000)==0) construct is very inefficient. It forces compiler generate both a bit mask test and a comparison operation. While many modern processors have very effective bit test instructions, and we have to tell to the compiler that we want only a bit test.
Simple !(GetVersion() & 0x80000000) is much better IMO.
Actually it doesn't force the compiler to do anything different, both mean exactly the same thing, and gcc will generate the exact same code. -- Alexandre Julliard julliard(a)winehq.com
On November 26, 2003 11:22 pm, Dmitry Timoshkov wrote:
(GetVersion() & 0x80000000)==0) construct is very inefficient. It forces compiler generate both a bit mask test and a comparison operation. While many modern processors have very effective bit test instructions, and we have to tell to the compiler that we want only a bit test.
Simple !(GetVersion() & 0x80000000) is much better IMO.
Do you honestly think that a compiler can't figure out this is the same thing?!? Even if it didn't, you'd be hard pressed to even measure such minute differences, so no, it's not very inefficient by any means. Case in point. The following program: [dimi(a)dimi wine]$ cat temp.c int a(); int b(); void c() { if ( (a() & 0x8000000) == 0) b(); } void d() { if ( !(a() & 0x8000000)) b(); } Compiled with gcc _without_ optimizations: [dimi(a)dimi wine]$ gcc -c temp.c yields _identical_ code for both c() and d(): [dimi(a)dimi wine]$ objdump -dS temp.o temp.o: file format elf32-i386 Disassembly of section .text: 00000000 <c>: 0: 55 push %ebp 1: 89 e5 mov %esp,%ebp 3: 83 ec 08 sub $0x8,%esp 6: e8 fc ff ff ff call 7 <c+0x7> b: 25 00 00 00 08 and $0x8000000,%eax 10: 85 c0 test %eax,%eax 12: 75 05 jne 19 <c+0x19> 14: e8 fc ff ff ff call 15 <c+0x15> 19: c9 leave 1a: c3 ret 0000001b <d>: 1b: 55 push %ebp 1c: 89 e5 mov %esp,%ebp 1e: 83 ec 08 sub $0x8,%esp 21: e8 fc ff ff ff call 22 <d+0x7> 26: 25 00 00 00 08 and $0x8000000,%eax 2b: 85 c0 test %eax,%eax 2d: 75 05 jne 34 <d+0x19> 2f: e8 fc ff ff ff call 30 <d+0x15> 34: c9 leave 35: c3 ret So while I prefer your version for stylistic reasons, I can't agree with the spirit of the complaint. -- Dimi.
Dimitrie O. Paun wrote:
On November 26, 2003 11:22 pm, Dmitry Timoshkov wrote:
(GetVersion() & 0x80000000)==0) construct is very inefficient. It forces compiler generate both a bit mask test and a comparison operation. While many modern processors have very effective bit test instructions, and we have to tell to the compiler that we want only a bit test.
Simple !(GetVersion() & 0x80000000) is much better IMO.
Do you honestly think that a compiler can't figure out this is the same thing?!? Even if it didn't, you'd be hard pressed to even measure such minute differences, so no, it's not very inefficient by any means.
...
So while I prefer your version for stylistic reasons, I can't agree with the spirit of the complaint.
Something that does affect efficiency, which I debated with myself for some time, is the order of the tests. As the "if" is, by far, much more likely to be false than true, we should order the condition using a cost function that takes into account how much each condition costs to test, and how likely it is to fail (as that's the case wer'e trying to optimize - we don't really care how long it's going to take in case the "if" is true). My original gut feeling was to place the conditions in reverse order. That is, first test for version, only then test whether there are any wildcards in the string. The reason I submitted it as I did was that the first attempt would not work. It appears that "DOSFS_GetFullName" is called before the structs needed for "GetVersion" to function are initialized. As such, reversing the order was necessary to make it work (that, or insert complicated changes throughout the entire kernel initialization). In retrospect, this form is better. GetVersion is not a simple function, and is far less likely to fail than strchr. I guess ideal would be to have a global inside kernel.dll that tells us whether wer'e NT or not. There is no such global at the moment, however, and I did not wish to introduce one. -- Shachar Shemesh Open Source integration & consulting Home page & resume - http://www.shemesh.biz/
Hi, On Thu, Nov 27, 2003 at 09:01:00AM +0200, Shachar Shemesh wrote:
Something that does affect efficiency, which I debated with myself for some time, is the order of the tests. As the "if" is, by far, much more likely to be false than true, we should order the condition using a cost function that takes into account how much each condition costs to test, and how likely it is to fail (as that's the case wer'e trying to optimize - we don't really care how long it's going to take in case the "if" is true).
My original gut feeling was to place the conditions in reverse order. That is, first test for version, only then test whether there are any wildcards in the string. The reason I submitted it as I did was that the first attempt would not work. It appears that "DOSFS_GetFullName" is called before the structs needed for "GetVersion" to function are initialized. As such, reversing the order was necessary to make it work (that, or insert complicated changes throughout the entire kernel initialization).
In retrospect, this form is better. GetVersion is not a simple function, and is far less likely to fail than strchr. I guess ideal would be to have a global inside kernel.dll that tells us whether wer'e NT or not. There is no such global at the moment, however, and I did not wish to introduce one. Hmm, given your GetVersion thoughts, do we already have a janitorial or "Fun Project" that deals with sorting all concatenated if's to have the smallest penalty first?
This might have some good effect on Wine speed... Or maybe not? Andreas Mohr -- Wir kommen alle als Original zur Welt, aber die meisten von uns sterben als Kopien (Autor unbekannt)
Dmitry Timoshkov wrote:
"Shachar Shemesh" <wine-patches(a)shemesh.biz> wrote:
+ /* If wer'e in NT mode - don't allow wildcards in file name */ + if( (strchrW(name, '?') || strchrW(name, '*')) && (GetVersion()&0x80000000)==0 )
(GetVersion() & 0x80000000)==0) construct is very inefficient. It forces compiler generate both a bit mask test and a comparison operation. While many modern processors have very effective bit test instructions, and we have to tell to the compiler that we want only a bit test.
Simple !(GetVersion() & 0x80000000) is much better IMO.
Alexandre and Dimi already answered the technical aspect. I will just mention that my stylistic preferences say that you should only use boolean operators on things that are conceptually boolean. Since C (unlike modern C++) doesn't have a bool type, that means merely referring to the BOOL type, and the result of boolean returning operators (==, &&, etc.). In this case the test I wish to make is "is the uppermost bit clear?", and that translates to "(blah & 0x80000000)==0". It was not boolean before the operator, and & is not a boolean operator. Now, this is not an attempt to force my style on anyone else. I do not intend this to become a religious war. This is just the rational behind my style. While I will be happy to hear Dimi's rational for his, I don't think there is much room for an actual "discussion", as these things tend to turn into religious flame wars. Shachar P.S. A good exam, I think, to see whether your reply is presenting your own style or is an argument is to see whether your reply relies on my reply. If Dimi says "I do that because....", that's a reply, while if he says "My way is better because...", it's probably an argument. -- Shachar Shemesh Open Source integration & consulting Home page & resume - http://www.shemesh.biz/
On November 27, 2003 01:38 am, Shachar Shemesh wrote:
While I will be happy to hear Dimi's rational for his, I don't think there is much room for an actual "discussion", as these things tend to turn into religious flame wars.
OK, I'll take the bait. Of course, there is no big difference between them, but I prefer the ! variant simply because it's (1) more terse, and (2) more idiomatic (in C) for testing for 0. Note that I reached the conclusion that being religious about NULL vs. 0 is just mental masturbation. In fact, I ofter prefer 0 instead of NULL for the same reason as above: it's a lot more terse (like when calling a function that takes 10 args, 9 out of which must be 0). There are some things that are _absolutely_ fundamental to the language, and one of them is that NULL is 0. The 'NULL' abstraction is paper thin, and it simply doesn't give you anything. Can you change the definition of NULL to, say, 1 on some architecture without breaking stuff left right and center? No. So then what is its purpose, other than some sort of warm and fuzzy feeling of doing the 'right' thing? It just shows that we don't understand that in C values are either zero or non-zero, and that is the first stone at the foundation of the language, and you simply can not change that. Same holds true for booleans. I see no purpose whatsoever in pretending that boolean operators are only for BOOL. They are perfectly fine for figuring if a value is zero or not, anything else is politics ;). -- Dimi.
"Dimitrie O. Paun" <dpaun(a)rogers.com> writes:
On November 27, 2003 01:38 am, Shachar Shemesh wrote:
While I will be happy to hear Dimi's rational for his, I don't think there is much room for an actual "discussion", as these things tend to turn into religious flame wars.
There are some things that are _absolutely_ fundamental to the language, and one of them is that NULL is 0. The 'NULL' abstraction is paper thin, and it simply doesn't give you anything. Can you change the definition of NULL to, say, 1 on some architecture without breaking stuff left right and center? No.
Let me get into this (while waiting for my previous posts to arrive to the list). I'd rather say that 0 is NULL. Of course, NULL exists only in our minds, it's a #define, but According to the language definition, a constant 0 in a pointer context is converted into a null pointer at compile time. <http://www.eskimo.com/~scs/C-faq/q5.2.html> Also, we have to take care: To generate a null pointer in a function call context, an explicit cast may be required, to force the 0 to be recognized as a pointer. For example, the Unix system call execl takes a variable-length, null-pointer- terminated list of character pointer arguments, and is correctly called like this: execl("/bin/sh", "sh", "-c", "date", (char *)0);
So then what is its purpose, other than some sort of warm and fuzzy feeling of doing the 'right' thing? It just shows that we don't understand that in C values are either zero or non-zero, and that is the first stone at the foundation of the language, and you simply can not change that.
I can't agree with the above, see the surrsoundings of the cited page. To cite more: Summary: Unadorned 0 okay: | Explicit cast required: ---------------------+---------------------------- initialization | function call, | no prototype in scope assignment | | variable argument in comparison | varargs function call | function call, | prototype in scope, | fixed argument | Hmm, those winetests code samples seem to be missing in action. I give them some more time, then repost them. Feri.
According to the language definition, a constant 0 in a pointer context is converted into a null pointer at compile time.
Indeed even if the stored bit pattern for the 'NULL' pointer isn't all zero [1], then the literal 0 denotates a NULL pointer. So: char *p = 0; if (*(int *)&p != 0) printf(...); can call printf. It is also valid to '#define NULL (void *)0' that value is compatible with function pointers - even though a function pointer can't be assigned to a 'void *' variable. David [1] I don't know of any such C implementation, but I some mainframe OS have used the all 1 bit pattern for illegal pointers. -- David Laight: david(a)l8s.co.uk
participants (7)
-
Alexandre Julliard -
Andreas Mohr -
David Laight -
Dimitrie O. Paun -
Dmitry Timoshkov -
Ferenc Wagner -
Shachar Shemesh