Review Request 117131: Implement KUser::faceIconPath on Windows.

[Nicolás Alvarez]

> On March 28, 2014, 10:49 a.m., Alexander Richardson wrote:
> > src/lib/util/kuser_win.cpp, line 391
> > <https://git.reviewboard.kde.org/r/117131/diff/1/?file=257913#file257913line391>
> >
> >     WCHAR pathBuf[MAX_PATH];
> >     
> >     then you don't need the reinterpret_cast and can use QString::fromWCharArray()

Ah nice, I didn't know about fromWCharArray, and without that, using WCHAR meant I needed a reinterpret_cast in the fromUtf16 call anyway. I'll change it.


> On March 28, 2014, 10:49 a.m., Alexander Richardson wrote:
> > src/lib/util/kuser_win.cpp, line 388
> > <https://git.reviewboard.kde.org/r/117131/diff/1/?file=257913#file257913line388>
> >
> >     maybe make this static so it only gets resolved once:
> >     
> >     
> >     static SGUPP_ptr SHGetUserPicturePath = reinterpret_cast<SGUPP_ptr>(GetProcAddress(LoadLibraryW(L"shell32.dll"), MAKEINTRESOURCEA(261)));

Agreed. And it makes sense to make the library handle static too.


> On March 28, 2014, 10:49 a.m., Alexander Richardson wrote:
> > src/lib/util/kuser_win.cpp, line 384
> > <https://git.reviewboard.kde.org/r/117131/diff/1/?file=257913#file257913line384>
> >
> >     Don't think we need the RAII object here, AFAIK we link to shell32.dll anyway, so it will only be deleted on process exit anyway.

-1. You could make the same argument about not needing to uninitialize COM. It's simply good practice to free what you allocate, or in this case, decrement reference counts when you're done even if you know it'll never reach 0 anyway.


- Nicolás


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117131/#review54437
-----------------------------------------------------------


On March 27, 2014, 10:07 p.m., Nicolás Alvarez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/117131/
> -----------------------------------------------------------
> 
> (Updated March 27, 2014, 10:07 p.m.)
> 
> 
> Review request for KDE Frameworks and kdewin.
> 
> 
> Repository: kcoreaddons
> 
> 
> Description
> -------
> 
> Implement KUser::faceIconPath on Windows.
> 
> I use an undocumented Windows API (http://undoc.airesoft.co.uk/shell32.dll/SHGetUserPicturePath.php) that stores the profile image in a temporary file and returns the path to it. The previous code was just trying to load that temporary file, and would only work if another app had created the file recently (such as the control panel section where the image is changed).
> 
> This only works on Windows Vista and later; on Windows XP the undocumented API is different, and faceIconPath will just return an empty string.
> 
> 
> Diffs
> -----
> 
>   src/lib/util/kuser_win.cpp 96cf2f0b89ac18a68783793d3a8b2827b72dd968 
> 
> Diff: https://git.reviewboard.kde.org/r/117131/diff/
> 
> 
> Testing
> -------
> 
> Compiled with MSVC2010, tested via the new faceicontest on Windows 7.
> 
> 
> Thanks,
> 
> Nicolás Alvarez
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20140330/f3d81f0c/attachment-0001.html>