On Sun, Sep 7, 2008 at 5:59 AM, Reece Dunn msclrhd@googlemail.com wrote:
This fixes these crypt32:chain testGetCertChain tests on Vista and Windows 2008.
- ok(!ret && GetLastError() == ERROR_INVALID_DATA, - "Expected ERROR_INVALID_DATA, got %d\n", GetLastError()); + ok(!ret, "Expected CertGetCertificateChain to fail\n"); + ok(GetLastError() == ERROR_INVALID_DATA || + GetLastError() == CRYPT_E_ASN1_BADTAG /* Vista/win2k8 */,
I'm not NACK'ing this patch, but for future reference, please put comments *after* the comma or logical OR marker, and each system condition should be on its own line. I've seen this in several patches recently. While it's syntactically OK, it looks wrong and there's no need for everyone to have to play Find The Comma. Yes there are other places in the code where this happens, but that doesn't make it correct or preferable. For example, the following is significantly easier to read:
ok(result == ERROR_BUFFER_TOO_SMALL || result == ERROR_INVALID_USER_BUFFER || /* win98 */ result == ERROR_INVALID_DATA, /* Vista */ "Expected ERROR_BUFFER_TOO_SMALL, got %08d\n", result);
than
ok(result == ERROR_BUFFER_TOO_SMALL || result == ERROR_INVALID_USER_BUFFER /* win98 */ || result == ERROR_INVALID_DATA /* Vista */, "Expected ERROR_BUFFER_TOO_SMALL, got %08d\n", result);
Am 08.09.2008 um 01:30 schrieb James Hawkins:
For example, the following is significantly easier to read:
ok(result == ERROR_BUFFER_TOO_SMALL || result == ERROR_INVALID_USER_BUFFER || /* win98 */ result == ERROR_INVALID_DATA, /* Vista */ "Expected ERROR_BUFFER_TOO_SMALL, got %08d\n", result);
Wouldn't it be even better to print the expected number in the same format as the actually received result?
... "Expected ERROR_BUFFER_TOO_SMALL (%08d), got %08d\n", ERROR_BUFFER_TOO_SMALL, result);
This should ease log reading as well.
MarKus
On Mon, Sep 8, 2008 at 4:38 AM, Markus Hitter mah@jump-ing.de wrote:
Am 08.09.2008 um 01:30 schrieb James Hawkins:
For example, the following is significantly easier to read:
ok(result == ERROR_BUFFER_TOO_SMALL || result == ERROR_INVALID_USER_BUFFER || /* win98 */ result == ERROR_INVALID_DATA, /* Vista */ "Expected ERROR_BUFFER_TOO_SMALL, got %08d\n", result);
Wouldn't it be even better to print the expected number in the same format as the actually received result?
Yes, the %08d should just be %d....I just copied the first example I could find in my git log.
... "Expected ERROR_BUFFER_TOO_SMALL (%08d), got %08d\n", ERROR_BUFFER_TOO_SMALL, result);This should ease log reading as well.
I don't see how that would help. The decimal values of error constants aren't related in a meaningful way. If you want to know the decimal value of ERROR_BUFFER_TOO_SMALL, grep include/winerror.h.
On Mon, Sep 8, 2008 at 4:41 AM, James Hawkins truiken@gmail.com wrote:
On Mon, Sep 8, 2008 at 4:38 AM, Markus Hitter mah@jump-ing.de wrote:
Am 08.09.2008 um 01:30 schrieb James Hawkins:
For example, the following is significantly easier to read:
ok(result == ERROR_BUFFER_TOO_SMALL || result == ERROR_INVALID_USER_BUFFER || /* win98 */ result == ERROR_INVALID_DATA, /* Vista */ "Expected ERROR_BUFFER_TOO_SMALL, got %08d\n", result);
Wouldn't it be even better to print the expected number in the same format as the actually received result?
Yes, the %08d should just be %d....I just copied the first example I could find in my git log.
I'm sorry, I thought you were asking about the %08d i.e. the format of the value. My answer to your actual question is no as explained in my previous email.
-
James Hawkins -
Markus Hitter