August 2011 - Wine-Devel - list.winehq.org

Re: [PATCH 5/5] shell32: Implement a default property sheet for file system objects
by Nikolay Sivov Aug. 22, 2011

Aug. 22, 2011

On 8/21/2011 19:21, Jay Yang wrote: > + LoadStringA(shell32_hInstance,IDS_PROPSHEET_VARIOUS, > + various_str,sizeof(various_str)); > + LoadStringW(shell32_hInstance,IDS_PROPSHEET_COUNT_FORMAT, > + count_format,sizeof(count_format)/sizeof(WCHAR)); Why -A call here? > + WCHAR count_str[64]; Again some magic. > +static INT_PTR DefaultPropSheet_OnNotify(HWND hwndDlg,WPARAM wParam, NMHDR *header) > +{ > + switch(header->code) > + { > + case PSN_APPLY: > + { > + UINT index; > + PROPSHEETPAGEW *page; > + DefaultPropSheet *propsheet; > + WCHAR name[MAX_PATH]; > + UINT readonly_state, hidden_state; > + UINT i; > + index = SendMessageW(GetParent(hwndDlg),PSM_HWNDTOINDEX,(WPARAM)hwndDlg,0); > + page = (PROPSHEETPAGEW*)SendMessageW(GetParent(hwndDlg),PSM_INDEXTOPAGE,index,0); > + propsheet = (DefaultPropSheet*)page->lParam; > + > + /*set name*/ > + if(propsheet->cidl==1) > + { > + BOOL is_folder; > + LPCITEMIDLIST pidl_child; > + LPITEMIDLIST pidl,pidl_new,pidl_child_new,temp; > + IShellFolder *folder; > + WCHAR old_name[MAX_PATH]; > + GetDlgItemTextW(hwndDlg,IDC_PROPSHEET_NAME,name,sizeof(name)/sizeof(WCHAR)); > + pidl = ILCombine(propsheet->folder_pidl,propsheet->apidl[0]); > + is_folder = _ILIsFolder(pidl); > + temp = ILClone(pidl); > + ILRemoveLastID(temp); > + SHBindToParent(pidl,&IID_IShellFolder,(void**)&folder,&pidl_child); > + ILGetDisplayNameExW(folder, pidl_child, old_name,SHGDN_NORMAL); > + if(strcmpW(old_name,name)!=0) > + { > + if(SUCCEEDED(IShellFolder_SetNameOf(folder,hwndDlg,pidl_child,name,SHGDN_NORMAL,&pidl_child_new))) > + { > + pidl_new = ILCombine(temp,pidl_child_new); > + SHChangeNotify(is_folder ? SHCNE_RENAMEFOLDER : SHCNE_RENAMEITEM,SHCNF_IDLIST,pidl,pidl_new); > + ILFree(pidl_child_new); > + ILFree(pidl_new); > + } > + } > + ILFree(temp); > + ILFree(pidl); > + IShellFolder_Release(folder); > + } > + /*set readonly/hidden*/ > + readonly_state = IsDlgButtonChecked(hwndDlg,IDC_PROPSHEET_READONLY); > + hidden_state = IsDlgButtonChecked(hwndDlg,IDC_PROPSHEET_HIDDEN); > + for(i=0;i<propsheet->cidl;i++) > + { > + LPITEMIDLIST pidl; > + WCHAR path[MAX_PATH]; > + DWORD attributes; > + pidl = ILCombine(propsheet->folder_pidl,propsheet->apidl[0]); > + SHGetPathFromIDListW(pidl,path); > + ILFree(pidl); > + attributes = GetFileAttributesW(path); > + switch(readonly_state) > + { > + case BST_CHECKED: > + attributes |= FILE_ATTRIBUTE_READONLY; > + break; > + case BST_UNCHECKED: > + attributes&= ~FILE_ATTRIBUTE_READONLY; > + break; > + } > + switch(hidden_state) > + { > + case BST_CHECKED: > + attributes |= FILE_ATTRIBUTE_HIDDEN; > + break; > + case BST_UNCHECKED: > + attributes&= ~FILE_ATTRIBUTE_HIDDEN; > + break; > + } > + SetFileAttributesW(path,attributes); > + } > + break; > + } > + case PSN_RESET: > + { > + UINT index; > + PROPSHEETPAGEW *page; > + DefaultPropSheet *propsheet; > + index = SendMessageW(GetParent(hwndDlg),PSM_HWNDTOINDEX,(WPARAM)hwndDlg,0); > + page = (PROPSHEETPAGEW*)SendMessageW(GetParent(hwndDlg),PSM_INDEXTOPAGE,index,0); > + propsheet = ((DefaultPropSheet*)page->lParam); > + IShellPropSheetExt_Release(&propsheet->IShellPropSheetExt_iface); > + } Missed 'break' probably? > + default: > + return FALSE; > + } > + return TRUE; > +}

1 0

Re: [PATCH 3/5] shell32: Implement SHOpenPropSheet
by Nikolay Sivov Aug. 22, 2011

Aug. 22, 2011

On 8/21/2011 19:20, Jay Yang wrote: > --- > dlls/shell32/shell32.spec | 3 ++ > dlls/shell32/shellord.c | 88 +++++++++++++++++++++++++++++++++++++++++++++ > include/shlobj.h | 4 ++ > 3 files changed, 95 insertions(+), 0 deletions(-) It's not clear for me why you placed this in shellord.c, it's supposed to contain exported by ordinal calls only, as I understand it. If Julliard approves new file for propsheets then you should place this in it. IMO it's ok to place all of this in shlmenu.c for example. > +BOOL WINAPI SHOpenPropSheetA(LPCSTR pszCaption, HKEY *ahkey, UINT ckeys, const CLSID *pclsidDef, IDataObject *pdtobj, IShellBrowser *psb, LPCSTR pStartPage) > +{ > + BOOL ret; > + LPWSTR wszCaption,wszStartPage; > + UINT length; > + length = MultiByteToWideChar(CP_ACP, 0, pszCaption, -1, NULL, 0); > + wszCaption = HeapAlloc(GetProcessHeap(),0,sizeof(WCHAR)*length); > + length = MultiByteToWideChar(CP_ACP, 0, pszCaption, -1, wszCaption, length); > + length = MultiByteToWideChar(CP_ACP, 0, pStartPage, -1, NULL, 0); > + wszStartPage = HeapAlloc(GetProcessHeap(),0,sizeof(WCHAR)*length); > + length = MultiByteToWideChar(CP_ACP, 0, pStartPage, -1, wszStartPage, length); > + ret = SHOpenPropSheetW(wszCaption,ahkey,ckeys,pclsidDef,pdtobj,psb,wszStartPage); > + HeapFree(GetProcessHeap(),0,wszCaption); > + HeapFree(GetProcessHeap(),0,wszStartPage); > + return ret; > +} Hmm, actually there's HEAP_strdupAtoW() for that (you need to make in non-static though). > + ZeroMemory(&psh, sizeof(PROPSHEETHEADERW)); > + psh.dwSize = sizeof (PROPSHEETHEADERW); > + psh.hwndParent = NULL; > + psh.dwFlags = PSH_PROPTITLE; > + if(pStartPage) > + { > + psh.dwFlags |= PSH_USEPSTARTPAGE; > + psh.u2.pStartPage = pStartPage; > + } Please don't mix tabs with spaces, use 4 spaces everywhere. > + if(pszCaption) > + psh.pszCaption = pszCaption; > + else > + psh.pszCaption = empty_str; Is it really needed? I mean pszCaption == NULL is not the same as empty string in this context? > + IShellPropSheetExt *def_ext; > + IShellExtInit *init_iface; > + PROPSHEETHEADERW psh; > + HPROPSHEETPAGE pages[99]; It's not clear again where this size comes from, just enough for anyone? Also I think it's better to reduce scope as much as possible, def_ext doesn't need to be global for example.

1 0

Re: [PATCH 2/5] shell32: Use SHCreateDefaultContextMenu to create context menus (try 2)
by Nikolay Sivov Aug. 22, 2011

Aug. 22, 2011

On 8/21/2011 19:20, Jay Yang wrote: > Changes from last time: > > Factored out the code to find the registry keys for the extensions. > > --- > dlls/shell32/classes.c | 47 +++++++++++++++++++++++++++++++++++++++++ > dlls/shell32/shell32_main.h | 1 + > dlls/shell32/shfldr.h | 9 +++++++ > dlls/shell32/shfldr_desktop.c | 8 ++---- > dlls/shell32/shfldr_fs.c | 7 +---- > dlls/shell32/shfldr_mycomp.c | 4 +-- > dlls/shell32/shfldr_unixfs.c | 11 ++++++--- > dlls/shell32/shlmenu.c | 40 ++++++++++++++++++++++++++++++++++ > dlls/shell32/shlview.c | 44 +++++++++++++++++++++++++------------- > 9 files changed, 139 insertions(+), 32 deletions(-) > +HRESULT SHELL_CreateItemContextMenu(HWND hwnd, IShellFolder *folder, > + LPITEMIDLIST folder_pidl, > + LPCITEMIDLIST *apidl, UINT cidl, > + REFIID riid, void **ppv) > +{ > + DEFCONTEXTMENU menu_info = {hwnd,NULL,folder_pidl,folder,cidl,apidl,NULL,0,NULL}; > + UINT cKeys = 0; > + HKEY aKeys[16] = {NULL}; Where size of 16 comes from? > + UINT i; > + HRESULT hres; > + cKeys = HCR_GetExtensionsKeysForPidl(apidl[0],aKeys); > + menu_info.cKeys=cKeys; > + menu_info.aKeys=aKeys; > + hres = SHCreateDefaultContextMenu(&menu_info,riid,ppv); > + for(i=0;i<cKeys;i++) > + if(aKeys[i]) > + RegCloseKey(aKeys[i]); RegCloseKey already check for NULL handle. > + hr = IShellFolder_CreateViewObject(This->pSFParent,This->hWndParent,&IID_IContextMenu,(void**)&pCM); > + if(FAILED(hr)){ > + LPITEMIDLIST folder_pidl; > + IPersistFolder2 *persist; > + IShellFolder_QueryInterface(This->pSFParent,&IID_IPersistFolder2,(void**)&persist); > + IPersistFolder2_GetCurFolder(persist,&folder_pidl); > + IPersistFolder2_Release(persist); > + hr= SHELL_CreateBackgroundContextMenu(This->hWndParent,This->pSFParent, > + folder_pidl,&IID_IContextMenu, > + (void**)&pCM); > + ILFree(folder_pidl); > + } I'm not sure about that, but it looks like it deserves some check for 'persist' pointer. Or better a return value being S_OK (from query).

1 0

Re: [PATCH 1/5] shell32: Implement loading of context menu shell extensions (try 3)
by Nikolay Sivov Aug. 22, 2011

Aug. 22, 2011

On 8/21/2011 19:20, Jay Yang wrote: > The first two patches in the sequence relate to context menus and the last three to property sheets, > but some of the changes in the last three depend on a function I created in the second patch so I've > sent them as a sequence. > > Changes from last time (for this file): > cleaned up the code. > > --- > dlls/shell32/shlmenu.c | 170 ++++++++++++++++++++++++++++++++++++++++++++++- > 1 files changed, 166 insertions(+), 4 deletions(-) > + HKEY *new_keys=NULL; > > + new_keys = HeapAlloc(GetProcessHeap(),0,sizeof(HKEY)*cKeys); No need to initialize that explicitly. > + ULONG status = RegOpenKeyExW(aKeys[i],cmenu_handler_key,0,KEY_READ,&new_key); > + if(status==ERROR_SUCCESS) This call return LSTATUS which is signed LONG. A common way to test return value is to use (!RegOpen*()). > + CLSID *clsid_table=NULL; ... > + clsid_table = HeapAlloc(GetProcessHeap(),HEAP_ZERO_MEMORY,sizeof(CLSID)*handler_count); Same here, no need to NULL it. > + static const CLSID zero_clsid = {0}; > + CLSID *curr_clsid = clsid_table+j; > + if(IsEqualCLSID(curr_clsid,&zero_clsid)) There's CLSID_NULL for that. > + TRACE("Loading shell extension with clsid %s\n",shdebugstr_guid(&id)); I'm not sure shdebugstr_guid() is supposed to know shell extension names. > + if(new_keys[i]) > + RegCloseKey(new_keys[i]); This null check is redundant.

1 0

re: [2/3] wined3d: Constant load optimisations
by Dan Kegel Aug. 22, 2011

Aug. 22, 2011

Hi Norbert, patch 2/3 has a couple failures here: http://buildbot.kegel.com/builders/runtests/builds/104 ../../../tools/runtest -q -P wine -M d3d9.dll -T ../../.. -p d3d9_test.exe.so visual.c && touch visual.ok ... visual.c:4991: Test failed: quad 1 has color 00000000, expected 0x00bfbf80 visual.c:4998: Test failed: quad 3 has color 00000000, expected 0x00bfbf80 visual.c:5143: Test failed: quad 4 has color 00000000, expected 0x00bfbf80 patch 3/3 has lots of failures here: http://buildbot.kegel.com/builders/runtests/builds/105 ../../../tools/runtest -q -P wine -M d3d8.dll -T ../../.. -p d3d8_test.exe.so visual.c && touch visual.ok fixme:win:EnumDisplayDevicesW ((null),0,0x32f5ac,0x00000000), stub! fixme:d3d_draw:drawPrimitive Using software emulation because manual fog coordinates are provided visual.c:739: Test failed: RCP test returned color 0x00ffffff, expected 0x00808080. visual.c:769: Test failed: RSQ test returned color 0x00ffffff, expected 0x00b4b4b4. ... visual.c:4991: Test failed: quad 1 has color 00000000, expected 0x00bfbf80 visual.c:4998: Test failed: quad 3 has color 00000000, expected 0x00bfbf80 visual.c:5143: Test failed: quad 4 has color 00000000, expected 0x00bfbf80 ... Here's my system description: os: Ubuntu 11.04, 2.6.38-10-generic-pae, pulseaudio 0.9.22-24-g67d18, Advanced Linux Sound Architecture Driver Version 1.0.23. ram: 4020 MB cpu: Intel(R) Core(TM)2 Quad CPU Q9300 @ 2.50GHz gpu: GeForce 8500 GT/PCI/SSE2 3.3.0 NVIDIA 270.41.06

2 5

buildbot status
by Dan Kegel Aug. 22, 2011

Aug. 22, 2011

The bot found a real problem again in someone's patch today. Mail is not yet automated, so I let the author know by hand. Because several people asked whether they could run headless slaves, I added headless support; now if DISPLAY is not set, the bot won't try to run any tests known to fail with DISPLAY unset. We'll probably group the headless servers differently, and use them for specialized tests (e.g. builds with different compilers). Because someone expressed interest in running on Debian Squeeze, I tried running the demo on that OS, and cleaned up a couple issues that turned up. readme-wine.txt now suggests starting the master before the slave, to avoid a spurious error message. The master fell over this morning, not sure why. So its max uptime so far is something like three days. - Dan

1 0

Re: wined3d: invalidate streamsrc, vdecl and indexbuffer after strided draws
by Henri Verbeet Aug. 21, 2011

Aug. 21, 2011

On 21 August 2011 23:25, Stefan Dösinger <stefandoesinger(a)gmx.at> wrote: > + * provided strided data has ovwritten pretty much the entire vertex and Typo. > + * and index stream related states */ Missing punctuation.

2 1

re: [2/3] ntdll: While requesting TokenGroups calculate required user buffer size in server
by Dan Kegel Aug. 21, 2011

Aug. 21, 2011

Doesn't build here: reply->user_len = FIELD_OFFSET( TOKEN_GROUPS, Groups[group_count] ) + sid_size); token.c: In function ‘req_get_token_groups’: token.c:1319:87: error: expected ‘;’ before ‘)’ token token.c:1319:87: error: expected statement before ‘)’ token http://buildbot.kegel.com/builders/runtests/builds/110

2 2

Re: wined3d: Add WINED3D_BUFFER_HASDESC at index buffer creation
by Henri Verbeet Aug. 21, 2011

Aug. 21, 2011

On 20 August 2011 18:46, Norbert Lataille <nlataill(a)gmail.com> wrote: > Patch is forcing WINED3D_BUFFER_HASDESC flag at Index Buffer creation time, I doubt this works properly, unloads will drop it again.

2 1

re: [PATCH 01/10] cmd: Use CSTR_* instead of hardcoded values as result of CompareStringW
by Dan Kegel Aug. 21, 2011

Aug. 21, 2011

Hi Frédéric, your patch series seems to have failed the cmd tests: batch.c:259: Test failed: unexpected char 0x53 position 0 in line 301 (got 'Syntax error', wanted '1') batch.c:259: Test failed: unexpected char 0x31 position 0 in line 302 (got '1', wanted '3') batch.c:259: Test failed: unexpected char 0x33 position 0 in line 303 (got '3', wanted '5') See http://buildbot.kegel.com/builders/runtests/builds/86 I'll change the buildbot so it tests each subset of a patch series, and pinpoint the one that broke things. Incidentally, that 'Syntax error' probably should go to stderr; and since the cmd tests ignore stderr, that might fix your failure.

3 16