After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 724608 - Convert all occurrences of "stat()" to GFileInfo in prefs.c
Convert all occurrences of "stat()" to GFileInfo in prefs.c
Status: RESOLVED FIXED
Product: easytag
Classification: Other
Component: general
master
Other All
: Normal normal
: 2.1
Assigned To: EasyTAG maintainer(s)
EasyTAG maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2014-02-18 07:29 UTC by Abhinav
Modified: 2014-02-18 17:37 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
This patch will change occurrences of stat from stdio to GFileInfo from GIO (1.88 KB, patch)
2014-02-18 07:34 UTC, Abhinav
needs-work Details | Review
This patch will change occurrences of stat from stdio to GFileInfo from GIO (3.46 KB, patch)
2014-02-18 14:42 UTC, Abhinav
committed Details | Review

Description Abhinav 2014-02-18 07:29:21 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.
Comment 1 Abhinav 2014-02-18 07:34:12 UTC
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.
Comment 2 David King 2014-02-18 13:32:53 UTC
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;
}
Comment 3 Abhinav 2014-02-18 14:42:18 UTC
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 4 David King 2014-02-18 17:37:03 UTC
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.