Created attachment 241643 [details] [review]
Fixed the above bug moreover fixed when user tries to browse a directory which is denied then its sub directories are removed from BrowserTree

Review of attachment 241643 [details] [review]:

This works, but has a few problems. Firstly, the directory without read permissions should have an emblem added to the icon indicating that it is not possible to read the contents of that directory. In general, do not change the coding style of lines that you are not otherwise changing, and for your new changes please make sure to look at recent commits to EasyTAG to see what the style should be.

::: src/browser.c
@@ +952,3 @@
+                g_string_append_printf (path_so_far, "%s/", parts[index1]);
+            //As dir name wasn't found in any node, so check whether it exists or not
+            if ( (dir=opendir (path_so_far->str)) != NULL)

This is in a function with a comment above it which mentions that the function does not read the directory. It would be best to keep it that way.

Created attachment 241671 [details] [review]
Fixed the problem of icon. 

I also, changed the code according to the commits. I hope this time coding style will be good.

Review of attachment 241671 [details] [review]:

::: src/browser.c
@@ +944,3 @@
+
+            //To check if dir exists or not
+            if ( (dir=opendir (path_so_far->str)) != NULL)

You are still reading a directory in a function where the comment above the function says that this function does not read a directory. As I have already mentioned this, either change the code to not read the directory, or give good reasoning why you cannot do that.

@@ +1014,2 @@
     rootPath = gtk_tree_model_get_path(GTK_TREE_MODEL(directoryTreeModel), &parentNode);
+    if(rootPath)

It looks like there are still whitespace changes to lines that you have not otherwise touched.

Created attachment 241686 [details] [review]
This patch solves all above problems

Actually, the cases taken there is if directory is not present in the BrowserTree. this could happen if either directory doesn't exist or in the case given in the bug. For this I have to make sure that directory exist, hence I earlier used g_dir_open. but now I used stat(). This will not open directory and will find if it exists or not.

Review of attachment 241686 [details] [review]:

::: src/browser.c
@@ +795,3 @@
             {
+                struct stat stat_buf;
+                if (stat(pathName, &stat_buf) == 0)

Rather than using stat, use GFile and g_file_query_file_type.

@@ +908,3 @@
     // Expand root node (fill parentNode and rootPath)
 #ifdef G_OS_WIN32
+    if (!Browser_Win32_Get_Drive_Root (parts[0], &parentNode, &rootPath))

There are still unnecessary whitespace changes. Remove them.

@@ +939,3 @@
+
+            //To check if dir exists or not
+            if (stat (path_so_far->str, &stat_buf) == 0 && S_ISDIR (stat_buf.st_mode))

Again, GFile and g_file_query_file_type.

@@ +949,3 @@
+                //Create a new node for this dir name
+                icon = get_gicon_for_path (path_so_far->str);
+                gtk_tree_store_append (GTK_TREE_STORE (directoryTreeModel),

Use gtk_tree_store_insert_with_values instead of appending a row and then setting it.

Created attachment 241717 [details] [review]
New patch, having all the modifications told earlier

I hope this will work.

Review of attachment 241717 [details] [review]:

This does not quite work. If I view a directory "a/b", where 'a' is not marked readable and "a/b" is readable, the result is fine. Then, if I collapse the 'a' part of the tree view and expand it, "a/b" disappears and there is an empty row in the tree.

Created attachment 241800 [details] [review]
Oh!! God I forgot about this case. Fixed it too.

Review of attachment 241800 [details] [review]:

If I use the usual "a/b" example, the neither 'b' nor 'a' are using the "folder-open" icon, even though every other open directory in the tree has the correct icon.

Created attachment 241880 [details] [review]
changed the icon to be displayed to closed icon.

Review of attachment 241880 [details] [review]:

Now the "unreadable" emblem is not shown correctly on the 'a' directory. Additionally, there are still lots of coding style problems.

::: src/browser.c
@@ +796,3 @@
             if (gtk_tree_selection_get_selected(selection, NULL, &selectedIter))
             {
+                GFile *_gfile;

Rather than "_gfile", use "file".

@@ +806,3 @@
                     selectedPath = gtk_tree_model_get_path(GTK_TREE_MODEL(directoryTreeModel), &parentIter);
                     gtk_tree_selection_select_iter (selection, &parentIter);
+                    if (gtk_tree_model_iter_children (GTK_TREE_MODEL (directoryTreeModel), &child_iter, &selectedIter) == FALSE &&

This line needs breaking up, as otherwise I cannot review the patch correctly (it extends off my screen during the review process).

@@ +810,2 @@
                     {
+                        gtk_tree_view_collapse_row(GTK_TREE_VIEW(BrowserTree),selectedPath);

Space before the opening parentheses, space after the comma.

@@ +812,3 @@
+                        if (OPEN_SELECTED_BROWSER_NODE)
+                        {
+                            gtk_tree_view_expand_row(GTK_TREE_VIEW(BrowserTree),selectedPath,FALSE);

Space before the opening parentheses, space after the comma.

@@ +814,3 @@
+                            gtk_tree_view_expand_row(GTK_TREE_VIEW(BrowserTree),selectedPath,FALSE);
+                        }
+                        gtk_tree_path_free(selectedPath);

Space before the opening parenthsis.

@@ +934,3 @@
+            int index1;
+            GString *path_so_far;
+            GFile *_gfile;

"file" rather than "_gfile".

@@ +939,3 @@
+
+            for (index1 = 0; index1<=index; index1++)
+                g_string_append_printf (path_so_far,"%s/",parts[index1]);

You can just use current_path, rather than this construction.

@@ +954,3 @@
+                icon = get_gicon_for_path (path_so_far->str);
+                if (check_for_subdir (path_so_far->str))
+                    has_subdir = TRUE;

Why do you use this awkward conditional? Replace it with "has_subdir = check_for_subdir (current_path);".

@@ +2994,3 @@
+        // remove dummy node
+        gtk_tree_model_iter_children(GTK_TREE_MODEL(directoryTreeModel), &subNodeIter, iter);
+        gtk_tree_store_remove(directoryTreeModel, &subNodeIter);

Spaces before opening parentheses.

@@ +3032,3 @@
     GIcon *icon;
+    GFile *_gfile;
+    GFileInfo *_gfileinfo;

Use "file" and "fileinfo".

@@ +3034,3 @@
+    GFileInfo *_gfileinfo;
+    GError *error;
+    error = NULL;

GError *error = NULL;

@@ +3046,3 @@
+                                    G_FILE_QUERY_INFO_NONE, NULL, &error);
+
+    if (!error && !g_file_info_get_attribute_boolean (_gfileinfo, G_FILE_ATTRIBUTE_ACCESS_CAN_READ))

As g_file_query_info() returns NULL on error, and you do not print the message contained in the error, just pass NULL and check if "fileinfo" is NULL.

Created attachment 242117 [details] [review]
Did, all the changes you said except using path_so_far. 

The reason I used path_so_far is current_path is the path which has to be traversed. Take for example, "a/b/c" with b and c having read permissions, then current_path will refer to "a/b/c", but I also need to construct node "b" before "c", but "b" cannot be constructed with current_path hence, path_so_far is used to refer to the path so far being traversed by BrowserTree.
Also, now the icon displayed for "a" will be a folder-open icon with "x" emblem. 
Also, fixed the coding style problems as you said.

Review of attachment 242117 [details] [review]:

::: src/browser.c
@@ +3001,3 @@
+        GIcon *folder_open_icon, *unreadable_icon;
+
+        folder_open_icon = g_themed_icon_new ("folder-open");

You have essentially duplicated get_gicon_for_path() in creating this emblemed icon, so you should find a way to use that instead.

(In reply to comment #14)
> You have essentially duplicated get_gicon_for_path() in creating this emblemed
> icon, so you should find a way to use that instead.

I improved get_gicon_for_path() so that it should be easier to do what you need:

https://git.gnome.org/browse/easytag/commit/?id=4bbfe020cac253be741c6d600533c2ff2df808dc

Created attachment 242235 [details] [review]
Did, all the changes!! 

Oh!! Thank you for that :)

Review of attachment 242235 [details] [review]:

There are still quite a few coding style issues, which you should be capable of fixing yourself by now.

::: src/browser.c
@@ +953,3 @@
+
+            for(index1 = 0; index1<=index; index1++)
+                g_string_append_printf (path_so_far, "%s/", parts[index1]);

Rather than doing this with GString (and using format strings!) you should instead use strrchr() and g_strndup().

Created attachment 242268 [details] [review]
Used g_strndup

Oh!! Sorry, for some coding style problems, corrected them too.

Review of attachment 242268 [details] [review]:

::: src/browser.c
@@ +810,3 @@
+                GFile *file;
+
+                file = g_file_new_for_path (pathName);

I do not see where you are unreffing this file. Are you?

@@ +953,3 @@
+
+            for(index1 = 0; index1<=index; index1++)
+                g_string_append_printf (path_so_far, "%s/", parts[index1]);

You are still not using g_strndup here.

@@ +966,3 @@
+                //Create a new node for this dir name
+                icon = get_gicon_for_path (path_so_far->str,ET_PATH_STATE_CLOSED);
+                has_subdir = check_for_subdir (path_so_far->str);

You can move this check_for_subdir() call into the gtk_tree_store_insert_with_values() below, which will avoid the need for the "has_subdir" variable.

Created attachment 242276 [details] [review]
oh!! sorry sorry, i accidentally uploaded previos patch. I am very sorry about this.

Err.. Unreffed means?

Review of attachment 242276 [details] [review]:

::: src/browser.c
@@ +810,3 @@
+                GFile *file;
+
+                file = g_file_new_for_path (pathName);

The GFile that you create here does not seem to be unreffed, so it is leaked. You need to call g_object_unref() on "file".

@@ +821,3 @@
+                    if (gtk_tree_model_iter_has_child (GTK_TREE_MODEL (directoryTreeModel),
+                                                     &selectedIter) == FALSE &&
+                                                    !g_file_query_exists (file, NULL))

It seems like you are reducing the indentation by a space each time here.

@@ +953,3 @@
+
+            for(index1 = 0; index1<=index; index1++)
+                 path_so_far_length += strlen (parts [index1]) + 1; //1 for '/'

Replace this with strrchr(), as I already mentioned.

Created attachment 242395 [details] [review]
Fixed the bug completely. Review it please

Used g_build_path() as discussed earlier.

Review of attachment 242395 [details] [review]:

::: src/browser.c
@@ +980,3 @@
+            }
+            else
+                break;

Because you break here, you do not unref "file" nor do you free "path_so_far".

@@ +3072,3 @@
+        g_object_unref (file);
+        g_object_unref (fileinfo);
+        return;

You do not free "path" here.

Created attachment 242399 [details] [review]
Fixed the memory leaks of my previous patch.

Comment on attachment 242399 [details] [review]
Fixed the memory leaks of my previous patch.

Thanks, I pushed a modified patch to master as commit 77485d2b62060f9311a33b4cec820793834a07d9.