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 724461 - Replace stat with GFileInfo in et_core.c
Replace stat with GFileInfo in et_core.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-16 06:59 UTC by Abhinav
Modified: 2014-02-16 20:37 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
This patch will change occurrences of stat from stdio to GFileInfo from GIO (6.67 KB, patch)
2014-02-16 07:06 UTC, Abhinav
needs-work Details | Review
This patch will change occurrences of stat from stdio to GFileInfo from GIO (5.13 KB, patch)
2014-02-16 12:46 UTC, Abhinav
committed Details | Review

Description Abhinav 2014-02-16 06:59:01 UTC
Source code of et_core.c contains stat from stdio. Replacing stat with GFileInfo from GIO will lead to many improvements.
Comment 1 Abhinav 2014-02-16 07:06:53 UTC
Created attachment 269277 [details] [review]
This patch will change occurrences of stat from stdio to GFileInfo from GIO

There may be some coding style violations. I didn't know what to do in those cases. I did what I think is best. Please review the patch and tell me what to do in those cases. These are line numbers 54-55, 110-112, 113-117 in the patch.
Comment 2 David King 2014-02-16 11:03:47 UTC
Review of attachment 269277 [details] [review]:

The coding style looks nice and consistent, thanks!
I think that most of the fetching and setting of attributes can be left out, as EasyTAG should probably not be trying to override what the filesystem is doing. However, this will need a bit of testing.

::: src/et_core.c
@@ +611,3 @@
+    else
+    {
+        Log_Print (LOG_ERROR,

I do not think that there is much point in logging the error here. If the modification time could not be retrieved, g_file_info_get_attribute_uint64() will return 0, which can be stored in ETFile->FileModificationTime without problems.

@@ +3933,2 @@
     // Save permissions and dates of the file (cause they may change with files on NFS)
+    file = g_file_new_for_path (cur_filename);

I do not think that it makes sense for EasyTAG to be manually fetching and setting these attributes. This logic should just be removed entirely, and be left to the filesystem (after testing to see that the results are as expected).

@@ +3995,3 @@
     {
 #ifndef G_OS_WIN32
+        chmod (cur_filename,

Likewise, this should be left to the filesystem.

@@ +4018,1 @@
             utime(cur_filename,&utimbufbuf);

For preserving the timestamps, it is probably better to fetch all the time attributes from the file (with the attribute "time::*") and then set them on the file with g_file_set_attributes_from_info().

@@ +4033,3 @@
+        g_object_unref (fileinfo);
+    }
+    g_object_unref (file);

Leave a blank line between the closing brace and the unref.
Comment 3 Abhinav 2014-02-16 12:46:02 UTC
Created attachment 269300 [details] [review]
This patch will change occurrences of stat from stdio to GFileInfo from GIO

I have applied all the modifications from previous post.
I also tested it. It seems to work fine for me.
Comment 4 David King 2014-02-16 20:37:22 UTC
Comment on attachment 269300 [details] [review]
This patch will change occurrences of stat from stdio to GFileInfo from GIO

Thanks for the updated patch. I made some minor changes and pushed the patch to master as 2c803c58791645af443a20039df1b9478f83c0ff.