December 2022 - wine-gitlab

Re: [PATCH v3 0/3] MR1825: kernelbase/path.c: UrlFixupW()
by Zebediah Figura (＠zfigura) 18 Dec '22

18 Dec '22

Zebediah Figura (@zfigura) commented about dlls/kernelbase/path.c: > if (!url) > return E_FAIL; > > - srcLen = lstrlenW(url) + 1; > + /* > + * First check for known, valid and typo free scheme > + */ > + for (pos=1; pos<ARRAY_SIZE(url_scheme); pos++) > + { > + len = wcslen(url_scheme[pos]); > + if ( (len <= wcslen(url)) && (!_wcsnicmp(url, url_scheme[pos], len)) ) > + { > + /* > + * check if string fits into maxChars > + */ > + if (len+1 >= maxChars) This comment doesn't seem to tell us anything more than the following line does. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1825#note_19810

1 0

Re: [PATCH v3 0/3] MR1825: kernelbase/path.c: UrlFixupW()
by Zebediah Figura (＠zfigura) 18 Dec '22

18 Dec '22

Zebediah Figura (@zfigura) commented about dlls/kernelbase/path.c: > + * First check for known, valid and typo free scheme > + */ > + for (pos=1; pos<ARRAY_SIZE(url_scheme); pos++) > + { > + len = wcslen(url_scheme[pos]); > + if ( (len <= wcslen(url)) && (!_wcsnicmp(url, url_scheme[pos], len)) ) > + { > + /* > + * check if string fits into maxChars > + */ > + if (len+1 >= maxChars) > + return S_FALSE; > + > + lstrcpynW(save_str, url_scheme[pos], len+1); > + url += len; > + goto scheme_done; Can we use a helper function to fix up the scheme instead of using a goto? This will probably also help make the function easier to follow, by virtue of being less monolithic. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1825#note_19809

1 0

Re: [PATCH v3 0/3] MR1825: kernelbase/path.c: UrlFixupW()
by Zebediah Figura (＠zfigura) 18 Dec '22

18 Dec '22

Zebediah Figura (@zfigura) commented about dlls/kernelbase/path.c: > */ > HRESULT WINAPI UrlFixupW(PCWSTR url, PWSTR translatedUrl, DWORD maxChars) > { > - DWORD srcLen; > + DWORD srcLen, dstLen, len; > + DWORD colon_pos = 0, pos = 0; > + wchar_t *helper_str = NULL; > + PWSTR save_str = translatedUrl; We use WCHAR, not wchar_t. It'd also be nice to use a consistent style for all of variable names in this function (wrt snake case vs camel case). In new code I think we prefer snake case. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1825#note_19808

1 0

Re: [PATCH v3 0/3] MR1825: kernelbase/path.c: UrlFixupW()
by Zebediah Figura (＠zfigura) 18 Dec '22

18 Dec '22

Zebediah Figura (@zfigura) commented about dlls/kernelbase/path.c: > - srcLen = lstrlenW(url) + 1; > + /* > + * First check for known, valid and typo free scheme > + */ > + for (pos=1; pos<ARRAY_SIZE(url_scheme); pos++) > + { > + len = wcslen(url_scheme[pos]); > + if ( (len <= wcslen(url)) && (!_wcsnicmp(url, url_scheme[pos], len)) ) > + { > + /* > + * check if string fits into maxChars > + */ > + if (len+1 >= maxChars) > + return S_FALSE; > + > + lstrcpynW(save_str, url_scheme[pos], len+1); We already know it fits, so this can just be memcpy(). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1825#note_19807

1 0

Re: [PATCH v3 0/3] MR1825: kernelbase/path.c: UrlFixupW()
by Zebediah Figura (＠zfigura) 18 Dec '22

18 Dec '22

Zebediah Figura (@zfigura) commented about dlls/kernelbase/path.c: > + DWORD colon_pos = 0, pos = 0; > + wchar_t *helper_str = NULL; > + PWSTR save_str = translatedUrl; > > - FIXME("%s, %p, %ld stub\n", wine_dbgstr_w(url), translatedUrl, maxChars); > + const WCHAR *url_scheme[] = { L"", L"ftp:", L"https:", L"gopher:", L"mailto:", L"news:", > + L"nntp:", L"telnet:", L"wais:", L"file:", L"mk:", > + L"http:", L"shell:", L"snews:", L"local:", > + L"javascript:", L"vbscript:", L"about:", L"res:", > + L"ms-shell-rooted:", L"ms-shell-idlist:", L"hcp:"}; > + > + const WCHAR *url_scheme_short[] = { L"", L"ftp", L"https", L"gopher", L"mailto", L"snews", > + L"nntp", L"telnet", L"wais", L"file", L"mk", > + L"http", L"ms-shell-rooted", L"news", L"local", > + L"javascript", L"vbscript", L"about", L"res", > + L"ms-shell-idlist", L"shell", L"hcp"}; These are constant arrays, so should be "static const WCHAR *const". Also, it seems that one array is the same as the other without the final colon; can we avoid defining two arrays? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1825#note_19806

1 0

Re: [PATCH v3 0/3] MR1825: kernelbase/path.c: UrlFixupW()
by Zebediah Figura (＠zfigura) 18 Dec '22

18 Dec '22

Zebediah Figura (@zfigura) commented about dlls/kernelbase/path.c: > > -HRESULT WINAPI UrlFixupW(const WCHAR *url, WCHAR *translatedUrl, DWORD maxChars) > +/* > + * from Documentation: > + * https://docs.microsoft.com/en-us/windows/desktop/api/shlwapi/nf-shlwapi-url… > + * > + * UrlFixupW attempts to correct a URL whose protocol identifier is incorrect. > + * For example, htttp will be changed to http. > + * > + * LWSTDAPI UrlFixupW( > + * [in] PCWSTR pcszUrl, > + * [out] PWSTR pszTranslatedUrl, > + * DWORD cchMax > + * ); > +*/ > +HRESULT WINAPI UrlFixupW(PCWSTR url, PWSTR translatedUrl, DWORD maxChars) Without trying to get bogged down in style, this change is somewhat counterproductive; we tend to avoid P* typedefs in new code, and we also have been abandoning the use of "documentation" style headers, which rarely convey any useful information. (This also doesn't really belong in a commit which should only affect tests.) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1825#note_19805

1 0

Re: [PATCH v3 0/3] MR1825: kernelbase/path.c: UrlFixupW()
by Thomas Csovcsity (＠thc13) 18 Dec '22

18 Dec '22

On Sun Dec 18 23:01:30 2022 +0000, Zebediah Figura wrote: > Hi Thomas, thanks for the patch! > We want the tests to "pass" after every commit, i.e. have no failures. > That may mean, in this case, introducing the tests marked as todo, and > then later removing those todos. Ok. Got it. Will do it soon -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1825#note_19804

1 0

Re: [PATCH v3 0/3] MR1825: kernelbase/path.c: UrlFixupW()
by Zebediah Figura (＠zfigura) 18 Dec '22

18 Dec '22

Hi Thomas, thanks for the patch! We want the tests to "pass" after every commit, i.e. have no failures. That may mean, in this case, introducing the tests marked as todo, and then later removing those todos. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1825#note_19803

1 0

[PATCH 0/4] MR1755: refcount return voiding
by Max Figura (＠maxfigura) 18 Dec '22

18 Dec '22

Setting the return type of various wined3d refcounting functions to void These patches modify: - wined3d_incref - wined3d_decref - wined3d_resource_incref - wined3d_resource_decref -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1755

4 9

[PATCH 0/3] MR1836: DMusic: Implement the IDirectMusicSegment8_Clone() and semi-stub the IDirectMusicChordMap_ComposeSegmentFromTemplate() methods.
by Michael Stefaniuc (＠mstefani) 18 Dec '22

18 Dec '22

Fixes https://bugs.winehq.org/show_bug.cgi?id=53941 Should be a fairly safe change as both methods were returning S_OK without touching the return pointer. Also Chicken Tournament is the first application in my collection that uses those methods. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1836

2 3

2025

2024

2023

2022

wine-gitlab December 2022