Re: feature/android b91e8ada70e 3/5: Fix auto-revert-mode on Android

[Michael Albinus]

Po Lu via Mailing list for Emacs changes <emacs-diffs@gnu.org> writes:

Hi,

>     Fix auto-revert-mode on Android
>
>     * src/inotify.c (Finotify_add_watch): Handle asset files.
> ---
>  src/inotify.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/src/inotify.c b/src/inotify.c
> index 7562ffb1701..b2a48884efa 100644
> --- a/src/inotify.c
> +++ b/src/inotify.c
> @@ -419,6 +419,7 @@ IN_ONESHOT  */)
>    int wd = -1;
>    uint32_t imask = aspect_to_inotifymask (aspect);
>    uint32_t mask = imask | IN_MASK_ADD | IN_EXCL_UNLINK;
> +  char *name;
>
>    CHECK_STRING (filename);
>
> @@ -432,7 +433,23 @@ IN_ONESHOT  */)
>      }
>
>    encoded_file_name = ENCODE_FILE (filename);
> -  wd = inotify_add_watch (inotifyfd, SSDATA (encoded_file_name), mask);
> +  name = SSDATA (encoded_file_name);
> +
> +#if defined HAVE_ANDROID && !defined ANDROID_STUBIFY
> +  /* If FILENAME actually lies in a special directory, return now
> +     instead of letting inotify fail.  These directories cannot
> +     receive file notifications as they are read only.  */
> +
> +  if (strcmp (name, "/assets")
> +      || strcmp (name, "/assets/")
> +      || strcmp (name, "/content")
> +      || strcmp (name, "/content/")
> +      || strncmp (name, "/assets/", sizeof "/assets")
> +      || strncmp (name, "/content", sizeof "/content"))
> +    return Qnil;
> +#endif
> +
> +  wd = inotify_add_watch (inotifyfd, name, mask);
>    if (wd < 0)
>      report_file_notify_error ("Could not add watch for file", filename);

I have two problems with this patch. First, nil is not defined as return
value of *-add-watch functions. If we want to allow this, we must extend
the interface.

Second, do we really want to have such a hard coded list in inotify.c?
What about other directories on other systems, like "/proc"? Shouldn't
we add rather a common interface for excluding directories from being
watched, say file-notify-excluded-directories (a list of strings)? This
could be modified on Lisp level, no need to do it in the notification
backends.

Best regards, Michael.