July 2014 - Wine-Devel - list.winehq.org

Re: [PATCH 1/2] riched20: Implement ITextSelection::GetText. (try 3)

by Huw Davies

On Sat, Jul 19, 2014 at 01:08:19AM +0800, Jactry Zeng wrote: > Hi Huw, > > Is this the right way? > @@ -199,7 +205,9 @@ int ME_SetSelection(ME_TextEditor *editor, int from, int to) > /* Selection is not allowed in the middle of an end paragraph run. */ > if (editor->pCursors[1].pRun->member.run.nFlags & MERF_ENDPARA) > editor->pCursors[1].nOffset = 0; > - if (editor->pCursors[0].pRun->member.run.nFlags & MERF_ENDPARA) > + if (editor->pCursors[0].pRun->member.run.nFlags & MERF_ENDPARA && to > len) > + editor->pCursors[0].nOffset = editor->pCursors[0].pRun->member.run.len; > + else if (editor->pCursors[0].pRun->member.run.nFlags & MERF_ENDPARA) > editor->pCursors[0].nOffset = 0; > return to; > } Looks ok apart from this last hunk. It would be cleaner to test the nFlags once and have a nested if/else for the to > len case. Huw.

July 21, 2014

Re: [PATCH] vbscript: Implemented IsNumeric

by Shuai Meng

Thanks very much for commenting~ 2014-07-19 0:22 GMT+08:00 Jacek Caban <jacek(a)codeweavers.com>: > Hi Shuai, > > A few comments: > > - We probably should propagate unrelated errors. So if FAILED(hres) && > hres != DISP_E_TYPEMISMATCH, we should return hres > > if(FAILED(hres)) > return return_bool(res, FALSE); > else > return return_bool(res, TRUE); > > Why not: > return return_bool(res, SUCCEEDED(hres)) > > static HRESULT return_bool(VARIANT *res, BOOL val) > { > if(res) { > V_VT(res) = VT_BOOL; > V_BOOL(res) = val; > } > return S_OK; > } > > VARIANT_BOOL uses VARIANT_TRUE/VARIANT_FALSE values, so this should be: > V_BOOL(res) = val ? VARIANT_TRUE : VARIANT_FALSE; > > Cheers, > Jacek > > > > On 07/18/14 17:29, Shuai Meng wrote: > > Thanks all for commenting, I changed the implementation and put the > > codes > > here: > https://github.com/Shuai-Meng/wine/commit/bb212763ffd639a238b5a47f2c6a27047… > > > > It passed on both winxp and wine. But this patch still needs your > > comments. > > > > > > > > 2014-07-18 17:30 GMT+08:00 Jacek Caban <jacek(a)codeweavers.com > > <mailto:jacek(a)codeweavers.com>>: > > > > On 07/18/14 04:11, Shuai Meng wrote: > >> > >> > >> > >> 2014-07-17 11:11 GMT+08:00 Dmitry Timoshkov <dmitry(a)baikal.ru > >> <mailto:dmitry(a)baikal.ru>>: > >> > >> Shuai Meng <mengshuaicalendr(a)gmail.com > >> <mailto:mengshuaicalendr(a)gmail.com>> wrote: > >> > >> > + switch(V_VT(arg)) { > >> > + case VT_UI1: > >> > + case VT_I2: > >> > + case VT_I4: > >> > + case VT_I8: > >> > + case VT_R4: > >> > + case VT_R8: > >> > + case VT_BOOL: > >> > + case VT_EMPTY: > >> > + case VT_CY: > >> > + V_BOOL(res) = VARIANT_TRUE; > >> > + break; > >> > >> This list is far from being complete. It seems that it should > >> also > >> contain at least VT_UI2, VT_UI4, VT_UI8, VT_I1, VT_INT, VT_UINT. > >> VT_VECTOR, VT_ARRAY and VT_BYREF modifers probably also should > be > >> taken into account. > >> > >> -- > >> Dmitry. > >> > >> Hi, I think over it again. Let's face the fact: what are all the > >> possible results of switch(V_VT(arg)) ? Will VT_UI2, VT_UI4, > >> VT_UI8, VT_I1, VT_INT, VT_UINT and so on exist? My answer is no. > >> arg comes from users' input, which means it only belongs to the > >> subtypes of variant, see here: > >> http://msdn.microsoft.com/en-us/library/9e7a57cf(v=vs.84).aspx > >> <http://msdn.microsoft.com/en-us/library/9e7a57cf%28v=vs.84%29.aspx > > > > > > No, your answer is wrong. VBScript code may be called from > > non-VBScript (and the other way around) and then any type of > > VARIANT may be passed around. Also, please add a test with an > > object having default value, which is of numeric type. I have a > > feeling that the right thing to do here is to call to_double and > > see if it fails or not, but this needs tests to confirm. > > > > Jacek > > > > > >

July 21, 2014

Re: [PATCH 5/5] ws2_32: Add an interactive test for interface change notifications (resend 2).

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=8031 Your paranoid android. === w7u (32 bit sock) === sock.c:618: Test failed: oob_server (b20): not at the OOB mark: 1

July 18, 2014

Re: [website] German translation for release 1.7.22

by Rico Schüller

On 16.07.2014 00:24, Julian Rüger wrote: > + <p>Der Quelltext ist ab sofort <a href="http://prdownloads.sourceforge.net/wine/wine-1.7.22.tar.bz2">verfügbar</a>. > +15 Binärpakete werden gerade erstellt und stehen bald auf den jeweiligen <a href="{$root}/download">Downloadseiten</a> zur Verfügung. There is a 15 too much...

July 18, 2014

Re: [PATCH 1/2] riched20: Implement ITextSelection::GetText. (try 3)

by Jactry Zeng

Hi Huw, Is this the right way? Testbot result: https://testbot.winehq.org/JobDetails.pl?Key=8030 2014-07-16 19:44 GMT+08:00 Huw Davies <huw(a)codeweavers.com>: > On 15 Jul 2014, at 14:43, Jactry Zeng wrote: > 2014-07-15 21:29 GMT+08:00 Huw Davies <huw(a)codeweavers.com>: > > > > > > Can you at least point to a test that shows this bug? > > > > Do you mean tests about bug of S/GetSelection? > > It seem is a known bug: > > > http://source.winehq.org/git/wine.git/blob/HEAD:/dlls/riched20/tests/editor… > > Yes. So adding a special case to SetSelection should fix this. > > 'to' is already setup correctly to len+1, so the problem is > adjusting cursors[0]. MoveCursorChars is clamping the > cursor to len. > > So you have two options, either alter MoveCursorChars > to allow it to move to len + 1. You'd need to retain the current > behaviour in some circumstances though (such as moving > to the right on right-arrow keys press), so you'd need to pass > a flag or something. > > Option 2 is just the modify cursor[0] in SetSelection. If this > is the only place where one can select the final EOP, then > this is probably the preferred solution. > > Either way, you probably need to change the 'middle of > end paragraph run' fix up for cursor[0] at the end of > SetSelection (probably by setting nOffset = run.len). > > Having done this, check the tests still work (and the > todos are fixed). Also play with wordpad for a while > to see it behaves correctly. > > Does that make sense? > > Huw. > > > -- Regards, Jactry Zeng

July 18, 2014

Re: [PATCH] vbscript: Implemented IsNumeric

by Jacek Caban

Hi Shuai, A few comments: - We probably should propagate unrelated errors. So if FAILED(hres) && hres != DISP_E_TYPEMISMATCH, we should return hres if(FAILED(hres)) return return_bool(res, FALSE); else return return_bool(res, TRUE); Why not: return return_bool(res, SUCCEEDED(hres)) static HRESULT return_bool(VARIANT *res, BOOL val) { if(res) { V_VT(res) = VT_BOOL; V_BOOL(res) = val; } return S_OK; } VARIANT_BOOL uses VARIANT_TRUE/VARIANT_FALSE values, so this should be: V_BOOL(res) = val ? VARIANT_TRUE : VARIANT_FALSE; Cheers, Jacek On 07/18/14 17:29, Shuai Meng wrote: > Thanks all for commenting, I changed the implementation and put the > codes > here: https://github.com/Shuai-Meng/wine/commit/bb212763ffd639a238b5a47f2c6a27047… > > It passed on both winxp and wine. But this patch still needs your > comments. > > > > 2014-07-18 17:30 GMT+08:00 Jacek Caban <jacek(a)codeweavers.com > <mailto:jacek(a)codeweavers.com>>: > > On 07/18/14 04:11, Shuai Meng wrote: >> >> >> >> 2014-07-17 11:11 GMT+08:00 Dmitry Timoshkov <dmitry(a)baikal.ru >> <mailto:dmitry(a)baikal.ru>>: >> >> Shuai Meng <mengshuaicalendr(a)gmail.com >> <mailto:mengshuaicalendr(a)gmail.com>> wrote: >> >> > + switch(V_VT(arg)) { >> > + case VT_UI1: >> > + case VT_I2: >> > + case VT_I4: >> > + case VT_I8: >> > + case VT_R4: >> > + case VT_R8: >> > + case VT_BOOL: >> > + case VT_EMPTY: >> > + case VT_CY: >> > + V_BOOL(res) = VARIANT_TRUE; >> > + break; >> >> This list is far from being complete. It seems that it should >> also >> contain at least VT_UI2, VT_UI4, VT_UI8, VT_I1, VT_INT, VT_UINT. >> VT_VECTOR, VT_ARRAY and VT_BYREF modifers probably also should be >> taken into account. >> >> -- >> Dmitry. >> >> Hi, I think over it again. Let's face the fact: what are all the >> possible results of switch(V_VT(arg)) ? Will VT_UI2, VT_UI4, >> VT_UI8, VT_I1, VT_INT, VT_UINT and so on exist? My answer is no. >> arg comes from users' input, which means it only belongs to the >> subtypes of variant, see here: >> http://msdn.microsoft.com/en-us/library/9e7a57cf(v=vs.84).aspx >> <http://msdn.microsoft.com/en-us/library/9e7a57cf%28v=vs.84%29.aspx> > > No, your answer is wrong. VBScript code may be called from > non-VBScript (and the other way around) and then any type of > VARIANT may be passed around. Also, please add a test with an > object having default value, which is of numeric type. I have a > feeling that the right thing to do here is to call to_double and > see if it fails or not, but this needs tests to confirm. > > Jacek > >

July 18, 2014

Re: [PATCH] vbscript: Implemented IsNumeric

by Shuai Meng

Thanks all for commenting, I changed the implementation and put the codes here: https://github.com/Shuai-Meng/wine/commit/bb212763ffd639a238b5a47f2c6a27047… It passed on both winxp and wine. But this patch still needs your comments. 2014-07-18 17:30 GMT+08:00 Jacek Caban <jacek(a)codeweavers.com>: > On 07/18/14 04:11, Shuai Meng wrote: > > > > > 2014-07-17 11:11 GMT+08:00 Dmitry Timoshkov <dmitry(a)baikal.ru>: > >> Shuai Meng <mengshuaicalendr(a)gmail.com> wrote: >> >> > + switch(V_VT(arg)) { >> > + case VT_UI1: >> > + case VT_I2: >> > + case VT_I4: >> > + case VT_I8: >> > + case VT_R4: >> > + case VT_R8: >> > + case VT_BOOL: >> > + case VT_EMPTY: >> > + case VT_CY: >> > + V_BOOL(res) = VARIANT_TRUE; >> > + break; >> >> This list is far from being complete. It seems that it should also >> contain at least VT_UI2, VT_UI4, VT_UI8, VT_I1, VT_INT, VT_UINT. >> VT_VECTOR, VT_ARRAY and VT_BYREF modifers probably also should be >> taken into account. >> >> -- >> Dmitry. >> > Hi, I think over it again. Let's face the fact: what are all the possible > results of switch(V_VT(arg)) ? Will VT_UI2, VT_UI4, VT_UI8, VT_I1, VT_INT, > VT_UINT and so on exist? My answer is no. arg comes from users' input, > which means it only belongs to the subtypes of variant, see here: > http://msdn.microsoft.com/en-us/library/9e7a57cf(v=vs.84).aspx > > > No, your answer is wrong. VBScript code may be called from non-VBScript > (and the other way around) and then any type of VARIANT may be passed > around. Also, please add a test with an object having default value, which > is of numeric type. I have a feeling that the right thing to do here is to > call to_double and see if it fails or not, but this needs tests to confirm. > > Jacek >

July 18, 2014

Re: [PATCH] vbscript: Implemented IsNumeric

by Jacek Caban

On 07/18/14 04:11, Shuai Meng wrote: > > > > 2014-07-17 11:11 GMT+08:00 Dmitry Timoshkov <dmitry(a)baikal.ru > <mailto:dmitry(a)baikal.ru>>: > > Shuai Meng <mengshuaicalendr(a)gmail.com > <mailto:mengshuaicalendr(a)gmail.com>> wrote: > > > + switch(V_VT(arg)) { > > + case VT_UI1: > > + case VT_I2: > > + case VT_I4: > > + case VT_I8: > > + case VT_R4: > > + case VT_R8: > > + case VT_BOOL: > > + case VT_EMPTY: > > + case VT_CY: > > + V_BOOL(res) = VARIANT_TRUE; > > + break; > > This list is far from being complete. It seems that it should also > contain at least VT_UI2, VT_UI4, VT_UI8, VT_I1, VT_INT, VT_UINT. > VT_VECTOR, VT_ARRAY and VT_BYREF modifers probably also should be > taken into account. > > -- > Dmitry. > > Hi, I think over it again. Let's face the fact: what are all the > possible results of switch(V_VT(arg)) ? Will VT_UI2, VT_UI4, VT_UI8, > VT_I1, VT_INT, VT_UINT and so on exist? My answer is no. arg comes > from users' input, which means it only belongs to the subtypes of > variant, see here: > http://msdn.microsoft.com/en-us/library/9e7a57cf(v=vs.84).aspx > <http://msdn.microsoft.com/en-us/library/9e7a57cf%28v=vs.84%29.aspx> No, your answer is wrong. VBScript code may be called from non-VBScript (and the other way around) and then any type of VARIANT may be passed around. Also, please add a test with an object having default value, which is of numeric type. I have a feeling that the right thing to do here is to call to_double and see if it fails or not, but this needs tests to confirm. Jacek

July 18, 2014

Re: [PATCH] vbscript: Implemented IsNumeric

by Shuai Meng

2014-07-17 11:11 GMT+08:00 Dmitry Timoshkov <dmitry(a)baikal.ru>: > Shuai Meng <mengshuaicalendr(a)gmail.com> wrote: > > > + switch(V_VT(arg)) { > > + case VT_UI1: > > + case VT_I2: > > + case VT_I4: > > + case VT_I8: > > + case VT_R4: > > + case VT_R8: > > + case VT_BOOL: > > + case VT_EMPTY: > > + case VT_CY: > > + V_BOOL(res) = VARIANT_TRUE; > > + break; > > This list is far from being complete. It seems that it should also > contain at least VT_UI2, VT_UI4, VT_UI8, VT_I1, VT_INT, VT_UINT. > VT_VECTOR, VT_ARRAY and VT_BYREF modifers probably also should be > taken into account. > > -- > Dmitry. > Hi, I think over it again. Let's face the fact: what are all the possible results of switch(V_VT(arg)) ? Will VT_UI2, VT_UI4, VT_UI8, VT_I1, VT_INT, VT_UINT and so on exist? My answer is no. arg comes from users' input, which means it only belongs to the subtypes of variant, see here: http://msdn.microsoft.com/en-us/library/9e7a57cf(v=vs.84).aspx

July 18, 2014

Re: [2/4] server: Support sending process and thread security descriptors for the "new_process" request in the protocol.

by Alexandre Julliard

Joris van der Wel <joris(a)jorisvanderwel.com> writes: > + int inherit_all; /* inherit all handles from parent */ > + unsigned int create_flags; /* creation flags */ > + int socket_fd; /* file descriptor for process socket */ > + obj_handle_t exe_file; /* file handle for main exe */ > + unsigned int process_access; /* access rights for process object */ > + unsigned int process_attr; /* attributes for process object */ > + unsigned int thread_access; /* access rights for thread object */ > + unsigned int thread_attr; /* attributes for thread object */ > + cpu_type_t cpu; /* CPU that the new process will use */ > + data_size_t process_sd_size; /* size of the process security descriptor */ > + data_size_t thread_sd_size; /* size of the thread security descriptor */ > + data_size_t info_size; /* size of startup info */ > + data_size_t env_size; /* size of the environment */ > + VARARG(process_sd,security_descriptor,process_sd_size); /* security descriptor to set on the process */ > + VARARG(thread_sd,security_descriptor,thread_sd_size); /* security descriptor to set on the thread */ > + VARARG(info,startup_info,info_size); /* startup information */ > + VARARG(env,unicode_str,env_size); /* environment for new process */ There's no reason to add the environment size. Also you need to validate all the sizes before using them. And please avoid reformatting lines that you are not changing. -- Alexandre Julliard julliard(a)winehq.org

July 17, 2014