From: Sergei Trofimovich <[email protected]>
Hi guys!
TL;DR:
the patches remove (currently or already) dead code
and localize symbol visibility used only in one module.
Thanks!
The Story:
Once upon a time I've got very simple idea to find dead code
in large C/C++ mixed project at work.
The things of my primary interest were things of too large
visibility scope, like:
- externally visible global variables used only in module
defining the variable
- and functions of the same property
Once you identify those functions and mark as 'static' you
achieve two goals:
- minor: give compiler possibility to inline code a bit better
(especially for one-caller case)
- more interesting: possibility to spot that the symbol is no
longer used at all
Hence the project idea:
1. scan all intermediate object (.o) files for exported symbols
2. find such symbols that are not imported into any .o files
Meet uselex.rb: one-file script to parse 'nm' output:
https://github.com/trofi/uselex/blob/master/uselex.rb
For linux's kernel it's use is as simple as:
$ make $config && make
$ uselex.rb `find -name '*.o'`
and you've got material to look at!
It outputs 3000 symbols thus I've started small:
$ uselex.rb `find fs/btrfs -name '*.o'
To get an impression it reports such things:
set_state_private: [R]: exported from: fs/btrfs/built-in.o fs/btrfs/btrfs.o fs/btrfs/extent_io.o
btrfs_get_inode_ref_index: [R]: exported from: fs/btrfs/built-in.o fs/btrfs/inode-item.o fs/btrfs/btrfs.o
btrfs_print_tree: [R]: exported from: fs/btrfs/built-in.o fs/btrfs/print-tree.o fs/btrfs/btrfs.o
btrfs_reada_detach: [R]: exported from: fs/btrfs/built-in.o fs/btrfs/reada.o fs/btrfs/btrfs.o
__btrfs_getxattr: [R]: exported from: fs/btrfs/built-in.o fs/btrfs/xattr.o fs/btrfs/btrfs.o
__btrfs_setxattr: [R]: exported from: fs/btrfs/built-in.o fs/btrfs/xattr.o fs/btrfs/btrfs.o
btrfs_read_root_item: [R]: exported from: fs/btrfs/root-tree.o fs/btrfs/built-in.o fs/btrfs/btrfs.o
__tracepoint_btrfs_sync_fs: [R]: exported from: fs/btrfs/super.o fs/btrfs/built-in.o fs/btrfs/btrfs.o
btrfs_start_transaction_lflush: [R]: exported from: fs/btrfs/transaction.o fs/btrfs/built-in.o fs/btrfs/btrfs.o
btrfs_write_and_wait_marked_extents: [R]: exported from: fs/btrfs/transaction.o fs/btrfs/built-in.o fs/btrfs/btrfs.o
ulist_fini: [R]: exported from: fs/btrfs/ulist.o fs/btrfs/built-in.o fs/btrfs/btrfs.o
ulist_init: [R]: exported from: fs/btrfs/ulist.o fs/btrfs/built-in.o fs/btrfs/btrfs.o
You just need to make sure it does not lie too much.
For example '__btrfs_getxattr' needs to be brought under '#ifdef CONFIG_BTRFS_FS_POSIX_ACL'
and not removed entirely.
Thanks for your patience!
From: Sergei Trofimovich <[email protected]>
Found by uselex.rb:
> btrfs_reada_detach: [R]: exported from: fs/btrfs/btrfs.o fs/btrfs/built-in.o fs/btrfs/reada.o
Signed-off-by: Sergei Trofimovich <[email protected]>
---
fs/btrfs/ctree.h | 1 -
fs/btrfs/reada.c | 9 +--------
2 files changed, 1 insertion(+), 9 deletions(-)
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index e91ab9e..f35e086 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3861,7 +3861,6 @@ struct reada_control {
struct reada_control *btrfs_reada_add(struct btrfs_root *root,
struct btrfs_key *start, struct btrfs_key *end);
int btrfs_reada_wait(void *handle);
-void btrfs_reada_detach(void *handle);
int btree_readahead_hook(struct btrfs_root *root, struct extent_buffer *eb,
u64 start, int err);
diff --git a/fs/btrfs/reada.c b/fs/btrfs/reada.c
index 1031b69..c41d470 100644
--- a/fs/btrfs/reada.c
+++ b/fs/btrfs/reada.c
@@ -37,7 +37,7 @@
* To trigger a readahead, btrfs_reada_add must be called. It will start
* a read ahead for the given range [start, end) on tree root. The returned
* handle can either be used to wait on the readahead to finish
- * (btrfs_reada_wait), or to send it to the background (btrfs_reada_detach).
+ * (btrfs_reada_wait).
*
* The read ahead works as follows:
* On btrfs_reada_add, the root of the tree is inserted into a radix_tree.
@@ -979,10 +979,3 @@ int btrfs_reada_wait(void *handle)
return 0;
}
#endif
-
-void btrfs_reada_detach(void *handle)
-{
- struct reada_control *rc = handle;
-
- kref_put(&rc->refcnt, reada_control_release);
-}
--
1.8.3.2
From: Sergei Trofimovich <[email protected]>
Found by uselex.rb:
> btrfs_get_inode_ref_index: [R]: exported from: fs/btrfs/inode-item.o fs/btrfs/btrfs.o fs/btrfs/built-in.o
Signed-off-by: Sergei Trofimovich <[email protected]>
---
fs/btrfs/ctree.h | 6 -----
fs/btrfs/inode-item.c | 65 ---------------------------------------------------
2 files changed, 71 deletions(-)
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index f35e086..4822c84 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3480,12 +3480,6 @@ int btrfs_del_inode_ref(struct btrfs_trans_handle *trans,
struct btrfs_root *root,
const char *name, int name_len,
u64 inode_objectid, u64 ref_objectid, u64 *index);
-int btrfs_get_inode_ref_index(struct btrfs_trans_handle *trans,
- struct btrfs_root *root,
- struct btrfs_path *path,
- const char *name, int name_len,
- u64 inode_objectid, u64 ref_objectid, int mod,
- u64 *ret_index);
int btrfs_insert_empty_inode(struct btrfs_trans_handle *trans,
struct btrfs_root *root,
struct btrfs_path *path, u64 objectid);
diff --git a/fs/btrfs/inode-item.c b/fs/btrfs/inode-item.c
index e0b7034..d07e982 100644
--- a/fs/btrfs/inode-item.c
+++ b/fs/btrfs/inode-item.c
@@ -91,32 +91,6 @@ int btrfs_find_name_in_ext_backref(struct btrfs_path *path, u64 ref_objectid,
return 0;
}
-static struct btrfs_inode_ref *
-btrfs_lookup_inode_ref(struct btrfs_trans_handle *trans,
- struct btrfs_root *root,
- struct btrfs_path *path,
- const char *name, int name_len,
- u64 inode_objectid, u64 ref_objectid, int ins_len,
- int cow)
-{
- int ret;
- struct btrfs_key key;
- struct btrfs_inode_ref *ref;
-
- key.objectid = inode_objectid;
- key.type = BTRFS_INODE_REF_KEY;
- key.offset = ref_objectid;
-
- ret = btrfs_search_slot(trans, root, &key, path, ins_len, cow);
- if (ret < 0)
- return ERR_PTR(ret);
- if (ret > 0)
- return NULL;
- if (!find_name_in_backref(path, name, name_len, &ref))
- return NULL;
- return ref;
-}
-
/* Returns NULL if no extref found */
struct btrfs_inode_extref *
btrfs_lookup_inode_extref(struct btrfs_trans_handle *trans,
@@ -144,45 +118,6 @@ btrfs_lookup_inode_extref(struct btrfs_trans_handle *trans,
return extref;
}
-int btrfs_get_inode_ref_index(struct btrfs_trans_handle *trans,
- struct btrfs_root *root,
- struct btrfs_path *path,
- const char *name, int name_len,
- u64 inode_objectid, u64 ref_objectid, int mod,
- u64 *ret_index)
-{
- struct btrfs_inode_ref *ref;
- struct btrfs_inode_extref *extref;
- int ins_len = mod < 0 ? -1 : 0;
- int cow = mod != 0;
-
- ref = btrfs_lookup_inode_ref(trans, root, path, name, name_len,
- inode_objectid, ref_objectid, ins_len,
- cow);
- if (IS_ERR(ref))
- return PTR_ERR(ref);
-
- if (ref != NULL) {
- *ret_index = btrfs_inode_ref_index(path->nodes[0], ref);
- return 0;
- }
-
- btrfs_release_path(path);
-
- extref = btrfs_lookup_inode_extref(trans, root, path, name,
- name_len, inode_objectid,
- ref_objectid, ins_len, cow);
- if (IS_ERR(extref))
- return PTR_ERR(extref);
-
- if (extref) {
- *ret_index = btrfs_inode_extref_index(path->nodes[0], extref);
- return 0;
- }
-
- return -ENOENT;
-}
-
static int btrfs_del_inode_extref(struct btrfs_trans_handle *trans,
struct btrfs_root *root,
const char *name, int name_len,
--
1.8.3.2
From: Sergei Trofimovich <[email protected]>
Found by uselex.rb:
> btrfs_read_root_item: [R]: exported from: fs/btrfs/btrfs.o fs/btrfs/built-in.o fs/btrfs/root-tree.o
Signed-off-by: Sergei Trofimovich <[email protected]>
---
fs/btrfs/ctree.h | 2 --
fs/btrfs/root-tree.c | 4 ++--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index e795bf1..e91ab9e 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3414,8 +3414,6 @@ int __must_check btrfs_update_root(struct btrfs_trans_handle *trans,
struct btrfs_root *root,
struct btrfs_key *key,
struct btrfs_root_item *item);
-void btrfs_read_root_item(struct extent_buffer *eb, int slot,
- struct btrfs_root_item *item);
int btrfs_find_root(struct btrfs_root *root, struct btrfs_key *search_key,
struct btrfs_path *path, struct btrfs_root_item *root_item,
struct btrfs_key *root_key);
diff --git a/fs/btrfs/root-tree.c b/fs/btrfs/root-tree.c
index ffb1036..65ecc15 100644
--- a/fs/btrfs/root-tree.c
+++ b/fs/btrfs/root-tree.c
@@ -29,8 +29,8 @@
* generation numbers as then we know the root was once mounted with an older
* kernel that was not aware of the root item structure change.
*/
-void btrfs_read_root_item(struct extent_buffer *eb, int slot,
- struct btrfs_root_item *item)
+static void btrfs_read_root_item(struct extent_buffer *eb, int slot,
+ struct btrfs_root_item *item)
{
uuid_le uuid;
int len;
--
1.8.3.2
From: Sergei Trofimovich <[email protected]>
Found by uselex.rb:
> btrfs_start_transaction_lflush: [R]: exported from: fs/btrfs/btrfs.o fs/btrfs/transaction.o fs/btrfs/built-in.o
Signed-off-by: Sergei Trofimovich <[email protected]>
---
fs/btrfs/transaction.c | 7 -------
fs/btrfs/transaction.h | 2 --
2 files changed, 9 deletions(-)
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index ff891d2..57c3239 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -497,13 +497,6 @@ struct btrfs_trans_handle *btrfs_start_transaction(struct btrfs_root *root,
BTRFS_RESERVE_FLUSH_ALL);
}
-struct btrfs_trans_handle *btrfs_start_transaction_lflush(
- struct btrfs_root *root, int num_items)
-{
- return start_transaction(root, num_items, TRANS_START,
- BTRFS_RESERVE_FLUSH_LIMIT);
-}
-
struct btrfs_trans_handle *btrfs_join_transaction(struct btrfs_root *root)
{
return start_transaction(root, 0, TRANS_JOIN, 0);
diff --git a/fs/btrfs/transaction.h b/fs/btrfs/transaction.h
index af8293f..05d4be1 100644
--- a/fs/btrfs/transaction.h
+++ b/fs/btrfs/transaction.h
@@ -131,8 +131,6 @@ int btrfs_end_transaction(struct btrfs_trans_handle *trans,
struct btrfs_root *root);
struct btrfs_trans_handle *btrfs_start_transaction(struct btrfs_root *root,
int num_items);
-struct btrfs_trans_handle *btrfs_start_transaction_lflush(
- struct btrfs_root *root, int num_items);
struct btrfs_trans_handle *btrfs_join_transaction(struct btrfs_root *root);
struct btrfs_trans_handle *btrfs_join_transaction_nolock(struct btrfs_root *root);
struct btrfs_trans_handle *btrfs_attach_transaction(struct btrfs_root *root);
--
1.8.3.2
From: Sergei Trofimovich <[email protected]>
Found by uselex.rb:
> btrfs_write_and_wait_marked_extents: [R]: exported from: fs/btrfs/btrfs.o fs/btrfs/transaction.o fs/btrfs/built-in.o
Signed-off-by: Sergei Trofimovich <[email protected]>
---
fs/btrfs/transaction.c | 4 ++--
fs/btrfs/transaction.h | 2 --
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index d58cce7..ff891d2 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -837,8 +837,8 @@ int btrfs_wait_marked_extents(struct btrfs_root *root,
* them in one of two extent_io trees. This is used to make sure all of
* those extents are on disk for transaction or log commit
*/
-int btrfs_write_and_wait_marked_extents(struct btrfs_root *root,
- struct extent_io_tree *dirty_pages, int mark)
+static int btrfs_write_and_wait_marked_extents(struct btrfs_root *root,
+ struct extent_io_tree *dirty_pages, int mark)
{
int ret;
int ret2;
diff --git a/fs/btrfs/transaction.h b/fs/btrfs/transaction.h
index 005b037..af8293f 100644
--- a/fs/btrfs/transaction.h
+++ b/fs/btrfs/transaction.h
@@ -160,8 +160,6 @@ int btrfs_should_end_transaction(struct btrfs_trans_handle *trans,
void btrfs_throttle(struct btrfs_root *root);
int btrfs_record_root_in_trans(struct btrfs_trans_handle *trans,
struct btrfs_root *root);
-int btrfs_write_and_wait_marked_extents(struct btrfs_root *root,
- struct extent_io_tree *dirty_pages, int mark);
int btrfs_write_marked_extents(struct btrfs_root *root,
struct extent_io_tree *dirty_pages, int mark);
int btrfs_wait_marked_extents(struct btrfs_root *root,
--
1.8.3.2
From: Sergei Trofimovich <[email protected]>
Found by uselex.rb:
> set_state_private: [R]: exported from: fs/btrfs/disk-io.o fs/btrfs/built-in.o fs/btrfs/btrfs.o
Signed-off-by: Sergei Trofimovich <[email protected]>
---
fs/btrfs/extent_io.c | 2 +-
fs/btrfs/extent_io.h | 1 -
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 583d98b..3912396 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -1810,7 +1810,7 @@ out:
* set the private field for a given byte offset in the tree. If there isn't
* an extent_state there already, this does nothing.
*/
-int set_state_private(struct extent_io_tree *tree, u64 start, u64 private)
+static int set_state_private(struct extent_io_tree *tree, u64 start, u64 private)
{
struct rb_node *node;
struct extent_state *state;
diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
index 3b8c4e2..bb1f75d 100644
--- a/fs/btrfs/extent_io.h
+++ b/fs/btrfs/extent_io.h
@@ -261,7 +261,6 @@ int extent_readpages(struct extent_io_tree *tree,
get_extent_t get_extent);
int extent_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
__u64 start, __u64 len, get_extent_t *get_extent);
-int set_state_private(struct extent_io_tree *tree, u64 start, u64 private);
void extent_cache_csums_dio(struct extent_io_tree *tree, u64 start, u32 csums[],
int count);
void extent_cache_csums(struct extent_io_tree *tree, struct bio *bio,
--
1.8.3.2
From: Sergei Trofimovich <[email protected]>
Found by uselex.rb:
> btrfs_lookup_fs_root: [R]: exported from: fs/btrfs/disk-io.o fs/btrfs/built-in.o fs/btrfs/btrfs.o
Signed-off-by: Sergei Trofimovich <[email protected]>
---
fs/btrfs/disk-io.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 8816847..2338209 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1529,8 +1529,8 @@ fail:
return ret;
}
-struct btrfs_root *btrfs_lookup_fs_root(struct btrfs_fs_info *fs_info,
- u64 root_id)
+static struct btrfs_root *btrfs_lookup_fs_root(struct btrfs_fs_info *fs_info,
+ u64 root_id)
{
struct btrfs_root *root;
--
1.8.3.2
On 8/7/13 4:43 PM, Sergei Trofimovich wrote:
...
> Meet uselex.rb: one-file script to parse 'nm' output:
>
> https://github.com/trofi/uselex/blob/master/uselex.rb
Nice to meet you! I think I've met your close relative,
ftp://ftp.samba.org/pub/unpacked/junkcode/findstatic.pl :)
#!/usr/bin/perl -w
# find a list of fns and variables in the code that could be static
# usually called with something like this:
# findstatic.pl `find . -name "*.o"`
# Andrew Tridgell <[email protected]>
(just FWIW)
-Eric
On Wed, 07 Aug 2013 17:02:23 -0500
Eric Sandeen <[email protected]> wrote:
> On 8/7/13 4:43 PM, Sergei Trofimovich wrote:
>
> ...
>
> > Meet uselex.rb: one-file script to parse 'nm' output:
> >
> > https://github.com/trofi/uselex/blob/master/uselex.rb
>
> Nice to meet you! I think I've met your close relative,
> ftp://ftp.samba.org/pub/unpacked/junkcode/findstatic.pl :)
Yeah, even finds the same.
Thanks!
--
Sergei
On 07.08.2013 23:43, Sergei Trofimovich wrote:
> From: Sergei Trofimovich <[email protected]>
>
> Found by uselex.rb:
>> btrfs_reada_detach: [R]: exported from: fs/btrfs/btrfs.o fs/btrfs/built-in.o fs/btrfs/reada.o
even though the function is currently unused, I'm hesitating to remove it
as it's part of the reada-API and might be handy for anyone going to use
the API in the future.
-Arne
>
> Signed-off-by: Sergei Trofimovich <[email protected]>
> ---
> fs/btrfs/ctree.h | 1 -
> fs/btrfs/reada.c | 9 +--------
> 2 files changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index e91ab9e..f35e086 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -3861,7 +3861,6 @@ struct reada_control {
> struct reada_control *btrfs_reada_add(struct btrfs_root *root,
> struct btrfs_key *start, struct btrfs_key *end);
> int btrfs_reada_wait(void *handle);
> -void btrfs_reada_detach(void *handle);
> int btree_readahead_hook(struct btrfs_root *root, struct extent_buffer *eb,
> u64 start, int err);
>
> diff --git a/fs/btrfs/reada.c b/fs/btrfs/reada.c
> index 1031b69..c41d470 100644
> --- a/fs/btrfs/reada.c
> +++ b/fs/btrfs/reada.c
> @@ -37,7 +37,7 @@
> * To trigger a readahead, btrfs_reada_add must be called. It will start
> * a read ahead for the given range [start, end) on tree root. The returned
> * handle can either be used to wait on the readahead to finish
> - * (btrfs_reada_wait), or to send it to the background (btrfs_reada_detach).
> + * (btrfs_reada_wait).
> *
> * The read ahead works as follows:
> * On btrfs_reada_add, the root of the tree is inserted into a radix_tree.
> @@ -979,10 +979,3 @@ int btrfs_reada_wait(void *handle)
> return 0;
> }
> #endif
> -
> -void btrfs_reada_detach(void *handle)
> -{
> - struct reada_control *rc = handle;
> -
> - kref_put(&rc->refcnt, reada_control_release);
> -}
On Thu, Aug 08, 2013 at 09:33:14AM +0200, Arne Jansen wrote:
> On 07.08.2013 23:43, Sergei Trofimovich wrote:
> > From: Sergei Trofimovich <[email protected]>
> >
> > Found by uselex.rb:
> >> btrfs_reada_detach: [R]: exported from: fs/btrfs/btrfs.o fs/btrfs/built-in.o fs/btrfs/reada.o
>
> even though the function is currently unused, I'm hesitating to remove it
> as it's part of the reada-API and might be handy for anyone going to use
> the API in the future.
I agree. As replied here,
http://www.mail-archive.com/[email protected]/msg24047.html
please keep the function.
david
On Thu, Aug 08, 2013 at 12:43:20AM +0300, Sergei Trofimovich wrote:
> From: Sergei Trofimovich <[email protected]>
>
> Found by uselex.rb:
> > btrfs_start_transaction_lflush: [R]: exported from: fs/btrfs/btrfs.o fs/btrfs/transaction.o fs/btrfs/built-in.o
http://www.mail-archive.com/[email protected]/msg24047.html
"> btrfs_start_transaction_lflush()
Transcaction API, removing the func does not make sense without removing
BTRFS_RESERVE_FLUSH_LIMIT at the same time.
"
Miao introduced this function in 08e007d2e57744472a9424735a to enhance
flushing logic to avoid deadlocks.
david
On Thu, Aug 08, 2013 at 12:43:19AM +0300, Sergei Trofimovich wrote:
> From: Sergei Trofimovich <[email protected]>
>
> Found by uselex.rb:
> > btrfs_write_and_wait_marked_extents: [R]: exported from: fs/btrfs/btrfs.o fs/btrfs/transaction.o fs/btrfs/built-in.o
>
> Signed-off-by: Sergei Trofimovich <[email protected]>
> ---
> fs/btrfs/transaction.c | 4 ++--
> fs/btrfs/transaction.h | 2 --
> 2 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> index d58cce7..ff891d2 100644
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -837,8 +837,8 @@ int btrfs_wait_marked_extents(struct btrfs_root *root,
> * them in one of two extent_io trees. This is used to make sure all of
> * those extents are on disk for transaction or log commit
> */
> -int btrfs_write_and_wait_marked_extents(struct btrfs_root *root,
> - struct extent_io_tree *dirty_pages, int mark)
> +static int btrfs_write_and_wait_marked_extents(struct btrfs_root *root,
> + struct extent_io_tree *dirty_pages, int mark)
You may want to run the output through checkpatch.pl and fix obvious
style violations (line too long).
david
On Thu, Aug 08, 2013 at 12:43:23AM +0300, Sergei Trofimovich wrote:
> From: Sergei Trofimovich <[email protected]>
>
> Found by uselex.rb:
> > btrfs_get_inode_ref_index: [R]: exported from: fs/btrfs/inode-item.o fs/btrfs/btrfs.o fs/btrfs/built-in.o
>
> Signed-off-by: Sergei Trofimovich <[email protected]>
Safe to remove.
Reviewed-by: David Sterba <[email protected]>
> > even though the function is currently unused, I'm hesitating to remove it
> > as it's part of the reada-API and might be handy for anyone going to use
> > the API in the future.
>
> I agree. As replied here,
> http://www.mail-archive.com/[email protected]/msg24047.html
> please keep the function.
If we're keeping score, put me down for being in favour of removing dead
untested code. git ressurection is easy.
- z
On 08/08/13 19:46, Zach Brown wrote:
>>> even though the function is currently unused, I'm hesitating to remove it
>>> as it's part of the reada-API and might be handy for anyone going to use
>>> the API in the future.
>>
>> I agree. As replied here,
>> http://www.mail-archive.com/[email protected]/msg24047.html
>> please keep the function.
>
> If we're keeping score, put me down for being in favour of removing dead
> untested code. git ressurection is easy.
It's not really untested, it has been in use some time ago. But of
course there's a chance that some changes broke it.
Yes, git ressurection is easy. To inform potential users, you might
just leave a comment like this:
/*
* There has been a function once to detach from a running reada.
* If you need such functionality, just revert the commit that
* added this comment.
*/
-Arne
>
> - z
>
On Thu, Aug 08, 2013 at 09:11:01PM +0200, Arne Jansen wrote:
> On 08/08/13 19:46, Zach Brown wrote:
> >>> even though the function is currently unused, I'm hesitating to remove it
> >>> as it's part of the reada-API and might be handy for anyone going to use
> >>> the API in the future.
> >>
> >> I agree. As replied here,
> >> http://www.mail-archive.com/[email protected]/msg24047.html
> >> please keep the function.
> >
> > If we're keeping score, put me down for being in favour of removing dead
> > untested code. git ressurection is easy.
>
> It's not really untested, it has been in use some time ago. But of
> course there's a chance that some changes broke it.
> Yes, git ressurection is easy. To inform potential users, you might
> just leave a comment like this:
>
> /*
> * There has been a function once to detach from a running reada.
> * If you need such functionality, just revert the commit that
> * added this comment.
> */
And please write the exact commit sha1 instead of 'the commit' :)
I've used the _detach function when prototyping readdir readahead, that
did not bring the speedup as expected so more work is needed, that's why
I'm concerned about removing it. But, if Arne is ok with that, so be it.
david