[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-windows
Subject:    Re: [Kde-games-devel] Review Request: fixed file kioslave on windows
From:       George Kiagiadakis <kiagiadakis.george () gmail ! com>
Date:       2010-05-10 10:04:32
Message-ID: AANLkTil1_prDVGMOvC8i6eaMQbb7JXwRWKZCGsQMq8Bm () mail ! gmail ! com
[Download RAW message or body]

On Sun, May 9, 2010 at 11:30 PM, Christian Ehrlicher
<Ch.Ehrlicher@gmx.de> 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?
_______________________________________________
Kde-windows mailing list
Kde-windows@kde.org
https://mail.kde.org/mailman/listinfo/kde-windows
[prev in list] [next in list] [prev in thread] [next in thread]