Re: wintrust: Sign-compare warnings fix
Juan Lang wrote:
> Hi Andy,
>
> -if (pbEncoded[1] + 1 > cbEncoded)
> +if (pbEncoded[1] + 1U > cbEncoded)
>
> Is this change necessary? The resulting code is less clear than the
> original, IMO. It's clearly a spurious warning: a BYTE (max value
> 255) + 1 can't yield a value that overflows an unsigned int, so this
> comparison will always do what's wanted.
>
> Same with the change here:
> -else if (lenLen + 2 > cbEncoded)
> +else if (lenLen + 2U > cbEncoded)
>
> Otherwise, this patch looks fine to me.
> --Juan
Hi Juan,
I was curious to see how this one would fly. I fully take your point, of
course. If it were a good idea, the point would be to reduce the noise when
looking for real sign-compare problems and without introducing a cast. In a
similar vein, quite a lot of warnings are generated by code like this
if (dwSomething == -1)
{
...
Maybe, in this case the code actually is "wrong", since a variable that
operates in the range 0 to UINT_MAX (on a non-MSC system) can never be
strictly equal to -1, but in these cases I have resisted comparing against
"~0U", because there may be places where an internal function could/should
be rewritten not to use "minus one unsigned" as a return value, for example,
and this "fix" would tend to hide those opportunities.
I shall see how the patch fares and resubmit without the "U"s if it gets
rejected.
Thanks for the feedback.
--
Andy.
compiling Windows code with g++ on Linux using msvcrt - good idea? if so, how do you do it?
Hi, I have a huge amount of Windows code that I'm porting to Linux. Wine is turning out to be a read godsend, thank you guys! Anyway, I've had tons of luck including the directory /include/wine/windows in my include path. All my Windows types are there and everything is wonderful. I've even been able to link against the libraries by renaming, for example, foo.dll.so to libfoo.so and using -lfoo on the g++ command line. I actually haven't got all my functions defined so I don't know if this will actually run yet because the link isn't complete yet. My first question is whether this will work. Will all my Windows functions with function declarations defined in the windows directory and code compiled into foo.dll.so actually run? Second, I see that some of the functions I want to use are actually defined by headers in the msvcrt directory. However, this directory contains tons of header files that my system (and gcc) also have under /usr/include and gcc's own directories. I understand that msvcrt is Windows lib c, but I wonder how I'm supposed to use it with gcc to compile? I've tried -nostdinc to gcc, but the number of errors I get is enormous. It's not the end of the world if I can't use Wine's msvcrt, but if I could use it, it would define dozens more functions on which my program relies, or if its use was mandatory in order to get the functions implemented in foo.dll.so to work, then obviously I wonder how I'm supposed to compile my Windows code with Wine to make everything work? I'm sorry if this is a dumb question. I can't find a way to search the wine-devel mailing list other than with google and I can't find anything on there or other docs that answer this question, which I'm sure has been asked a million times and has a simple answer. Thank you thank you, Jeff _ Send e-mail faster without improving your typing skills. http://windowslive.com/Explore/hotmail?ocid=TXT_TAGLM_WL_hotmail_acq_speed_122008
[no subject]
Hi, I have a huge amount of Windows code that I'm porting to Linux. Wine is turning out to be a read godsend, thank you guys! Anyway, I've had tons of luck including the directory /include/wine/windows in my include path. All my Windows types are there and everything is wonderful. I've even been able to link against the libraries by renaming, for example, foo.dll.so to libfoo.so and using -lfoo on the g++ command line. I actually haven't got all my functions defined so I don't know if this will actually run yet because the link isn't complete yet. My first question is whether this will work. Will all my Windows functions with function declarations defined in the windows directory and code compiled into foo.dll.so actually run? Second, I see that some of the functions I want to use are actually defined by headers in the msvcrt directory. However, this directory contains tons of header files that my system (and gcc) also have under /usr/include and gcc's own directories. I understand that msvcrt is Windows lib c, but I wonder how I'm supposed to use it with gcc to compile? I've tried -nostdinc to gcc, but the number of errors I get is enormous. It's not the end of the world if I can't use Wine's msvcrt, but if I could use it, it would define dozens more functions on which my program relies, or if its use was mandatory in order to get the functions implemented in foo.dll.so to work, then obviously I wonder how I'm supposed to compile my Windows code with Wine to make everything work? I'm sorry if this is a dumb question. I can't find a way to search the wine-devel mailing list other than with google and I can't find anything on there or other docs that answer this question, which I'm sure has been asked a million times and has a simple answer. Thank you thank you, Jeff _ Send e-mail faster without improving your typing skills. http://windowslive.com/Explore/hotmail?ocid=TXT_TAGLM_WL_hotmail_acq_speed_122008
Re: wintrust: Sign-compare warnings fix
Hi Andy, -if (pbEncoded[1] + 1 > cbEncoded) +if (pbEncoded[1] + 1U > cbEncoded) Is this change necessary? The resulting code is less clear than the original, IMO. It's clearly a spurious warning: a BYTE (max value 255) + 1 can't yield a value that overflows an unsigned int, so this comparison will always do what's wanted. Same with the change here: -else if (lenLen + 2 > cbEncoded) +else if (lenLen + 2U > cbEncoded) Otherwise, this patch looks fine to me. --Juan
Re: Canonical and wine
Canonical doesn't want to include Wine, because they are trying to provide a complete desktop experience. Wine is a necessity for many people, but Canonical wants to market Ubuntu as the Linux distribution that works well for normal usage. Including a half-working Windows-emulator (functionality emulator) doesn't fall within that mission. It gives the message that Ubuntu on its own isn't ready (true or not). Why would they want to switch from a good product (Windows) to an inferior one with spotty support for Windows applications? I don't see them providing packages for proprietary applications. Not for the Ubuntu project, which is completely free (except drivers). Maybe through the Canonical store, which also provides a legal but proprietary DVD player. Remco
Re: Translating to winehq.org
One at a time is fine. As you can see from our Spanish translation, it does not need to be complete. The site will fall back to english when it can't find the template. I would start with home.template and branch out from there. If you see obsolete content, you can send patches to update the english as well. This will be a challenge, but luckily most of the content does not change often. -Newman Maik Schulz wrote: > Hi, > > I just started translating the new winehq.org to German. How would you > like to receive the git patches for the translated template files? One > at a time whenever I have them ready, or at the end in one batch? > > Also, what about obsolete content? E.g. contributing.template still > lists Wine 1.x tasks (with a link to an empty Bugzilla list) as well > as an empty Wine 0.9.x tasks list. > > Cheers, > -Maik > >
Translating to winehq.org
Hi, I just started translating the new winehq.org to German. How would you like to receive the git patches for the translated template files? One at a time whenever I have them ready, or at the end in one batch? Also, what about obsolete content? E.g. contributing.template still lists Wine 1.x tasks (with a link to an empty Bugzilla list) as well as an empty Wine 0.9.x tasks list. Cheers, -Maik
Re : RFC: Wine Icons
> On Friday 14 November 2008 19:13:32 Juan Lang wrote: > > > > I find that a bit alarming. I'm sure he's working very hard, and doing > > > good stuff, but I don't think Wine should be redrawing anything. Not > > > when we have Tango around - it's designed to try create some consistency > > > through standardisation. IMO standards are really good! - we use them if > > > we possibly can. > > > > The trouble is that the Tango icon set doesn't cover all the icons > > Wine needs. There's no Tango icon for regedit, for instance. > > IIRC there's Tango icon sets for specific applications. If we're doing our > own > icons anyway, we could try to keep them coherent with the rest of the Tango > look and get the icons upstream to Tango. > > This seems like the most reasonable approach. Of course the only things I > usually paint are walls so I'd have no idea how easy it is to match the Tango > specs when designing icons. > > Cheers, > Kai First of all, sorry to not have answered before and for my poor English, but I was quite busy these past months and I didn't see all these messages since today. I realize that the two patch I sent an hour ago are not the best answer to all these issues (I think these two newer icons look nicer in the open dialog boxes anyway) and that all theses points have to be discussed. When remaking icons, I tried to add a bit of consistency while improving a bit the icons. Obviously, I missed something. Reading all theses remarks, I agree that a better way to handle these icons are to use the Tango ones when they are available. As it was stated, including them is not straightforward and I haven't the skill to work on including these libraries into the wine tree. I hope somebody will work on these issues. In the meanwhile, maybe I'll try to improve the others shell32 icons as the bin or the computer which I found awful in this size but I don't know if it's a good idea. Cheers, Hervé
Re: Canonical and wine
> On Wednesday 10 December 2008 19:09:16 Dan Kegel wrote: > http://www.technewsworld.com/rsstory/65431.html > quotes Gerry Carr, marketing manager at Canonical: > "We aren't considering a pitch about using Wine or Parallels like on a > Mac. There is no real look at Wine. It doesn't always work well. So > this won't win over users to the benefits of Linux," > > So there you go. Canonical will think about Wine once it always works > well. I agree with Canonical that perhaps it doesn't make sense to make a Winebuntu or a new Ubuntu with Wine as a bigger focus for exactly that reason, it doesn't work for everything and that isn't a great experience. (Anything thats inconsistent is bad for something like Ubuntu. We love Wine but fact is fact, it isn't univerally perfect so they can't advertise 'Full Windows Application Support!' etc. etc. They could advertise 'Will Work with Some Windows Apps!' but then users ask which ones and it gets confusing from there and yada yada...) However perhaps it does make more sense to think about using Wine for application specific scenarios. I believe it has been proposed before to have .debs for things like Adobe Photoshop which first install Wine (or create a new prefix etc.) and then ask for the Photoshop CD; sort of like application specific bundlings of Wine. Do we know Canonical's stance on that? That scenario could be consistent and provide real end user benefit sans confusion.
Re: Canonical and wine
On Wednesday 10 December 2008 19:09:16 Dan Kegel wrote: > http://www.technewsworld.com/rsstory/65431.html > quotes Gerry Carr, marketing manager at Canonical: > "We aren't considering a pitch about using Wine or Parallels like on a > Mac. There is no real look at Wine. It doesn't always work well. So > this won't win over users to the benefits of Linux," > > So there you go. Canonical will think about Wine once it always works > well. Sure. because only 'hardcore Linux users' use Wine. That's just what I see every day when I look into #winehq. Hardcore Linux users who never need help with the basic questions of compiling Wine from source or updating to the latest Wine release, but who are stopped from running their games^Wprograms by Wine. Excuse me while I rush out to catch one of these flying pigs, Kai -- Kai Blin WorldForge developer http://www.worldforge.org/ Wine developerhttp://wiki.winehq.org/KaiBlin Samba team member http://www.samba.org/samba/team/ -- Will code for cotton. signature.asc Description: This is a digitally signed message part.
Canonical and wine
http://www.technewsworld.com/rsstory/65431.html quotes Gerry Carr, marketing manager at Canonical: "We aren't considering a pitch about using Wine or Parallels like on a Mac. There is no real look at Wine. It doesn't always work well. So this won't win over users to the benefits of Linux," So there you go. Canonical will think about Wine once it always works well.
Re: wininet/tests: Use new test URLs on test.winehq.org.
Hi Alexandre, These changes introduce 15 errors on all platforms. -- Cheers, Paul.
Re: [PATCH 14/14] msi: Add tests for MsiGetProductProperty.
On Wed, Dec 10, 2008 at 8:49 AM, Paul Vriens <[EMAIL PROTECTED]> wrote:
> James Hawkins wrote:
>>
>> ---
>> dlls/msi/tests/package.c | 232
>> ++
>> 1 files changed, 232 insertions(+), 0 deletions(-)
>>
>>
>>
>>
>>
> Hi James,
>
> These fail on Win9x as we are relying on some registry keys that are NT+
> specific:
>
> 9574 lstrcpyA(keypath,
> "Software\\Microsoft\\Windows\\CurrentVersion\\");
> 9575 lstrcatA(keypath, "Installer\\UserData\\S-1-5-18\\Products\\");
> 9576 lstrcatA(keypath, prod_squashed);
>
> Can we skip these tests, like we do with some others:
>
> scm = OpenSCManager(NULL, NULL, GENERIC_ALL);
> if (!scm && (GetLastError() == ERROR_CALL_NOT_IMPLEMENTED))
> {
>win_skip("Different registry keys on Win9x and WinMe\n");
>return;
> }
>
Doesn't matter to me.
--
James Hawkins
Re: [PATCH 14/14] msi: Add tests for MsiGetProductProperty.
James Hawkins wrote:
> ---
> dlls/msi/tests/package.c | 232
> ++
> 1 files changed, 232 insertions(+), 0 deletions(-)
>
>
>
>
>
Hi James,
These fail on Win9x as we are relying on some registry keys that are NT+
specific:
9574 lstrcpyA(keypath, "Software\\Microsoft\\Windows\\CurrentVersion\\");
9575 lstrcatA(keypath, "Installer\\UserData\\S-1-5-18\\Products\\");
9576 lstrcatA(keypath, prod_squashed);
Can we skip these tests, like we do with some others:
scm = OpenSCManager(NULL, NULL, GENERIC_ALL);
if (!scm && (GetLastError() == ERROR_CALL_NOT_IMPLEMENTED))
{
win_skip("Different registry keys on Win9x and WinMe\n");
return;
}
--
Cheers,
Paul.
Re: [5/8] jscript: Implement the String.anchor() method.
Hi Andrew,
Andrew Nguyen wrote:
> ---
> dlls/jscript/string.c | 58 ++
> dlls/jscript/tests/api.js | 17 +
> 2 files changed, 73 insertions(+), 2 deletions(-)
>
+static HRESULT do_attribute_tag_format(DispatchEx *dispex, LCID lcid, WORD
flags, DISPPARAMS *dp,
+VARIANT *retv, jsexcept_t *ei, IServiceProvider *sp, const WCHAR
*tagname, const WCHAR *attrname)
+{
+static const WCHAR attrtagfmtW[] = {'<','%','s','
','%','s','=','"','%','s','"','>','%','s','<','/','%','s','>',0};
+static const WCHAR undefinedW[] = {'u','n','d','e','f','i','n','e','d',0};
+StringInstance *string;
+BSTR ret;
+BSTR attrval = NULL;
+DWORD len;
+HRESULT hres = S_OK;
+
+TRACE("\n");
IMO it would be better to change FIXME to TRACE in String_anchor instead of
adding TRACE here. You use this function in a few places, and this TRACE
doesn't tell us, which function is called.
+
+if(!is_class(dispex, JSCLASS_STRING)) {
+WARN("this is not a string object\n");
+hres = E_NOTIMPL;
+goto cleanup;
You may just return E_NOTIMPL here.
+}
+
+string = (StringInstance*)dispex;
+
+if(retv) {
+if(arg_cnt(dp)) {
+hres = to_string(dispex->ctx, get_arg(dp, 0), ei, &attrval);
You should call to_string even if retv is NULL. Although it looks like a nice
optimization, we can't do this. Eg, if to_string throws exception, we have to
throw it here.
+if (FAILED(hres))
+goto cleanup;
You may just return hres here.
+}
+
+len = string->length + 2*strlenW(tagname) + strlenW(attrname) + 9;
+
+if (attrval)
+len += SysStringLen(attrval);
+else
+len += sizeof(undefinedW)/sizeof(WCHAR) - 1;
+
+ret = SysAllocStringLen(NULL, len);
+
+if(!ret)
+{
+hres = E_OUTOFMEMORY;
+goto cleanup;
Perhaps it's just me, but in cases that don't make the code cleaner by using
gotos, I'd avoid them. How about:
if(ret) {
sprintfW(ret, attrtagfmtW, tagname, attrname, (attrval ? attrval :
undefinedW), string->str, tagname);
V_VT(retv) = VT_BSTR;
V_BSTR(retv) = ret;
}else {
hres = E_OUTOFMEMORY;
}
+}
+
+sprintfW(ret, attrtagfmtW, tagname, attrname, (attrval ? attrval :
undefinedW), string->str, tagname);
+
+V_VT(retv) = VT_BSTR;
+V_BSTR(retv) = ret;
+}
+ cleanup:
+SysFreeString(attrval);
+return hres;
+}
Re: [5/8] jscript: Implement the String.anchor() method.
Hi Andrew,
Andrew Nguyen wrote:
> ---
> dlls/jscript/string.c | 58 ++
> dlls/jscript/tests/api.js | 17 +
> 2 files changed, 73 insertions(+), 2 deletions(-)
>
+static HRESULT do_attribute_tag_format(DispatchEx *dispex, LCID lcid, WORD
flags, DISPPARAMS *dp,
+VARIANT *retv, jsexcept_t *ei, IServiceProvider *sp, const WCHAR
*tagname, const WCHAR *attrname)
+{
+static const WCHAR attrtagfmtW[] = {'<','%','s','
','%','s','=','"','%','s','"','>','%','s','<','/','%','s','>',0};
+static const WCHAR undefinedW[] = {'u','n','d','e','f','i','n','e','d',0};
+StringInstance *string;
+BSTR ret;
+BSTR attrval = NULL;
+DWORD len;
+HRESULT hres = S_OK;
+
+TRACE("\n");
IMO it would be better to change FIXME to TRACE in String_anchor instead of
adding TRACE here. You use this function in a few places, and this TRACE
doesn't tell us, which function is called.
+
+if(!is_class(dispex, JSCLASS_STRING)) {
+WARN("this is not a string object\n");
+hres = E_NOTIMPL;
+goto cleanup;
You may just return E_NOTIMPL here.
+}
+
+string = (StringInstance*)dispex;
+
+if(retv) {
+if(arg_cnt(dp)) {
+hres = to_string(dispex->ctx, get_arg(dp, 0), ei, &attrval);
You should call to_string even if retv is NULL. Although it looks like a nice
optimization, we can't do this. Eg, if to_string throws exception, we have to
throw it here.
+if (FAILED(hres))
+goto cleanup;
You may just return hres here.
+}
+
+len = string->length + 2*strlenW(tagname) + strlenW(attrname) + 9;
+
+if (attrval)
+len += SysStringLen(attrval);
+else
+len += sizeof(undefinedW)/sizeof(WCHAR) - 1;
+
+ret = SysAllocStringLen(NULL, len);
+
+if(!ret)
+{
+hres = E_OUTOFMEMORY;
+goto cleanup;
Perhaps it's just me, but in cases that don't make the code cleaner by using
gotos, I'd avoid them. How about:
if(ret) {
sprintfW(ret, attrtagfmtW, tagname, attrname, (attrval ? attrval :
undefinedW), string->str, tagname);
V_VT(retv) = VT_BSTR;
V_BSTR(retv) = ret;
}else {
hres = E_OUTOFMEMORY;
}
+}
+
+sprintfW(ret, attrtagfmtW, tagname, attrname, (attrval ? attrval :
undefinedW), string->str, tagname);
+
+V_VT(retv) = VT_BSTR;
+V_BSTR(retv) = ret;
+}
+ cleanup:
+SysFreeString(attrval);
+return hres;
+}
Jacek
Re: oleaut32: added PICTYPE_NONE and PICTYPE_UNINITIALIZED to IPicture::Render with test (try3)
Paul Vriens wrote:
> Nikolay Sivov wrote:
>> Changelog(try3):
>> - fixed identation in testlist
>> Changelog(try2):
>> - added PICTYPE_NONE and PICTYPE_UNINITIALIZED to
>> IPicture::Render with test
>> - added test for zero dimensions
>>
>> P.S. Render with this types spotted in traces of bugs 6799 and 10050
>>
>>> From 3386e8674380390c3b60c6247443c605ffe6ff6a Mon Sep 17 00:00:00 2001
>> From: Nikolay Sivov <[EMAIL PROTECTED]>
>> Date: Wed, 10 Dec 2008 13:41:09 +0300
>> Subject: added PICTYPE_NONE and PICTYPE_UNINITIALIZED to
>> IPicture::Render
>>
>> ---
>> dlls/oleaut32/olepicture.c |8 +
>> dlls/oleaut32/tests/olepicture.c | 64
>> ++
>> 2 files changed, 72 insertions(+), 0 deletions(-)
>>
>> diff --git a/dlls/oleaut32/olepicture.c b/dlls/oleaut32/olepicture.c
>> index 498ee53..bdce729 100644
>> --- a/dlls/oleaut32/olepicture.c
>> +++ b/dlls/oleaut32/olepicture.c
>> @@ -637,6 +637,10 @@ static HRESULT WINAPI
>> OLEPictureImpl_Render(IPicture *iface, HDC hdc,
>> TRACE("prcWBounds (%d,%d) - (%d,%d)\n", prcWBounds->left,
>> prcWBounds->top,
>>prcWBounds->right, prcWBounds->bottom);
>>
>> + if(cx == 0 || cy == 0 || cxSrc == 0 || cySrc == 0){
>> +return CTL_E_INVALIDPROPERTYVALUE;
>> + }
>> +
>>/*
>> * While the documentation suggests this to be here (or after
>> rendering?)
>> * it does cause an endless recursion in my sample app. -MM 20010804
>> @@ -644,6 +648,10 @@ static HRESULT WINAPI
>> OLEPictureImpl_Render(IPicture *iface, HDC hdc,
>> */
>>
>>switch(This->desc.picType) {
>> + case PICTYPE_UNINITIALIZED:
>> + case PICTYPE_NONE:
>> +/* nothing to do */
>> +return S_OK;
>>case PICTYPE_BITMAP:
>> {
>>HBITMAP hbmpOld;
>> diff --git a/dlls/oleaut32/tests/olepicture.c
>> b/dlls/oleaut32/tests/olepicture.c
>> index f81c419..9e33121 100644
>> --- a/dlls/oleaut32/tests/olepicture.c
>> +++ b/dlls/oleaut32/tests/olepicture.c
>> @@ -25,6 +25,7 @@
>> #include
>>
>> #define COBJMACROS
>> +#define NONAMELESSUNION
>>
>> #include "wine/test.h"
>> #include
>> @@ -587,6 +588,68 @@ static void test_enhmetafile(void)
>> GlobalFree(hglob);
>> }
>>
>> +static void test_Render(void)
>> +{
>> +IPicture *pic;
>> +HRESULT hres;
>> +short type;
>> +PICTDESC desc;
>> +HDC hdc = GetDC(0);
>> +
>> +/* test IPicture::Render return code on uninitialized picture */
>> +OleCreatePictureIndirect(NULL, &IID_IPicture, TRUE, (VOID**)&pic);
>> +hres = IPicture_get_Type(pic, &type);
>> +ok(hres == S_OK, "IPicture_get_Type does not return S_OK, but
>> 0x%08x\n", hres);
>> +ok(type == PICTYPE_UNINITIALIZED, "Expected type =
>> PICTYPE_UNINITIALIZED, got = %d\n", type);
>> +/* zero dimensions */
>> +hres = IPicture_Render(pic, hdc, 0, 0, 0, 0, 0, 0, 0, 0, NULL);
>> +ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE);
>> +hres = IPicture_Render(pic, hdc, 0, 0, 10, 10, 0, 0, 10, 0, NULL);
>> +ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE);
>> +hres = IPicture_Render(pic, hdc, 0, 0, 10, 10, 0, 0, 0, 10, NULL);
>> +ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE);
>> +hres = IPicture_Render(pic, hdc, 0, 0, 10, 10, 0, 0, 0, 0, NULL);
>> +ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE);
>> +hres = IPicture_Render(pic, hdc, 0, 0, 0, 10, 0, 0, 10, 10, NULL);
>> +ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE);
>> +hres = IPicture_Render(pic, hdc, 0, 0, 10, 0, 0, 0, 10, 10, NULL);
>> +ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE);
>> +hres = IPicture_Render(pic, hdc, 0, 0, 0, 0, 0, 0, 10, 10, NULL);
>> +ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE);
>> +/* nonzero dimensions, PICTYPE_UNINITIALIZED */
>> +hres = IPicture_Render(pic, hdc, 0, 0, 10, 10, 0, 0, 10, 10, NULL);
>> +ole_expect(hres, S_OK);
>> +IPicture_Release(pic);
>> +
>> +desc.cbSizeofstruct = sizeof(PICTDESC);
>> +desc.picType = PICTYPE_ICON;
>> +desc.u.icon.hicon = LoadIcon(NULL, IDI_APPLICATION);
>> +if(!desc.u.icon.hicon){
>> +win_skip("LoadIcon failed. Skipping...\n");
>
> Shouldn't you also use ReleaseDC() here for consistency sake?
>
>> +return;
>> +}
>> +
>> +OleCreatePictureIndirect(&desc, &IID_IPicture, TRUE, (VOID**)&pic);
>> +/* zero dimensions, PICTYPE_ICON */
>> +hres = IPicture_Render(pic, hdc, 0, 0, 0, 0, 0, 0, 0, 0, NULL);
>> +ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE);
>> +hres = IPicture_Render(pic, hdc, 0, 0, 10, 10, 0, 0, 10, 0, NULL);
>> +ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE);
>> +hres = IPicture_Render(pic, hdc, 0, 0, 10, 10, 0, 0, 0, 10, NULL);
>> +ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE);
>> +hres = IPicture_Render(pic, hdc, 0, 0, 10, 10, 0, 0, 0, 0, NULL);
>> +ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE);
>> +hres = IPicture_Render(pic, hdc, 0, 0, 0, 10, 0, 0, 10, 10, NULL);
>> +ole_expect(hres, CTL_E_INVALIDPROPERT
Re: oleaut32: added PICTYPE_NONE and PICTYPE_UNINITIALIZED to IPicture::Render with test (try3)
Nikolay Sivov wrote:
> Changelog(try3):
> - fixed identation in testlist
> Changelog(try2):
> - added PICTYPE_NONE and PICTYPE_UNINITIALIZED to IPicture::Render with
> test
> - added test for zero dimensions
>
> P.S. Render with this types spotted in traces of bugs 6799 and 10050
>
>>From 3386e8674380390c3b60c6247443c605ffe6ff6a Mon Sep 17 00:00:00 2001
> From: Nikolay Sivov <[EMAIL PROTECTED]>
> Date: Wed, 10 Dec 2008 13:41:09 +0300
> Subject: added PICTYPE_NONE and PICTYPE_UNINITIALIZED to IPicture::Render
>
> ---
> dlls/oleaut32/olepicture.c |8 +
> dlls/oleaut32/tests/olepicture.c | 64
> ++
> 2 files changed, 72 insertions(+), 0 deletions(-)
>
> diff --git a/dlls/oleaut32/olepicture.c b/dlls/oleaut32/olepicture.c
> index 498ee53..bdce729 100644
> --- a/dlls/oleaut32/olepicture.c
> +++ b/dlls/oleaut32/olepicture.c
> @@ -637,6 +637,10 @@ static HRESULT WINAPI OLEPictureImpl_Render(IPicture
> *iface, HDC hdc,
> TRACE("prcWBounds (%d,%d) - (%d,%d)\n", prcWBounds->left,
> prcWBounds->top,
> prcWBounds->right, prcWBounds->bottom);
>
> + if(cx == 0 || cy == 0 || cxSrc == 0 || cySrc == 0){
> +return CTL_E_INVALIDPROPERTYVALUE;
> + }
> +
>/*
> * While the documentation suggests this to be here (or after rendering?)
> * it does cause an endless recursion in my sample app. -MM 20010804
> @@ -644,6 +648,10 @@ static HRESULT WINAPI OLEPictureImpl_Render(IPicture
> *iface, HDC hdc,
> */
>
>switch(This->desc.picType) {
> + case PICTYPE_UNINITIALIZED:
> + case PICTYPE_NONE:
> +/* nothing to do */
> +return S_OK;
>case PICTYPE_BITMAP:
> {
>HBITMAP hbmpOld;
> diff --git a/dlls/oleaut32/tests/olepicture.c
> b/dlls/oleaut32/tests/olepicture.c
> index f81c419..9e33121 100644
> --- a/dlls/oleaut32/tests/olepicture.c
> +++ b/dlls/oleaut32/tests/olepicture.c
> @@ -25,6 +25,7 @@
> #include
>
> #define COBJMACROS
> +#define NONAMELESSUNION
>
> #include "wine/test.h"
> #include
> @@ -587,6 +588,68 @@ static void test_enhmetafile(void)
> GlobalFree(hglob);
> }
>
> +static void test_Render(void)
> +{
> +IPicture *pic;
> +HRESULT hres;
> +short type;
> +PICTDESC desc;
> +HDC hdc = GetDC(0);
> +
> +/* test IPicture::Render return code on uninitialized picture */
> +OleCreatePictureIndirect(NULL, &IID_IPicture, TRUE, (VOID**)&pic);
> +hres = IPicture_get_Type(pic, &type);
> +ok(hres == S_OK, "IPicture_get_Type does not return S_OK, but 0x%08x\n",
> hres);
> +ok(type == PICTYPE_UNINITIALIZED, "Expected type =
> PICTYPE_UNINITIALIZED, got = %d\n", type);
> +/* zero dimensions */
> +hres = IPicture_Render(pic, hdc, 0, 0, 0, 0, 0, 0, 0, 0, NULL);
> +ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE);
> +hres = IPicture_Render(pic, hdc, 0, 0, 10, 10, 0, 0, 10, 0, NULL);
> +ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE);
> +hres = IPicture_Render(pic, hdc, 0, 0, 10, 10, 0, 0, 0, 10, NULL);
> +ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE);
> +hres = IPicture_Render(pic, hdc, 0, 0, 10, 10, 0, 0, 0, 0, NULL);
> +ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE);
> +hres = IPicture_Render(pic, hdc, 0, 0, 0, 10, 0, 0, 10, 10, NULL);
> +ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE);
> +hres = IPicture_Render(pic, hdc, 0, 0, 10, 0, 0, 0, 10, 10, NULL);
> +ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE);
> +hres = IPicture_Render(pic, hdc, 0, 0, 0, 0, 0, 0, 10, 10, NULL);
> +ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE);
> +/* nonzero dimensions, PICTYPE_UNINITIALIZED */
> +hres = IPicture_Render(pic, hdc, 0, 0, 10, 10, 0, 0, 10, 10, NULL);
> +ole_expect(hres, S_OK);
> +IPicture_Release(pic);
> +
> +desc.cbSizeofstruct = sizeof(PICTDESC);
> +desc.picType = PICTYPE_ICON;
> +desc.u.icon.hicon = LoadIcon(NULL, IDI_APPLICATION);
> +if(!desc.u.icon.hicon){
> +win_skip("LoadIcon failed. Skipping...\n");
Shouldn't you also use ReleaseDC() here for consistency sake?
> +return;
> +}
> +
> +OleCreatePictureIndirect(&desc, &IID_IPicture, TRUE, (VOID**)&pic);
> +/* zero dimensions, PICTYPE_ICON */
> +hres = IPicture_Render(pic, hdc, 0, 0, 0, 0, 0, 0, 0, 0, NULL);
> +ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE);
> +hres = IPicture_Render(pic, hdc, 0, 0, 10, 10, 0, 0, 10, 0, NULL);
> +ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE);
> +hres = IPicture_Render(pic, hdc, 0, 0, 10, 10, 0, 0, 0, 10, NULL);
> +ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE);
> +hres = IPicture_Render(pic, hdc, 0, 0, 10, 10, 0, 0, 0, 0, NULL);
> +ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE);
> +hres = IPicture_Render(pic, hdc, 0, 0, 0, 10, 0, 0, 10, 10, NULL);
> +ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE);
> +hres = IPicture_Render(pic, hdc, 0, 0, 10, 0, 0, 0, 10, 10, NULL);
> +ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE);
> +hres =
Re: oleaut32: added PICTYPE_NONE and PICTYPE_UNINITIALIZED to IPicture::Render with test
Michael Karcher wrote: > Am Mittwoch, den 10.12.2008, 13:31 +0300 schrieb Nikolay Sivov: > - remove some tabs from testfile >>> Please don't change formatting, especially with an incorrect tab size. >>> >> Ok, but I'm using 4 spaces tab size. Is it wrong? >> > > Tab size in wine source code is defined to be 8. Your patch was mixing > tabbed lines with space-indented lines. This causes the code to look > wrong at the standard tab size. > > As a rule of thumb: You should indent you new code with the same > character sequence (spaces/tabs) as the code of the function that > already is there. Your newest iteration still is not OK. The indentation > of the tests in the main test list at the end is (although this is not > nice) partly indented with tabs, partly indented with 8 spaces. You add > a line indented with 4 spaces. > > In the Wine project, whitespace cleanup patches (replacing tabs by > spaces or the other way around) to lines that do not get other changes > are generally frowned upon and usually reject, so don't feel requested > to fix the tab-n-space mix in the test list as well. Just choose one of > the correct variants, that is 8 spaces or one tab. > > Regards, > Michael Karcher > > > Resent. Thank you.
Re: oleaut32: added PICTYPE_NONE and PICTYPE_UNINITIALIZED to IPicture::Render with test
Am Mittwoch, den 10.12.2008, 13:31 +0300 schrieb Nikolay Sivov: > >> - remove some tabs from testfile > > Please don't change formatting, especially with an incorrect tab size. > Ok, but I'm using 4 spaces tab size. Is it wrong? Tab size in wine source code is defined to be 8. Your patch was mixing tabbed lines with space-indented lines. This causes the code to look wrong at the standard tab size. As a rule of thumb: You should indent you new code with the same character sequence (spaces/tabs) as the code of the function that already is there. Your newest iteration still is not OK. The indentation of the tests in the main test list at the end is (although this is not nice) partly indented with tabs, partly indented with 8 spaces. You add a line indented with 4 spaces. In the Wine project, whitespace cleanup patches (replacing tabs by spaces or the other way around) to lines that do not get other changes are generally frowned upon and usually reject, so don't feel requested to fix the tab-n-space mix in the test list as well. Just choose one of the correct variants, that is 8 spaces or one tab. Regards, Michael Karcher
Re: [5/5] winex11.drv: Fix color conversion for 16 bpp cursors.
Henri Verbeet <[EMAIL PROTECTED]> writes: > case 16: > -/* BGR, 5 red, 6 green, 5 blue */ > -*pixel_ptr = *xor_ptr * 0x1f; > -*pixel_ptr |= (*xor_ptr & 0xe0) << 3; > +/* [gggb][rggg] -> > [][r000][gg00][b000] */ It seems to me that zero-padding isn't quite right, you should expand the values linearly. -- Alexandre Julliard [EMAIL PROTECTED]
Re: oleaut32: added PICTYPE_NONE and PICTYPE_UNINITIALIZED to IPicture::Render with test
Alexandre Julliard wrote: > Nikolay Sivov <[EMAIL PROTECTED]> writes: > > >> Changelog: >> - added PICTYPE_NONE and PICTYPE_UNINITIALIZED to IPicture::Render with >> test >> - added test for zero dimensions >> - remove some tabs from testfile >> > > Please don't change formatting, especially with an incorrect tab size. > > Ok, but I'm using 4 spaces tab size. Is it wrong?
Re: oleaut32: added PICTYPE_NONE and PICTYPE_UNINITIALIZED to IPicture::Render with test
Nikolay Sivov <[EMAIL PROTECTED]> writes: > Changelog: > - added PICTYPE_NONE and PICTYPE_UNINITIALIZED to IPicture::Render with > test > - added test for zero dimensions > - remove some tabs from testfile Please don't change formatting, especially with an incorrect tab size. -- Alexandre Julliard [EMAIL PROTECTED]
