[Kde-games-devel] Review Request: fixed file kioslave on windows (it was reporting non-existing files as existing)

George Kiagiadakis kiagiadakis.george-Re5JQEeQqe8AvxtiuMwx3w at public.gmane.org
Mon May 10 11:04:32 BST 2010


On Sun, May 9, 2010 at 11:30 PM, Christian Ehrlicher
<Ch.Ehrlicher-Mmb7MZpHnFY at public.gmane.org> wrote:
> Am 09.05.2010 20:38, schrieb Ilie Halip:
>>
>> -----------------------------------------------------------
>> This is an automatically generated e-mail. To reply, visit:
>> http://reviewboard.kde.org/r/3938/
>> -----------------------------------------------------------
>>
>> Review request for KDE Games.
>>
> this isn't a review for kdegames but for k-c-d & kde-windows.
> If you want to break symlink handling for non-ascii filenames on windows
> you can commit it. There's a reason for createUDSEntryWin() ...
>
> KDE_lstat() could be replaced with KDE::lstat() but there's no
> readlink() implementation which takes utf-16 on windows.
>
>
> Christian

It would be wise to put comments in the code to explain why there is a
different implementation for windows. The code looks similar in both
implementations, but unfortunately createUDSEntryWin() is broken. The
mistake is that it doesn't return bool, so it has no way of reporting
errors like createUDSEntry() does. This way, stat() never fails and
reports non-existent files as existent. Fixing it would require
copying code from createUDSEntry(), but since they are similar and
createUDSEntry() compiles, I thought it could work too and that is why
I suggested Ilie to try this solution. To our surprise, it worked.

So, if symlink handling is broken with readlink(), then I can think of
two solutions:
1) Provide a good implementation for it in the kdewin library.
2) #ifdef this line out on windows and use QFileInfo, as
createUDSEntryWin() does. There is no reason to copy all the function
just to change this detail, is there?

Regarding lstat(), how many implementations are there and why?
KDE_lstat(), KDE::lstat(), kdewin_lstat()... KDE_lstat() seems to work
there, so what is the problem?



More information about the kde-core-devel mailing list