re: msi: Fix some copy/paste bugs in the implementation of condition operators.
Hans wrote:
[http://www.winehq.org/pipermail/wine-cvs/2006-June/023395.html] Fix some copy/paste bugs in the implementation of condition operators.
Congratulations on getting your patch into cvs! How far does this go towards fixing America's Army (http://bugs.winehq.org/show_bug.cgi?id=5139)? Also, are you still working on a conformance test for this, or did you stop once you found the obvious problem? - Dan -- Wine for Windows ISVs: http://kegel.com/wine/isv
obviously my patch wasn't "wrong" as you so put it, as this is the same thing as the one I submitted. I just phrased the definition of it wrong assuming the conditional statements were used by SQL.
From: "Dan Kegel" <dank(a)kegel.com> To: wine-devel <wine-devel(a)winehq.org>, "Hans Leidekker" <hans(a)it.vu.nl> CC: "EA Durbin" <ead1234(a)hotmail.com> Subject: re: msi: Fix some copy/paste bugs in the implementation of condition operators. Date: Mon, 5 Jun 2006 08:07:17 -0700
Hans wrote:
[http://www.winehq.org/pipermail/wine-cvs/2006-June/023395.html] Fix some copy/paste bugs in the implementation of condition operators.
Congratulations on getting your patch into cvs!
How far does this go towards fixing America's Army (http://bugs.winehq.org/show_bug.cgi?id=5139)?
Also, are you still working on a conformance test for this, or did you stop once you found the obvious problem? - Dan
-- Wine for Windows ISVs: http://kegel.com/wine/isv
EA Durbin wrote:
obviously my patch wasn't "wrong" as you so put it, as this is the same thing as the one I submitted. I just phrased the definition of it wrong assuming the conditional statements were used by SQL.
Your patch caused the regression tests to fail, and Hans' did not. err:msi:msi_get_dialog_record query failed for dialog L"boo" fixme:msi:ACTION_PerformUIAction unhandled msi action L"boo" package.c:618: Test failed: wrong return val make[1]: *** [package.ok] Error 1 You seem to have a different theory of what's causing the failure every day. I'm somewhat disappointed that you refuse to write a regression test to show what you've found. It would give me confidence in what your analysis of the problem, assist me in fixing the problem, and prevent it from happening again. Mike
My analysis has not changed it still remains the same to this day. Case, america's army installer is working backward, not going through the media in the correct order. Upon troubleshooting this I found that this is due to the query returning the wrong results for LastSequence. I don't have an analysis for what is causing this yet. But this is what is causing the bug and I've never swayed on that. I did in fact write a test, which does prove this. Hans has a copy of it, and i think i sent it to wine-devel too. As for files.c, the query in ready_media_for_file() should be passing an unsinged integer as LastSequence must be greater than zero. http://search.msdn.microsoft.com/search/Redirect.aspx?title=Media+Table+%5bW...
less than or equal to the value in the LastSequence column, and greater than the LastSequence value of the previous disk (or greater than 0, for the first entry in the Media table).
From: Mike McCormack <mike(a)codeweavers.com> To: EA Durbin <ead1234(a)hotmail.com> CC: dank(a)kegel.com, wine-devel(a)winehq.org, hans(a)it.vu.nl Subject: Re: msi: Fix some copy/paste bugs in the implementation of condition operators. Date: Tue, 06 Jun 2006 00:24:00 +0900
EA Durbin wrote:
obviously my patch wasn't "wrong" as you so put it, as this is the same thing as the one I submitted. I just phrased the definition of it wrong assuming the conditional statements were used by SQL.
Your patch caused the regression tests to fail, and Hans' did not.
err:msi:msi_get_dialog_record query failed for dialog L"boo" fixme:msi:ACTION_PerformUIAction unhandled msi action L"boo" package.c:618: Test failed: wrong return val make[1]: *** [package.ok] Error 1
You seem to have a different theory of what's causing the failure every day.
I'm somewhat disappointed that you refuse to write a regression test to show what you've found. It would give me confidence in what your analysis of the problem, assist me in fixing the problem, and prevent it from happening again.
Mike
EA Durbin wrote:
I did in fact write a test, which does prove this. Hans has a copy of it, and i think i sent it to wine-devel too.
I haven't seen any patch for the Wine regression test suite as yet, and that is what I have been asking you to write.
As for files.c, the query in ready_media_for_file() should be passing an unsinged integer as LastSequence must be greater than zero.
You patch is wrong because the value you are printing is an INT, so using %d is correct, and using %u will print the wrong thing when the INT is less than zero. Mike
It doesn't use %d currently. It uses %i. And its not to print. It uses this value in the SQL statement. The LastSequence value of the Media table is NEVER negative as i pointed out in the MSDN link. It must always be zero or larger and this is handled by passing %u. The SELECT * FROM `Media` WHERE `LastSequence` >= %i, should be The SELECT * FROM `Media` WHERE `LastSequence` >= %u. In ready_media_for_files.
From: Mike McCormack <mike(a)codeweavers.com> To: EA Durbin <ead1234(a)hotmail.com> CC: dank(a)kegel.com, wine-devel(a)winehq.org, hans(a)it.vu.nl Subject: Re: msi: Fix some copy/paste bugs in the implementation of condition operators. Date: Tue, 06 Jun 2006 00:38:16 +0900
EA Durbin wrote:
I did in fact write a test, which does prove this. Hans has a copy of it, and i think i sent it to wine-devel too.
I haven't seen any patch for the Wine regression test suite as yet, and that is what I have been asking you to write.
As for files.c, the query in ready_media_for_file() should be passing an unsinged integer as LastSequence must be greater than zero.
You patch is wrong because the value you are printing is an INT, so using %d is correct, and using %u will print the wrong thing when the INT is less than zero.
Mike
Why would you add all your findings and "test cases" into the Wine's test suite? Until then you should stop flaming list. With all the energy you spent writing these e-mails BACKWARDS as I might add. And you outright IGNORED my suggestions on the IRC chat room. So until you show us some code that verifies what you are talking about, please stay away from the e-mail clinet. Vitaliy. Monday, June 5, 2006, 10:05:39 AM, EA Durbin wrote:
It doesn't use %d currently. It uses %i. And its not to print. It uses this value in the SQL statement. The LastSequence value of the Media table is NEVER negative as i pointed out in the MSDN link. It must always be zero or larger and this is handled by passing %u.
The SELECT * FROM `Media` WHERE `LastSequence` >= %i, should be The SELECT * FROM `Media` WHERE `LastSequence` >= %u. In ready_media_for_files.
From: Mike McCormack <mike(a)codeweavers.com> To: EA Durbin <ead1234(a)hotmail.com> CC: dank(a)kegel.com, wine-devel(a)winehq.org, hans(a)it.vu.nl Subject: Re: msi: Fix some copy/paste bugs in the implementation of condition operators. Date: Tue, 06 Jun 2006 00:38:16 +0900
EA Durbin wrote:
I did in fact write a test, which does prove this. Hans has a copy of it, and i think i sent it to wine-devel too.
I haven't seen any patch for the Wine regression test suite as yet, and that is what I have been asking you to write.
As for files.c, the query in ready_media_for_file() should be passing an unsinged integer as LastSequence must be greater than zero.
You patch is wrong because the value you are printing is an INT, so using %d is correct, and using %u will print the wrong thing when the INT is less than zero.
Mike
I'm not flaming, I'm trying to get assitance with the problem and sharing my findings with other developers. I openly admit im a novice C hacker, and could use some help which I thought was what the wine-devel mailing list was used for, as its for developers. As Robert Shearman stated, why give it a 4 byte value when it's using a 2 byte value. The sequence should never be a negative number, where am I mistaken on this?
From: Vitaliy Margolen <wine-devel(a)kievinfo.com> Reply-To: wine-devel(a)winehq.org To: "EA Durbin" <ead1234(a)hotmail.com> CC: wine-devel(a)winehq.org Subject: Re: msi: Fix some copy/paste bugs in the implementation of condition operators. Date: Mon, 5 Jun 2006 10:14:50 -0600
Why would you add all your findings and "test cases" into the Wine's test suite? Until then you should stop flaming list. With all the energy you spent writing these e-mails BACKWARDS as I might add.
Monday, June 5, 2006, 11:02:38 AM, EA Durbin wrote:
As Robert Shearman stated, why give it a 4 byte value when it's using a 2 byte value.
The sequence should never be a negative number, where am I mistaken on this?
Both of those depend on the type and not on what it "should be". And the type is INT it's a "signed int". Vitaliy. PS: PLEASE PLEASE PLEASE ADD YOU COMMENTS BELOW THIS LINE NOT ABOVE. IT'S IMPOSSIBLE TO READ.
Vitaliy.
PS: PLEASE PLEASE PLEASE ADD YOU COMMENTS BELOW THIS LINE NOT ABOVE. IT'S IMPOSSIBLE TO READ.
By default microsoft outlook, hotmail, all add their replies above the previous message. It's the way 90% of the email I see works.
Both of those depend on the type and not on what it "should be". And the type is INT it's a "signed int".
Why must it be a signed int, the comparison is always a positive number which is an unsigned int. So if we always pass a positive number it should still work correctly. Both types are integers, only the signed int requires 2 more bytes for the sign. Aren't we wasting 2 bytes of memory by using signed int?
Am Montag, den 05.06.2006, 14:01 -0500 schrieb EA Durbin:
And the type is INT it's a "signed int". Why must it be a signed int,
Welcome to the World of Microsoft. Another Example from MS: A Function is declared to return a BOOL and documented to return '0', '1' or '2' We are Compatible, so our Declaration is also a BOOL. -- By By ... ... Detlef
Monday, June 5, 2006, 1:01:10 PM, EA Durbin wrote:
Both of those depend on the type and not on what it "should be". And the type is INT it's a "signed int".
Why must it be a signed int, the comparison is always a positive number which is an unsigned int. So if we always pass a positive number it should still work correctly. No it will not work correctly. What if something _will_ pass a negative number? Then instead of having it as -1 you'll have 4294967295.
Can you 100% guarantee that documentation is correct? And that there are no broken apps that will try to pass negative number?
Both types are integers, only the signed int requires 2 more bytes for the sign.
Aren't we wasting 2 bytes of memory by using signed int? Wrong. You need to read some c books. But sign uses only 1 bit, not 2 bytes.
Vitaliy.
Can you 100% guarantee that documentation is correct? And that there are no broken apps that will try to pass negative number? Vitaliy. Okay, i guess I see your point, but if the broken app passes a negative number its not going to work correctly anyway, as it will read from the wrong Disk medium when it encounters the query, once that works correctly.
Le lundi 05 juin 2006 à 14:01 -0500, EA Durbin a écrit :
Vitaliy.
PS: PLEASE PLEASE PLEASE ADD YOU COMMENTS BELOW THIS LINE NOT ABOVE. IT'S IMPOSSIBLE TO READ.
By default microsoft outlook, hotmail, all add their replies above the previous message. It's the way 90% of the email I see works.
Maybe that's because 90% of the people you know are using bad mail clients or are using bad settings. You'll see some reasons why top-posting is a bad thing(tm) here : http://catb.org/~esr/jargon/html/T/top-post.html Regards. Jonathan
PS: PLEASE PLEASE PLEASE ADD YOU COMMENTS BELOW THIS LINE NOT ABOVE. IT'S IMPOSSIBLE TO READ.
By default microsoft outlook, hotmail, all add their replies above the previous message. It's the way 90% of the email I see works.
Just because billions of flies eat sh*t every day, so should we all do too? But seriously, does top posting make sense to you?! If you're replying to something, you either thread it into the discussion, editing and amending the cited context as appropriate, or you simply make everyone's life miserable. Try reading one of your posts, from later into the discussion where there's a lot of context. Do it in a few months -- you'll see what I mean. You'll have to read them backwards *yourself*, just in order to understand what it was all about. Cheers, Kuba
On Mon, 05 Jun 2006 10:14:50 -0600, Vitaliy Margolen wrote:
So until you show us some code that verifies what you are talking about, please stay away from the e-mail clinet.
He has done exactly that though not in a form we can put in the test suite, and he is not "flaming" anybody. I think you need to calm down a bit. thanks -mike
On Mon, 05 Jun 2006 10:14:50 -0600, Vitaliy Margolen wrote:
So until you show us some code that verifies what you are talking about, please stay away from the e-mail clinet.
He has done exactly that though not in a form we can put in the test suite, and he is not "flaming" anybody. I think you need to calm down a bit.
thanks -mike
I've decided to concentrate my efforts in another area, so I won't be looking at msi any longer. I admit I 'm no C expert, I'm a web developer by trade. Though I started to look at the C code to get away from php/perl/ruby and get aquainted with the language in my leisure time, I need no assitance with web development. As I would still like to contribute to the wine project I will be focusing my efforts on Appdb and Bugzilla. But thanks for understanding my intentions Mike.
On 6/6/06, EA Durbin <ead1234(a)hotmail.com> wrote:
I've decided to concentrate my efforts in another area, so I won't be looking at msi any longer. I admit I 'm no C expert, I'm a web developer by trade. Though I started to look at the C code to get away from php/perl/ruby and get aquainted with the language in my leisure time, I need no assitance with web development. As I would still like to contribute to the wine project I will be focusing my efforts on Appdb and Bugzilla.
Well, that's great too but I don't want to see you give up! :) I know this particular adventure hasn't gone quite right but there's plenty of areas you can contribute. When I started I was no C expert either but it's a simple language and can be quickly learned. Often this happens, you debug something to find that it's in an area not many people can work on. If you want my advice, it'd be to just try your hand at debugging random apps. Find one that doesn't work, poke it a bit to find out why not, if you aren't getting anywhere throw it away and move onto the next. This lets you see a lot of the code quickly, and you'll probably find that some bugs are much easier to fix than the one you found. thanks -mike
EA Durbin wrote:
It doesn't use %d currently. It uses %i. And its not to print. It uses this value in the SQL statement. The LastSequence value of the Media table is NEVER negative as i pointed out in the MSDN link. It must always be zero or larger and this is handled by passing %u.
Unfortunately your lack of understanding of C lets you down here. Mike
Mike McCormack wrote:
EA Durbin wrote:
It doesn't use %d currently. It uses %i. And its not to print. It uses this value in the SQL statement. The LastSequence value of the Media table is NEVER negative as i pointed out in the MSDN link. It must always be zero or larger and this is handled by passing %u.
Unfortunately your lack of understanding of C lets you down here.
Is there any guarantee that the value will be a 2-byte integer rather than a 4-byte integer? In that case, it will make a difference. BTW, I suspect that the America's Army problem is caused by the installer using a 4-byte integer in place of the usual 2-byte integer, which is parsed slightly differently. EA Durbin, if you want to test this theory you can instrument INT_evaluate in dlls/msi/where.c and dump the two values. -- Rob Shearman
Okay what am i misunderstanding?, explain it to me as its imperative I learn, and I'd love to learn. %u is an unsigned integer which is 0 to +32,767. %i is a signed integer 32,767 to +32,767. If the sequence number is always going to be a positive number why should we allot it the extra 32,767 value range?
From: Mike McCormack <mike(a)codeweavers.com> To: EA Durbin <ead1234(a)hotmail.com> CC: dank(a)kegel.com, wine-devel(a)winehq.org, hans(a)it.vu.nl Subject: Re: msi: Fix some copy/paste bugs in the implementation of condition operators. Date: Tue, 06 Jun 2006 01:34:05 +0900
EA Durbin wrote:
It doesn't use %d currently. It uses %i. And its not to print. It uses this value in the SQL statement. The LastSequence value of the Media table is NEVER negative as i pointed out in the MSDN link. It must always be zero or larger and this is handled by passing %u.
Unfortunately your lack of understanding of C lets you down here.
Mike
On Monday, June 05, 2006 12:58, EA Durbin wrote:
Okay what am i misunderstanding?, explain it to me as its imperative I learn, and I'd love to learn.
%u is an unsigned integer which is 0 to +32,767.
%i is a signed integer 32,767 to +32,767.
If the sequence number is always going to be a positive number why should we allot it the extra 32,767 value range?
Not quite... neil(a)t40-n ~ $ cat >tmp.c <<EOF #include <stdint.h> #include <stdio.h> int main(void) { uint16_t i = -1; printf("%u\n", i); return 0; } EOF neil(a)t40-n ~ $ gcc tmp.c neil(a)t40-n ~ $ ./a.out 65535 neil(a)t40-n ~ $ if you inspect the memory that's at i, you'll find it's 0xffff. If you read it as signed, you interpret it using two's complement[1], if you read it as unsigned, you still use all the bits, but there's no sign bit*. [1] http://en.wikipedia.org/wiki/Two's_complement * Strictly speaking it's not a sign bit, but is frequently referred to as one anyways. - Neil
EA Durbin wrote:
Okay what am i misunderstanding?, explain it to me as its imperative I learn, and I'd love to learn.
%u is an unsigned integer which is 0 to +32,767.
%i is a signed integer –32,767 to +32,767.
If the sequence number is always going to be a positive number why should we allot it the extra 32,767 value range?
"A signed int can hold all the values between *INT_MIN* and *INT_MAX* inclusive. *INT_MIN* is required to be -32767 or less, *INT_MAX* must be at least 32767. Again, many 2's complement implementations will define *INT_MIN* to be -32768 but this is not required. An unsigned int can hold all the values between 0 and *UINT_MAX * inclusive. *UINT_MAX* must be at least 65535. The int types must contain *at least* 16 bits to hold the required range of values. *NOTE:* /The required ranges for signed and unsigned int are identical to those for signed and unsigned short. On compilers for 8 and 16 bit processors (including Intel x86 processors executing in 16 bit mode, such as under MS-DOS), an int is usually 16 bits and has exactly the same representation as a short. On compilers for 32 bit and larger processors (including Intel x86 processors executing in 32 bit mode, such as Win32 or Linux) an int is usually 32 bits long and has exactly the same representation as a long."/ AFAIK, all signed and unsigned versions of the same type actually use the same number of bits, it's just that by using one of those bits as the 'sign' bit, the signed version seems to only be able to hold values half as big as the unsigned version (in fact, they're both capable of storing the same number of unique values, it's just that for the signed version half of the 65535 values are below zero). As a tip to remember this, consider for example an unsigned char. A char is just a byte, which is the smallest unit of memory most computers can address. An 'unsigned' char wouldn't yield any space savings because the minimum you can allocate at once is a byte anyway (in fact on most modern systems much larger than that, a page, but C is abstracted away from that). So unsigned/signed types must take up the same amount of memory. (we could concoct our own language where unsigned ints are actually shorts, but we'd be going out of our way)
EA Durbin wrote:
Okay what am i misunderstanding?, explain it to me as its imperative I learn, and I'd love to learn.
%u is an unsigned integer which is 0 to +32,767.
%u is an unsigned integer which is 0 to +65535.
%i is a signed integer –32,767 to +32,767.
%i is a signed integer -32768 to +32767.
participants (12)
-
Dan Kegel -
Detlef Riekenberg -
EA Durbin -
Jakob Eriksson -
Jonathan Ernst -
Joseph Garvin -
Kuba Ober -
Mike Hearn -
Mike McCormack -
Neil Skrypuch -
Robert Shearman -
Vitaliy Margolen