On 13/05/16 19:27, David Turner wrote:
> From: Nguyễn Thái Ngọc Duy <pclo...@gmail.com>
[snip]

> +
> +static void reply_to_poke(int client_fd, const char *pid_buf)
> +{
> +     char *capabilities;
> +     struct strbuf sb = STRBUF_INIT;
> +
> +#ifdef USE_WATCHMAN
> +     pid_t client_pid = strtoull(pid_buf, NULL, 10);
> +
> +     prepare_index(client_pid);
> +#endif
> +     capabilities = strchr(pid_buf, ' ');

So, if the pid is *not* followed by a space, the capabilities
will be NULL here, and ...

> +
> +     if (!strcmp(capabilities, " watchman"))

... we segfault here.

> +#ifdef USE_WATCHMAN
> +             packet_buf_write(&sb, "OK watchman");
> +#else
> +             packet_buf_write(&sb, "NAK watchman");
> +#endif
> +     else
> +             packet_buf_write(&sb, "OK");
> +     if (write_in_full(client_fd, sb.buf, sb.len) != sb.len)
> +             warning(_("client write failed"));
> +}
> +
>  static void loop(int fd, int idle_in_seconds)
>  {
>       assert(idle_in_seconds < INT_MAX / 1000);
> @@ -252,11 +341,15 @@ static void loop(int fd, int idle_in_seconds)
>                       buf[bytes_read] = 0;
>                       if (!strcmp(buf, "refresh")) {
>                               refresh();
> -                     } else if (!strcmp(buf, "poke")) {
> -                             /*
> -                              * Just a poke to keep us
> -                              * alive, nothing to do.
> -                              */
> +                     } else if (starts_with(buf, "poke")) {
> +                             if (buf[4] == ' ') {
> +                                     reply_to_poke(client_fd, buf + 5);
> +                             } else {
> +                                     /*
> +                                      * Just a poke to keep us
> +                                      * alive, nothing to do.
> +                                      */
> +                             }
>                       } else {
>                               warning("BUG: Bogus command %s", buf);
>                       }
> diff --git a/read-cache.c b/read-cache.c
> index 1719f5a..8ec4be3 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -1235,7 +1235,7 @@ int refresh_index(struct index_state *istate, unsigned 
> int flags,
>               if (!new) {
>                       const char *fmt;
>  
> -                     if (really && cache_errno == EINVAL) {
> +                     if (really || cache_errno == EINVAL) {
>                               /* If we are doing --really-refresh that
>                                * means the index is not valid anymore.
>                                */
> @@ -1375,11 +1375,75 @@ static int verify_hdr(const struct cache_header *hdr, 
> unsigned long size)
>       return 0;
>  }
>  
> +static struct untracked_cache_dir *find_untracked_cache_dir(
> +     struct untracked_cache *uc, struct untracked_cache_dir *ucd,
> +     const char *name)
> +{
> +     int component_len;
> +     const char *end;
> +     struct untracked_cache_dir *dir;
> +
> +     if (!*name)
> +             return ucd;
> +
> +     end = strchr(name, '/');
> +     if (end)
> +             component_len = end - name;
> +     else
> +             component_len = strlen(name);
> +
> +     dir = lookup_untracked(uc, ucd, name, component_len);
> +     if (dir)
> +             return find_untracked_cache_dir(uc, dir, name + component_len + 
> 1);
> +
> +     return NULL;
> +}
> +
>  static void mark_no_watchman(size_t pos, void *data)
>  {
>       struct index_state *istate = data;
> +     struct cache_entry *ce = istate->cache[pos];
> +     struct strbuf sb = STRBUF_INIT;
> +     char *c;
> +     struct untracked_cache_dir *dir;
> +
>       assert(pos < istate->cache_nr);
> -     istate->cache[pos]->ce_flags |= CE_WATCHMAN_DIRTY;
> +     ce->ce_flags |= CE_WATCHMAN_DIRTY;
> +
> +     if (!istate->untracked || !istate->untracked->root)
> +             return;
> +
> +     strbuf_add(&sb, ce->name, ce_namelen(ce));
> +
> +     for (c = sb.buf + sb.len - 1; c > sb.buf; c--) {
> +             if (*c == '/') {
> +                     strbuf_setlen(&sb, c - sb.buf);
> +                     break;
> +             }
> +     }
> +
> +     if (c == sb.buf)
> +             strbuf_setlen(&sb, 0);
> +
> +     dir = find_untracked_cache_dir(istate->untracked,
> +                                    istate->untracked->root, sb.buf);
> +     if (dir)
> +             dir->valid = 0;
> +
> +     strbuf_release(&sb);
> +}
> +
> +static int mark_untracked_invalid(struct string_list_item *item, void *uc)
> +{
> +     struct untracked_cache *untracked = uc;
> +     struct untracked_cache_dir *dir;
> +
> +     dir = find_untracked_cache_dir(untracked, untracked->root,
> +                                    item->string);
> +     if (dir)
> +             dir->valid = 0;
> +
> +     return 0;
>  }
>  
>  static int read_watchman_ext(struct index_state *istate, const void *data,
> @@ -1409,10 +1473,24 @@ static int read_watchman_ext(struct index_state 
> *istate, const void *data,
>       ewah_each_bit(bitmap, mark_no_watchman, istate);
>       ewah_free(bitmap);
>  
> -     /*
> -      * TODO: update the untracked cache from the untracked data in this
> -      * extension.
> -      */
> +     if (istate->untracked && istate->untracked->root) {
> +             int i;
> +             const char *untracked;
> +
> +             untracked = (const char *)data + len + 8 + bitmap_size;
> +             for (i = 0; i < untracked_nr; ++i) {
> +                     int len = strlen(untracked);
> +                     
> string_list_append(&istate->untracked->invalid_untracked,
> +                                        untracked);
> +                     untracked += len + 1;
> +             }
> +
> +             for_each_string_list(&istate->untracked->invalid_untracked,
> +                      mark_untracked_invalid, istate->untracked);
> +
> +             if (untracked_nr)
> +                     istate->cache_changed |= WATCHMAN_CHANGED;
> +     }
>       return 0;
>  }
>  
> @@ -1645,29 +1723,88 @@ static void post_read_index_from(struct index_state 
> *istate)
>       tweak_untracked_cache(istate);
>  }
>  
> +/* in ms */
> +#define WATCHMAN_TIMEOUT 1000
> +
> +static int poke_and_wait_for_reply(int fd)
> +{
> +     int ret = -1;
> +     struct pollfd pollfd;
> +     int bytes_read;
> +     char reply_buf[4096];
> +     const char *requested_capabilities = "";
> +
> +#ifdef USE_WATCHMAN
> +     requested_capabilities = "watchman";
> +#endif
> +
> +     if (fd < 0)
> +             return -1;
> +
> +     if (packet_write_gently(fd, "poke %d %s", getpid(), 
> requested_capabilities))

So, adding the empty capabilities (and more importantly the
separating space) is not so much 'doesn't hurt', rather than 
'prevents a core-dump!' ;-)

> +             return -1;
> +     if (packet_flush_gently(fd))
> +             return -1;

And yes, I'd forgotten about the 'maybe sometime in the future, we
could buffer the packets' ...

ATB,
Ramsay Jones

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to