GNOME Bugzilla – Bug 724608
Convert all occurrences of "stat()" to GFileInfo in prefs.c
Last modified: 2014-02-18 17:37:50 UTC
prefs.c uses stat() from stdio. It should be replaced with GFileInfo from GIO. This will bring support to more file systems and also will be totally crossplatform.
Created attachment 269506 [details] [review] This patch will change occurrences of stat from stdio to GFileInfo from GIO I removed following lines #ifdef G_OS_WIN32 /* On win32 : stat("c:\path\to\dir") succeed, while stat("c:\path\to\dir\") * fails. */ ET_Win32_Path_Remove_Trailing_Backslash(path_real); #endif /* G_OS_WIN32 */ As, stat is not used. So, I don't think there should be any use of them.
Review of attachment 269506 [details] [review]: Looks good in general. (In reply to comment #1) > I removed following lines > > #ifdef G_OS_WIN32 > /* On win32 : stat("c:\path\to\dir") succeed, while stat("c:\path\to\dir\") > * fails. > */ > ET_Win32_Path_Remove_Trailing_Backslash(path_real); > #endif /* G_OS_WIN32 */ > > As, stat is not used. So, I don't think there should be any use of them. OK, while this might be true, it needs testing. It is probably correct that when creating a GFile from a path, extra slashes will not affect the validity of the GFIle. From the GFile documentation (https://developer.gnome.org/gio/stable/GFile.html#GFile.description): “Note that GFile does some trivial canonicalization of pathnames passed in, so that trivial differences in the path string used at creation (duplicated slashes, slash at end of path, "." or ".." path segments, etc) does not create different GFiles.” However, this still needs testing, which I will do this evening on a Windows system. ::: src/prefs.c @@ +1714,3 @@ g_free(path_utf8); + + if (fileinfo) As you are checking whether "fileinfo" is valid twice, it is better to fold this into the check above, something like: if (fileinfo) { if (g_file_info_get_file_type (fileinfo) == G_FILE_TYPE_DIRECTORY)) { stuff; } g_object_unref (fileinfo); } else { more_stuff; }
Created attachment 269564 [details] [review] This patch will change occurrences of stat from stdio to GFileInfo from GIO In the above if-else statement case, following is happening. if (fileinfo && file is dir) { do stuff; } else { report error stuff; if (fileinfo) g_object_unref (fileinfo); } With what you suggested it will be like: if (fileinfo) { if (g_file_info_get_file_type (fileinfo) == G_FILE_TYPE_DIRECTORY)) { stuff; } g_object_unref (fileinfo); } else { report error stuff; } So, report error stuff will not be called if fileinfo give file type as G_FILE_TYPE_REGULAR or any other than DIRECTORY. So, I did following if (fileinfo) { if (file is directory) { stuff return TRUE; } g_object_unref (fileinfo); } report_error_stuff return FALSE; As, already in the code return statement is used.
Comment on attachment 269564 [details] [review] This patch will change occurrences of stat from stdio to GFileInfo from GIO I tested on a Windows system and this works fine, thanks! Pushed to master, and slightly modified, as commit 89711f22cb736d979612f1f55e447cc3338bc7d7.