> On Oct. 4, 2015, 5:45 p.m., Frank Reininghaus wrote: > > Looks good! The only thing that I'm not sure about is whether 1000 bytes > > are guaranteed to be enough. Some quick Googling tells me that path lengths > > of 4096 are possible. Maybe we could allocate a temporary large array on > > the heap if the readlink call fails with the 1000 byte buffer on the stack? > > Xuetian Weng wrote: > Just FYI, even PATH_MAX may not exists on some system (AFAIK, hurd, > allowed by posix). > And though st.st_size is mentioned in readlink man page, it may not > return useful value. > > > More information > http://stackoverflow.com/questions/9385386/howto-use-readlink-with-dynamic-memory-allocation > > David Faure wrote: > Just for info, I basically restored this code from kdelibs4 :) > But of course I could have a missed a bug report about someone with a > huge symlink target. > > Checking what Qt does... ah, PATH_MAX if defined, otherwise a loop which > reallocs twice more every time (starting from 256 bytes). > Personally I'm tempted to just make it 4096, "it ought to be enough for > everybody" as the saying goes :-) > > Xuetian Weng wrote: > So.. is it possible to directly use Qt API there? Why bother handle it > ourselves :) ?
I explain that in the comments in the patch: because Qt resolves relative symlink targets to absolute ones, which is exactly what triggered this bug in the first place. I think it makes sense for Qt to do so; the 99% use case for symlinks is to read the target file (or dir). In our case however, to copy a symlink, we do care whether it's relative or absolute. - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125515/#review86340 ----------------------------------------------------------- On Oct. 4, 2015, 9:24 a.m., David Faure wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125515/ > ----------------------------------------------------------- > > (Updated Oct. 4, 2015, 9:24 a.m.) > > > Review request for KDE Frameworks. > > > Bugs: 352927 > https://bugs.kde.org/show_bug.cgi?id=352927 > > > Repository: kio > > > Description > ------- > > BUG: 352927 > Change-Id: I7d3c988da32cae9d14750c8adb9ca5af6d140572 > > > Diffs > ----- > > autotests/jobtest.h ef8c3e111ec647cc59c5a9715ab3220ce1651c9e > src/ioslaves/file/file.cpp a0a533c957628b00eff175a949685d4497c5f095 > autotests/jobtest.cpp c24bcead70f78f2bec3b938819fb2fa842e937d5 > > Diff: https://git.reviewboard.kde.org/r/125515/diff/ > > > Testing > ------- > > 2 unit tests added > > > Thanks, > > David Faure > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
