diff mbox series

[09/20,software,coproarchaeology] dentry.h: kill a mysterious comment

Message ID 20231124060644.576611-9-viro@zeniv.linux.org.uk (mailing list archive)
State New
Headers show
Series [01/20] selinux: saner handling of policy reloads | expand

Commit Message

Al Viro Nov. 24, 2023, 6:06 a.m. UTC
there's a strange comment in front of d_lookup() declaration:

/* appendix may either be NULL or be used for transname suffixes */

Looks like nobody had been curious enough to track its history;
it predates git, it predates bitkeeper and if you look through
the pre-BK trees, you finally arrive at this in 2.1.44-for-davem:
  /* appendix may either be NULL or be used for transname suffixes */
 -extern struct dentry * d_lookup(struct inode * dir, struct qstr * name,
 -                               struct qstr * appendix);
 +extern struct dentry * d_lookup(struct dentry * dir, struct qstr * name);
In other words, it refers to the third argument d_lookup() used to have
back then.  It had been introduced in 2.1.43-pre, on June 12 1997,
along with d_lookup(), only to be removed by July 4 1997, presumably
when the Cthulhu-awful thing it used to be used for (look for
CONFIG_TRANS_NAMES in 2.1.43-pre, and keep a heavy-duty barfbag
ready) had been, er, noticed and recognized for what it had been.

Despite the appendectomy, the comment remained.  Some things really
need to be put out of their misery...

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 include/linux/dcache.h | 1 -
 1 file changed, 1 deletion(-)

Comments

Amir Goldstein Nov. 24, 2023, 7:37 a.m. UTC | #1
On Fri, Nov 24, 2023 at 8:07 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> there's a strange comment in front of d_lookup() declaration:
>
> /* appendix may either be NULL or be used for transname suffixes */
>
> Looks like nobody had been curious enough to track its history;
> it predates git, it predates bitkeeper and if you look through
> the pre-BK trees, you finally arrive at this in 2.1.44-for-davem:
>   /* appendix may either be NULL or be used for transname suffixes */
>  -extern struct dentry * d_lookup(struct inode * dir, struct qstr * name,
>  -                               struct qstr * appendix);
>  +extern struct dentry * d_lookup(struct dentry * dir, struct qstr * name);
> In other words, it refers to the third argument d_lookup() used to have
> back then.  It had been introduced in 2.1.43-pre, on June 12 1997,
> along with d_lookup(), only to be removed by July 4 1997, presumably
> when the Cthulhu-awful thing it used to be used for (look for
> CONFIG_TRANS_NAMES in 2.1.43-pre, and keep a heavy-duty barfbag
> ready) had been, er, noticed and recognized for what it had been.
>
> Despite the appendectomy, the comment remained.  Some things really
> need to be put out of their misery...
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
>  include/linux/dcache.h | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/include/linux/dcache.h b/include/linux/dcache.h
> index 9706bf1dc5de..a5e5e274eee0 100644
> --- a/include/linux/dcache.h
> +++ b/include/linux/dcache.h
> @@ -270,7 +270,6 @@ extern void d_move(struct dentry *, struct dentry *);
>  extern void d_exchange(struct dentry *, struct dentry *);
>  extern struct dentry *d_ancestor(struct dentry *, struct dentry *);
>
> -/* appendix may either be NULL or be used for transname suffixes */
>  extern struct dentry *d_lookup(const struct dentry *, const struct qstr *);
>  extern struct dentry *d_hash_and_lookup(struct dentry *, struct qstr *);
>

Al,

Since you like pre-git archeology...

Mind digging up what this comment in fs.h is about:

/* needed for stackable file system support */
extern loff_t default_llseek(struct file *file, loff_t offset, int whence);

Or we can just remove it without digging up what the comment used
to refer to ;)

Thanks,
Amir.
Al Viro Nov. 24, 2023, 8:11 a.m. UTC | #2
On Fri, Nov 24, 2023 at 09:37:16AM +0200, Amir Goldstein wrote:

> Since you like pre-git archeology...
> 
> Mind digging up what this comment in fs.h is about:
> 
> /* needed for stackable file system support */
> extern loff_t default_llseek(struct file *file, loff_t offset, int whence);

Umm...  I think it was about ecryptfs and its ilk, but it was a long
time ago...

<looks>

2.3.99pre6, along with exporting it:
-/* for stackable file systems (lofs, wrapfs, etc.) */
-EXPORT_SYMBOL(add_to_page_cache);
+/* for stackable file systems (lofs, wrapfs, cryptfs, etc.) */
+EXPORT_SYMBOL(default_llseek);
+EXPORT_SYMBOL(dentry_open);

Back then ->llseek == NULL used to mean default_llseek; that had
been changed much later.  And that was before vfs_llseek() as well,
so any layered filesystem had to open-code it - which required
default_llseek().

The comment is certainly stale, though - stackable filesystems do *not*
need it (vfs_llseek() is there), but every file_operation that used
to have NULL ->llseek does.

> Or we can just remove it without digging up what the comment used
> to refer to ;)

Too late - it will have to be removed with that ;-)
Al Viro Nov. 24, 2023, 8:26 a.m. UTC | #3
On Fri, Nov 24, 2023 at 08:11:41AM +0000, Al Viro wrote:
> On Fri, Nov 24, 2023 at 09:37:16AM +0200, Amir Goldstein wrote:
> 
> > Since you like pre-git archeology...
> > 
> > Mind digging up what this comment in fs.h is about:
> > 
> > /* needed for stackable file system support */
> > extern loff_t default_llseek(struct file *file, loff_t offset, int whence);
> 
> Umm...  I think it was about ecryptfs and its ilk, but it was a long
> time ago...
> 
> <looks>
> 
> 2.3.99pre6, along with exporting it:
> -/* for stackable file systems (lofs, wrapfs, etc.) */
> -EXPORT_SYMBOL(add_to_page_cache);
> +/* for stackable file systems (lofs, wrapfs, cryptfs, etc.) */
> +EXPORT_SYMBOL(default_llseek);
> +EXPORT_SYMBOL(dentry_open);
> 
> Back then ->llseek == NULL used to mean default_llseek; that had
> been changed much later.  And that was before vfs_llseek() as well,
> so any layered filesystem had to open-code it - which required
> default_llseek().
> 
> The comment is certainly stale, though - stackable filesystems do *not*
> need it (vfs_llseek() is there), but every file_operation that used
> to have NULL ->llseek does.
> 
> > Or we can just remove it without digging up what the comment used
> > to refer to ;)
> 
> Too late - it will have to be removed with that ;-)

BTW, there's this, covering more than just BK times:

git://git.kernel.org/pub/scm/linux/kernel/git/history/history.git/
Christian Brauner Nov. 24, 2023, 8:29 a.m. UTC | #4
> BTW, there's this, covering more than just BK times:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/history/history.git/

An absolute life-saver at times this repo for the ones of us who haven't
been around for 20+ years...
diff mbox series

Patch

diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index 9706bf1dc5de..a5e5e274eee0 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -270,7 +270,6 @@  extern void d_move(struct dentry *, struct dentry *);
 extern void d_exchange(struct dentry *, struct dentry *);
 extern struct dentry *d_ancestor(struct dentry *, struct dentry *);
 
-/* appendix may either be NULL or be used for transname suffixes */
 extern struct dentry *d_lookup(const struct dentry *, const struct qstr *);
 extern struct dentry *d_hash_and_lookup(struct dentry *, struct qstr *);