Messages in this thread | | | Date | Tue, 5 May 2015 08:01:52 -0700 | Subject | Re: [PATCH 04/79] ext4: split inode_operations for encrypted symlinks off the rest | From | Linus Torvalds <> |
| |
On Mon, May 4, 2015 at 10:21 PM, Al Viro <viro@zeniv.linux.org.uk> wrote: > } else if (S_ISLNK(inode->i_mode)) { > - if (ext4_inode_is_fast_symlink(inode) && > - !ext4_encrypted_inode(inode)) { > + if (ext4_encrypted_inode(inode)) { > +#ifdef CONFIG_EXT4_FS_ENCRYPTION > + inode->i_op = &ext4_encrypted_symlink_inode_operations; > + ext4_set_aops(inode); > +#else > + BUILD_BUG(); > +#endif > + } else if (ext4_inode_is_fast_symlink(inode)) { > inode->i_op = &ext4_fast_symlink_inode_operations; > nd_terminate_link(ei->i_data, inode->i_size, > sizeof(ei->i_data) - 1);
Ugh. Could we aim to *not* add code like this.
Instead, just declare (but don't define) the ext4_encrypted_symlink_inode_operations thing, so that *if* somebody uses it they get a link error, and make sure that "ext4_encrypted_inode()" ends up always returning zero when encryption isn't enabled, so that the compiler will actually optimize the whole thing out (which apparently is already the case, judging by the build-bug-on.
I really prefer to not see #ifdef's inside the middle of anything but trivial helper functions. I know we have them, and I wish we didn't, but at least we can aim to not add more of them.
Linus
| |