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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On Thu, 22 Oct 2015, Arnd Bergmann wrote:

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.

Thanks! It is much clear to me now :)
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.

I'll try to do this.

Shraddha

	Arnd

_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux