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 725223 - Convert directory functions of stdio to GFile from GIO in browser.c
Convert directory functions of stdio to GFile from GIO in browser.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-26 14:34 UTC by Abhinav
Modified: 2014-02-27 08:37 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
This function converts directory functions from stdio to GFile from GIO in browser.c (8.31 KB, patch)
2014-02-26 14:37 UTC, Abhinav
reviewed Details | Review
This function converts directory functions from stdio to GFile from GIO in browser.c (8.50 KB, patch)
2014-02-26 19:06 UTC, Abhinav
needs-work Details | Review
This function converts directory functions from stdio to GFile from GIO in browser.c (8.58 KB, patch)
2014-02-27 05:51 UTC, Abhinav
committed Details | Review

Description Abhinav 2014-02-26 14:34:19 UTC
Converting directory functions of stdio like opendir, readdir etc. to GFile from GIO would bring many improvements.
Comment 1 Abhinav 2014-02-26 14:37:17 UTC
Created attachment 270389 [details] [review]
This function converts directory functions from stdio to GFile from GIO in browser.c

I would have made check_for_subdir function to accept GFile instead of file path, but that would bring a problem in line 896, Browser_Tree_Select_Dir. This function doesn't create a GFile for current_path.
Comment 2 David King 2014-02-26 17:41:11 UTC
Review of attachment 270389 [details] [review]:

Looks fine in general, just a couple of questions about the implementation.

::: src/browser.c
@@ +2731,3 @@
         {
+            if (g_file_info_get_file_type (childinfo) ==
+                G_FILE_TYPE_DIRECTORY)

You removed the code which checks for hidden directories, is that no longer necessary?

@@ +2911,3 @@
+              || !( (g_ascii_strncasecmp(name,".", 1) == 0)
+                 && (strlen(name) > 1)
+                 && (g_ascii_strncasecmp(name+1,".", 1) != 0) ))

Is is possible to use g_file_info_get_is_hidden() instead?
Comment 3 Abhinav 2014-02-26 19:06:27 UTC
Created attachment 270409 [details] [review]
This function converts directory functions from stdio to GFile from GIO in browser.c

Oh!!, I deleted the code for hidden directory accidentally. Yes, g_file_info_get_is_hidden will be good.
Comment 4 David King 2014-02-26 20:47:15 UTC
Review of attachment 270409 [details] [review]:

I found a couple of memory leaks. You should investigate Valgrind as a part of your workflow:

https://wiki.gnome.org/Valgrind

It takes a bit of practice to use it effectively, but is very useful.

::: src/browser.c
@@ +2897,3 @@
+        GFileInfo *childinfo;
+
+        while ((childinfo = g_file_enumerator_next_file (enumerator,

You do not seem to unref childinfo in all cases, so this is a memory leak.

@@ +2907,3 @@
+            name = g_file_info_get_name (childinfo);
+            child = g_file_get_child (dir, name);
+            fullpath_file = g_file_get_path (child);

I do not think that you free fullpath_file later on, so this is a memory leak.
Comment 5 Abhinav 2014-02-27 05:51:37 UTC
Created attachment 270445 [details] [review]
This function converts directory functions from stdio to GFile from GIO in browser.c

The memory leak was in "check_for_subdirs". I didn't freed childinfo there.
Comment 6 David King 2014-02-27 08:37:13 UTC
Comment on attachment 270445 [details] [review]
This function converts directory functions from stdio to GFile from GIO in browser.c

Thanks, that fixes the leaks. I pushed a slightly-modified patch to master as 069d2d755235d6c8549a6baed9a0f18c1e61f549.