replace mkdtemp for more portabilitiy
Review Request #103692 - Created Jan. 13, 2012 and updated
since mkdtemp is not portable, I want to replace the code in question with the usage of KTempDir.
it builds well. Since I have no idea how that code is used, I hope you can give me some hints?
|The directory gets deleted when the KTempDir goes out of scope. The best fix would be to replace the *calls* ...||David Faure|
The directory gets deleted when the KTempDir goes out of scope. The best fix would be to replace the *calls* to createTempDir with usage of KTempDir. The less-effort-involved fix would be to add td.setAutoRemove(false); It just seems like a waste, to have code to clean up the directory by hand, when KTempDir's destructor could do it all for you ;-) PS: you can keep the call to strerror, by making it strerror(td.status()). status is documented to return an errno value.
Gwenview code used to call KTempDir before. I changed to call mkdtemp() because KTempDir attempts to chmod the dir, which fails on some filesystems and is not required here. For this reason I'd rather not go back to KTempDir, is there any other alternative? one could copy KTempDir Windows code, but that is not really nice.
So, you found a bug in KTempDir. How about we fix it in KTempDir then? Maybe it could fail on chmod only when the directoryPrefix argument is empty/default, and ignore chmod failures on other paths? I presume this isn't about /tmp in your case, but about a temporary directory in an application-specified directory (e.g. image dir) which could be on a FAT system?
Lukas (new gwenview maintainer) - can you check this? The patch looks good to me, but I don't know how to test it. Of course in the frameworks branch, one can use my QTemporaryDir instead :-)
(that last message was from me - don't trust the From field... I guess I can add people as CC because I'm admin of some sorts... gerrit allows everyone to add reviewers.... just saying....)