-
Notifications
You must be signed in to change notification settings - Fork 1
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
add patches for memleak and missing stat in cached readdir; rel 2
from: - libfuse/sshfs#305 - libfuse/sshfs#306
- Loading branch information
Showing
3 changed files
with
172 additions
and
1 deletion.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
From 8bb9d33d16f6004cc0420592b0dcf78571cdc746 Mon Sep 17 00:00:00 2001 | ||
From: Jan Palus <[email protected]> | ||
Date: Mon, 17 Jun 2024 00:08:34 +0200 | ||
Subject: [PATCH] Fix memleak in cache after readlink | ||
|
||
--- | ||
cache.c | 1 + | ||
1 file changed, 1 insertion(+) | ||
|
||
diff --git a/cache.c b/cache.c | ||
index 9436c5a7..70dc35b4 100644 | ||
--- a/cache.c | ||
+++ b/cache.c | ||
@@ -67,6 +67,7 @@ static void free_node(gpointer node_) | ||
{ | ||
struct node *node = (struct node *) node_; | ||
g_strfreev(node->dir); | ||
+ g_free(node->link); | ||
g_free(node); | ||
} | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,146 @@ | ||
From 5f767dec5b3fc07f665fdd4fbd9eb728a790f35f Mon Sep 17 00:00:00 2001 | ||
From: Jan Palus <[email protected]> | ||
Date: Mon, 17 Jun 2024 11:48:44 +0200 | ||
Subject: [PATCH] Fill stat info when returning cached data for readdir | ||
|
||
Uncached and cached results for readdir were inconsistent -- the former | ||
returned correct stat info for directory entries while the latter | ||
didn't. That's because only names of entries were saved in cache without | ||
stat info. In turn this leads to issues like | ||
https://github.com/junegunn/fzf/issues/3832 since directory traversal | ||
library (https://github.com/charlievieth/fastwalk in this case) relies | ||
on proper stat info returned by readdir. Hence when unchached result was | ||
returned it gave proper outcome, while with cached result it was wrong. | ||
|
||
Cache stat info next to entry name to fix the issue. While file | ||
attributes are saved in cache already, they use full path as key. To | ||
avoid potentially plenty of allocations, string copying and cache | ||
lookups to get each attr, let's keep a copy of stat struct independently | ||
to be on the fast path. | ||
--- | ||
cache.c | 43 +++++++++++++++++++++++++++++++------------ | ||
1 file changed, 31 insertions(+), 12 deletions(-) | ||
|
||
diff --git a/cache.c b/cache.c | ||
index 9436c5a7..c57829dc 100644 | ||
--- a/cache.c | ||
+++ b/cache.c | ||
@@ -14,6 +14,7 @@ | ||
#include <errno.h> | ||
#include <glib.h> | ||
#include <pthread.h> | ||
+#include <sys/stat.h> | ||
|
||
#define DEFAULT_CACHE_TIMEOUT_SECS 20 | ||
#define DEFAULT_MAX_CACHE_SIZE 10000 | ||
@@ -40,7 +41,7 @@ static struct cache cache; | ||
struct node { | ||
struct stat stat; | ||
time_t stat_valid; | ||
- char **dir; | ||
+ GPtrArray *dir; | ||
time_t dir_valid; | ||
char *link; | ||
time_t link_valid; | ||
@@ -63,14 +64,28 @@ struct file_handle { | ||
unsigned long fs_fh; | ||
}; | ||
|
||
+struct cache_dirent { | ||
+ char *name; | ||
+ struct stat stat; | ||
+}; | ||
+ | ||
static void free_node(gpointer node_) | ||
{ | ||
struct node *node = (struct node *) node_; | ||
- g_strfreev(node->dir); | ||
+ if (node->dir != NULL) | ||
+ g_ptr_array_free(node->dir, TRUE); | ||
g_free(node->link); | ||
g_free(node); | ||
} | ||
|
||
+static void free_cache_dirent(gpointer data) { | ||
+ struct cache_dirent *cache_dirent = (struct cache_dirent *) data; | ||
+ if (cache_dirent != NULL) { | ||
+ g_free(cache_dirent->name); | ||
+ g_free(cache_dirent); | ||
+ } | ||
+} | ||
+ | ||
static int cache_clean_entry(void *key_, struct node *node, time_t *now) | ||
{ | ||
(void) key_; | ||
@@ -186,13 +201,14 @@ void cache_add_attr(const char *path, const struct stat *stbuf, uint64_t wrctr) | ||
pthread_mutex_unlock(&cache.lock); | ||
} | ||
|
||
-static void cache_add_dir(const char *path, char **dir) | ||
+static void cache_add_dir(const char *path, GPtrArray *dir) | ||
{ | ||
struct node *node; | ||
|
||
pthread_mutex_lock(&cache.lock); | ||
node = cache_get(path); | ||
- g_strfreev(node->dir); | ||
+ if (node->dir != NULL) | ||
+ g_ptr_array_free(node->dir, TRUE); | ||
node->dir = dir; | ||
node->dir_valid = time(NULL) + cache.dir_timeout_secs; | ||
if (node->dir_valid > node->valid) | ||
@@ -341,7 +357,10 @@ static int cache_dirfill (void *buf, const char *name, | ||
ch = (struct readdir_handle*) buf; | ||
err = ch->filler(ch->buf, name, stbuf, off, flags); | ||
if (!err) { | ||
- g_ptr_array_add(ch->dir, g_strdup(name)); | ||
+ struct cache_dirent *cdent = g_malloc(sizeof(struct cache_dirent)); | ||
+ cdent->name = g_strdup(name); | ||
+ cdent->stat = *stbuf; | ||
+ g_ptr_array_add(ch->dir, cdent); | ||
if (stbuf->st_mode & S_IFMT) { | ||
char *fullpath; | ||
const char *basepath = !ch->path[1] ? "" : ch->path; | ||
@@ -361,8 +380,9 @@ static int cache_readdir(const char *path, void *buf, fuse_fill_dir_t filler, | ||
struct readdir_handle ch; | ||
struct file_handle *cfi; | ||
int err; | ||
- char **dir; | ||
+ GPtrArray *dir; | ||
struct node *node; | ||
+ struct cache_dirent **cdent; | ||
|
||
assert(offset == 0); | ||
|
||
@@ -371,9 +391,8 @@ static int cache_readdir(const char *path, void *buf, fuse_fill_dir_t filler, | ||
if (node != NULL && node->dir != NULL) { | ||
time_t now = time(NULL); | ||
if (node->dir_valid - now >= 0) { | ||
- for(dir = node->dir; *dir != NULL; dir++) | ||
- // FIXME: What about st_mode? | ||
- filler(buf, *dir, NULL, 0, 0); | ||
+ for(cdent = (struct cache_dirent**)node->dir->pdata; *cdent != NULL; cdent++) | ||
+ filler(buf, (*cdent)->name, &(*cdent)->stat, 0, 0); | ||
pthread_mutex_unlock(&cache.lock); | ||
return 0; | ||
} | ||
@@ -397,16 +416,16 @@ static int cache_readdir(const char *path, void *buf, fuse_fill_dir_t filler, | ||
ch.buf = buf; | ||
ch.filler = filler; | ||
ch.dir = g_ptr_array_new(); | ||
+ g_ptr_array_set_free_func(ch.dir, free_cache_dirent); | ||
ch.wrctr = cache_get_write_ctr(); | ||
err = cache.next_oper->readdir(path, &ch, cache_dirfill, offset, fi, flags); | ||
g_ptr_array_add(ch.dir, NULL); | ||
- dir = (char **) ch.dir->pdata; | ||
+ dir = ch.dir; | ||
if (!err) { | ||
cache_add_dir(path, dir); | ||
} else { | ||
- g_strfreev(dir); | ||
+ g_ptr_array_free(dir, TRUE); | ||
} | ||
- g_ptr_array_free(ch.dir, FALSE); | ||
|
||
return err; | ||
} |