Re: [PATCH v5 1/7] fs: introduce kernel_pread_file* support

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

 



Hi Luis,

A few comments inline before I cleanup.

On 2020-05-12 5:27 p.m., Luis Chamberlain wrote:
On Thu, May 07, 2020 at 05:27:33PM -0700, Scott Branden wrote:
diff --git a/fs/exec.c b/fs/exec.c
index 06b4c550af5d..cfab212fab9d 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -896,10 +896,14 @@ struct file *open_exec(const char *name)
  }
  EXPORT_SYMBOL(open_exec);
-int kernel_read_file(struct file *file, void **buf, loff_t *size,
-		     loff_t max_size, enum kernel_read_file_id id)
-{
-	loff_t i_size, pos;
+int kernel_pread_file(struct file *file, void **buf, loff_t *size,
+		      loff_t pos, loff_t max_size, unsigned int flags,
You use int flags, but.. these are mutually exclusive operations, and
so flags is a misnomer. Just use an enum instead of a define, that way
we can use kdoc for documentation.
OK, flags could be used to expand with additional flag options in the future (without change to function prototype, but will change to enum if that is what is desired.
+EXPORT_SYMBOL_GPL(kernel_pread_file);
+EXPORT_SYMBOL_GPL(kernel_pread_file_from_path);
+EXPORT_SYMBOL_GPL(kernel_pread_file_from_path_initns);
+EXPORT_SYMBOL_GPL(kernel_pread_file_from_fd);
If no one is using these don't export them. I think you only use one of
these. In fact just remove the code from the ones which are not used.
I do not use them but added them to provide consistent api with kernel_read_file_* functions.  That way someone can take advantage of the _from_path and from_fd variants in the future if desired. But if you want them removed it is simple to drop the EXPORT_SYMBOL_GPL and then add that back when first driver that calls them needs them in the future.

Note: Existing kernel_read_file_from_path_initns is not used in the kernel.  Should we delete that as well?


   Luis
Thanks,
 Scott



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux