GNOME Bugzilla – Bug 724461
Replace stat with GFileInfo in et_core.c
Last modified: 2014-02-16 20:37:36 UTC
Source code of et_core.c contains stat from stdio. Replacing stat with GFileInfo from GIO will lead to many improvements.
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.
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.
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 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.