[Outreachy kernel] [PATCH 0/2] Staging: lustre: Remove unused functions

[Arnd Bergmann]

On Thursday 22 October 2015 16:59:01 Shraddha Barke wrote:
> On Thu, 22 Oct 2015, Arnd Bergmann wrote:
> 
> > On Thursday 22 October 2015 15:16:39 Shraddha Barke wrote:
> >> On Thu, 22 Oct 2015, Arnd Bergmann wrote:
> >>
> >>> On Thursday 22 October 2015 14:50:18 Shraddha Barke wrote:
> >>>> These patches remove the definitions of functions which are not used.
> >>>>
> >>>> Shraddha Barke (2):
> >>>>   Staging: lustre: ptlrpc: Remove unused functions
> >>>>   Staging: lustre: obdclass: Remove unused functions
> >>>>
> >>>>  .../lustre/lustre/obdclass/lprocfs_status.c        | 139 ---------------------
> >>>>  .../staging/lustre/lustre/ptlrpc/lproc_ptlrpc.c    |  63 ----------
> >>>>  2 files changed, 202 deletions(-)
> >>>>
> >>>>
> >>>
> >>> How did you check that they are indeed unused? I suspect these two patches
> >>> are wrong.
> >>
> >> I grepped for the uses of these functions. When I did make
> >> M=drivers/staging/lustre, I didn't get errors. What am I missing here?
> >
> > I see now that you did not remove the declarations from the header files.
> > Please remove those and try again. Alternatively, you should see link-time
> > errors when you try to do 'make' without the listing directory.
> 
> Yes, you are right. This patchset is incorrect. However I still don't 
> understand why I get these errors when it seems like these 
> functions are not used anywhere. confused

The problem here is that there are macros using string concatenation
to create references to these functions in
drivers/staging/lustre/lustre/include/lprocfs_status.h:

#define LPROC_SEQ_FOPS_WR_ONLY(name, type)                              \
        static ssize_t name##_##type##_write(struct file *file,         \
                        const char __user *buffer, size_t count,        \
                                                loff_t *off)            \
        {                                                               \
                return lprocfs_wr_##type(file, buffer, count, off);     \
        }                                                               \
        static int name##_##type##_open(struct inode *inode, struct file *file) \
        {                                                               \
                return single_open(file, NULL, inode->i_private);       \
        }                                                               \
        static struct file_operations name##_##type##_fops = {  \
                .open   = name##_##type##_open,                         \
                .write  = name##_##type##_write,                        \
                .release = lprocfs_single_release,                      \
        }

I made the same mistake when I removed a lot of unused code from this
file system, but I caught it during the compile-testing.

I usually try to avoid macros like this in my own code, or at least make
references to other identifiers explicit. It might be a good idea to
try replacing the macros with calls to the normal APIs that we have
in include/linux/debugfs.h for the same purpose, but I'm not sure if
that is an overall win.

	Arnd