On Fri, 14 Jan 2011 17:11:35 +0100, Colomban wrote:
Hey,
I found a bug in project creation (and save) check: if the project path is a writable directory, update_config() won't see that write will fail, and no error at all will be reported to the user (though write will fail on a directory).
I fixed this (in the attached patch) by:
- check whether the path is not a directory, because
utils_is_file_writeable() [1] returns TRUE in this case, but write will still fail. Perhaps it should rather be fixed in utils_is_file_writeable()?
Could be done. Right now, utils_is_file_writable() has some logic to check whether the directory can be written if the given filename doesn't exist. And we use this check even in main.c:create_config_dir(). But I'd agree to split this code into two functions: utils_is_file_writable() and utils_is_directory_writable()
- check the return value of write_config(). This needed a little more
tuning for later calls to update_config() to work correctly, but now if write actually fails, and even if update_config() reported no error, an error will be displayed to the user and she will have a chance to retry.
Thanks for the patch, finally committed.
There is a last write_config() that may file silently though, the one in project_close(); not sure how this may be fixed.
I added a console warning at least for this case.
[1] shouldn't it be renamed utils_is_file_writable(), without the "e"? Not sure though, but it looks weird to me and my spellchecker complains about it...
Oops. You are right. The funny thing is, usually I'm using two different dictionaries for German<->English. The one, freedict, knows about writeable but not writable. The other one (it's just called german-english) knows only about writable but not writeable. And my spellchecker also knows only writable so that one won :D. Maybe Nick or Lex as native speakers can shed some light. For the meantime, I changed the function name to utils_is_file_writable() hoping this is more correct.
Regards, Enrico