[PATCH][RFC] fix directory listing when filesystem partially corrupted

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH][RFC] fix directory listing when filesystem partially corrupted

Jean-Tiare Le Bigot
When opening a folder on a partially corrupted filesystem, Thunar
displays an error dialog and leaves the folder content area empty.
This behavior is consistent with Nautilus.

The error message looks like:

    Failed to open directory "Toc toc toc ?".
    Error when getting information for file '/media/jean-tiare/Toc toc toc ?/.Trash-1000': Input/output error.

When listing the same directory with ls, it gracefuly fallback to
something like: (non-relevant output redacted)

    $ LC_ALL=C ls -la
    ls: cannot access .Trash-1000: Input/output error
    ls: cannot access .gnupg: Input/output error
    total 320
    drwx------  7 jean-tiare jean-tiare   4096 Aug 30 12:49 .
    drwxr-x---+ 4 root       root         4096 Aug 30 17:34 ..
    d?????????? ? ?          ?               ?            ? .Trash-1000
    d?????????? ? ?          ?               ?            ? .gnupg
    drwx------  4 jean-tiare jean-tiare   4096 Mar  2  2014 .ssh
    drwx------  2 root       root        16384 Dec 27  2012 lost+found
    drwx------  4 jean-tiare jean-tiare   4096 Jan  7  2013 profil

This patch attempts to reproduce a similar behavior by silently skipping
entries when meating a G_IO_ERROR_FAILED error so that sane entries may
be displayed.

It won't display an error message (although I'm sure why), and won't display
dummy entries either.

Also, I'm new with gobject(?) based program and obviously did something wrong
as I get warning messages in the console:

    LC_ALL=C ./thunar/thunar

    (lt-thunar:29769): GLib-WARNING **: GError set over the top of a previous GError or uninitialized memory.
    This indicates a bug in someone's code. You must ensure an error is NULL before it's set.
    The overwriting error message was: Error when getting information for file '/media/jean-tiare/Toc toc toc ?/.gnupg': Input/output error

--
Jean-Tiare

Jean-Tiare Le Bigot (1):
  fix: list partially corrupted folder

 thunar/thunar-io-jobs.c           |  2 +-
 thunar/thunar-io-scan-directory.c | 12 +++++++++++-
 2 files changed, 12 insertions(+), 2 deletions(-)

--
2.1.4

_______________________________________________
Xfce4-dev mailing list
[hidden email]
https://mail.xfce.org/mailman/listinfo/xfce4-dev
Reply | Threaded
Open this post in threaded view
|

[PATCH] fix: list partially corrupted folder

Jean-Tiare Le Bigot
---
 thunar/thunar-io-jobs.c           |  2 +-
 thunar/thunar-io-scan-directory.c | 12 +++++++++++-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/thunar/thunar-io-jobs.c b/thunar/thunar-io-jobs.c
index 9d904f9..00d6a49 100644
--- a/thunar/thunar-io-jobs.c
+++ b/thunar/thunar-io-jobs.c
@@ -1197,7 +1197,7 @@ _thunar_io_jobs_ls (ThunarJob  *job,
   if (err != NULL)
     {
       g_propagate_error (error, err);
-      return FALSE;
+      g_clear_error(&err);
     }
   else if (exo_job_set_error_if_cancelled (EXO_JOB (job), &err))
     {
diff --git a/thunar/thunar-io-scan-directory.c b/thunar/thunar-io-scan-directory.c
index 66ddbdf..269a440 100644
--- a/thunar/thunar-io-scan-directory.c
+++ b/thunar/thunar-io-scan-directory.c
@@ -112,7 +112,17 @@ thunar_io_scan_directory (ThunarJob          *job,
                                           &err);
 
       if (G_UNLIKELY (info == NULL))
-        break;
+        {
+          /* silently ignore broken file entries */
+          if (g_error_matches (err, G_IO_ERROR, G_IO_ERROR_FAILED))
+            {
+              g_propagate_error (error, err);
+              g_clear_error (&err);
+              continue;
+            }
+          break;
+        }
+
 
       is_mounted = TRUE;
       if (err != NULL)
--
2.1.4

_______________________________________________
Xfce4-dev mailing list
[hidden email]
https://mail.xfce.org/mailman/listinfo/xfce4-dev
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] fix: list partially corrupted folder

Jonas Kümmerlin
Am Sonntag, den 30.08.2015, 18:44 +0200 schrieb Jean-Tiare Le Bigot:
>    if (err != NULL)
>      {
>        g_propagate_error (error, err);
> -      return FALSE;
> +      g_clear_error(&err);
Do NOT ever do anything with err after you propagated it with
g_propagate_error(); g_propagate_error() invalidates the source. The only thing
you may (and have to if you plan to use the variable after that) do is simply
set err = NULL. (Hint: The docs explicitly spell this out. Read them.)
Everything else will very likely lead to memory corruption sooner or later.

> +          /* silently ignore broken file entries */
> +          if (g_error_matches (err, G_IO_ERROR, G_IO_ERROR_FAILED))
> +            {
> +              g_propagate_error (error, err);
> +              g_clear_error (&err);
same

The GError warnings could be caused by not initializing error pointers (grep for
"GError +\*[[:alnum:]]+;"), or by reusing an error variable without clearing it
before. You MUST avoid passing an already initialized GError* as out parameter
into a function; propagating errors in a loop violates this.

You can learn more about error reporting in GLib here:
https://developer.gnome.org/glib/stable/glib-Error-Reporting.html

Read it carefully; the detailed semantics are important.
_______________________________________________
Xfce4-dev mailing list
[hidden email]
https://mail.xfce.org/mailman/listinfo/xfce4-dev
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] fix: list partially corrupted folder

Jean-Tiare Le Bigot
Apparently, we ca't do much better than silently ignoring. I would have
prefered to yield fake/best-effort entries to keep file count right BUT
GFileEnumerator returns no data in case there were any error, not even
the filename (or did I miss something ?).

 - g_file_enumerator_get_container returns *parent* folder
 - g_file_enumerator_get_child requires a valid info

Hence, with your remarks applied, the patch would be:

diff --git a/thunar/thunar-io-scan-directory.c
b/thunar/thunar-io-scan-directory.c
index 66ddbdf..66d62c3 100644
--- a/thunar/thunar-io-scan-directory.c
+++ b/thunar/thunar-io-scan-directory.c
@@ -112,7 +112,16 @@ thunar_io_scan_directory (ThunarJob          *job,
                                           &err);

       if (G_UNLIKELY (info == NULL))
-        break;
+        {
+          /* silently ignore broken file entries */
+          if (g_error_matches (err, G_IO_ERROR, G_IO_ERROR_FAILED))
+            {
+              g_clear_error (&err);
+              continue;
+            }
+          break;
+        }
+

       is_mounted = TRUE;
       if (err != NULL)


Let me know if you want a properly git send-email formated patch.

Le 31/08/2015 23:08, Jonas Kümmerlin a écrit :

> Am Sonntag, den 30.08.2015, 18:44 +0200 schrieb Jean-Tiare Le Bigot:
>>    if (err != NULL)
>>      {
>>        g_propagate_error (error, err);
>> -      return FALSE;
>> +      g_clear_error(&err);
> Do NOT ever do anything with err after you propagated it with
> g_propagate_error(); g_propagate_error() invalidates the source. The only thing
> you may (and have to if you plan to use the variable after that) do is simply
> set err = NULL. (Hint: The docs explicitly spell this out. Read them.)
> Everything else will very likely lead to memory corruption sooner or later.
>
>> +          /* silently ignore broken file entries */
>> +          if (g_error_matches (err, G_IO_ERROR, G_IO_ERROR_FAILED))
>> +            {
>> +              g_propagate_error (error, err);
>> +              g_clear_error (&err);
> same
>
> The GError warnings could be caused by not initializing error pointers (grep for
> "GError +\*[[:alnum:]]+;"), or by reusing an error variable without clearing it
> before. You MUST avoid passing an already initialized GError* as out parameter
> into a function; propagating errors in a loop violates this.
>
> You can learn more about error reporting in GLib here:
> https://developer.gnome.org/glib/stable/glib-Error-Reporting.html
>
> Read it carefully; the detailed semantics are important.
> _______________________________________________
> Xfce4-dev mailing list
> [hidden email]
> https://mail.xfce.org/mailman/listinfo/xfce4-dev
>
_______________________________________________
Xfce4-dev mailing list
[hidden email]
https://mail.xfce.org/mailman/listinfo/xfce4-dev